-
Notifications
You must be signed in to change notification settings - Fork 12
Fix binding of PDO::PARAM_BOOL (Issue #12) #29
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
Conversation
|
@shivammathur is |
|
Yes, please use |
Thanks @shivammathur for the quick response |
|
|
||
| $conn = PDOTest::factory(); | ||
|
|
||
| $stmt = $conn->prepare('SELECT ?, ? FROM DUAL'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like another test with (temporary) table with BOOLEAN type. The expect result is false/true inserted correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mvorisek BOOLEAN data type columns are supported only from Oracle Database 23ai. Is that what you are referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the BOOLEAN data type gives a warning with Oracle 23ai (and 26ai) client libraries as these libraries have implemented the BOOLEAN data type.
Warning: PDOStatement::fetchAll(): Column 1 data was too large for buffer and was truncated to fit
You can check the failed builds in this PR for the error.
So the pdo_oci driver needs an enhancement to support the Oracle 23ai Client implementation of the BOOLEAN data type.
Co-authored-by: Michael Voříšek <[email protected]>
ndossche
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically looks fine, didn't try running the tests
|
I will be merging this for now. Please see comment #29 (comment) on the BOOLEAN data types. |
This is an improvement from PR #15 based on inputs from @mvorisek and @ndossche and with an added test.
Credited Ashutosh Agrawal and Michael Vorisek.