Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for issue 1011 - set PDO type int for double vars #1020

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

axelhahn
Copy link

set 'double' => PDO::PARAM_INT to prevent from the value quoting in generator().

@kukudeliu
Copy link

have the same problem

@jxcodes
Copy link

jxcodes commented Jul 15, 2023

I'm having the same issue, if it's a simpe change i wander why the author does not include it. @axelhahn, does your solution works well with decimal numbers?

@axelhahn
Copy link
Author

axelhahn commented Jul 15, 2023

@jxcodes Huuuh, it was a while ago hat I went ito that trap.

If I have a look to https://www.php.net/manual/en/pdo.constants.php - it says:

 PDO::PARAM_INT (int)
    Represents the SQL INTEGER data type. 

... so: the answer is NO - it doesn't handle float values - it is an integer.

https://www.w3schools.com/sql/sql_datatypes.asp
Database type INT: A medium integer. Signed range is from -2147483648 to 2147483647. Unsigned range is from 0 to 4294967295.

Maybe my issue is, that my integer value was detected as "double" instead of "integer".

If you need to handle a real float value handled as a double - this I didn't tested. Maybe the solution goes into the direction to use the current mappting to PDO::PARAM_STR but to add an if condition in https://github.com/catfan/Medoo/blob/master/src/Medoo.php#L756 that removes the quote chars from a string.

@catfan
Copy link
Owner

catfan commented Jul 16, 2023

The double data should be PDO::PARAM_STR for database binding, and PDO::PARAM_INT is incorrect.

For generate(), I still think it should handle it as a string with quote(), because it indeed does that for binding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants