Fix PDOStatement->execute() failed, then execute successfully, errorInfo() information is incorrect #8628
Conversation
…nfo() information is incorrect
Yurunsoft
force-pushed
the
fix-pdo-stmt-errorInfo-8.0
branch
from
3118ad0 to
e1ba1b2
Compare
ext/pdo/tests/gh8626.phpt
Outdated
| if (!extension_loaded('pdo') || !extension_loaded('pdo_mysql')) die('skip'); | ||
| $dir = getenv('REDIR_TEST_DIR'); | ||
| if (false == $dir) die('skip no driver'); | ||
| if (!str_starts_with(getenv('PDOTEST_DSN'), 'mysql')) die('skip non-mysql drivers'); |
There was a problem hiding this comment.
If the test is only meant for PDO_MySQL, there is no point in putting it into ext/pdo/tests and making it a redirect test. Instead, the test should be put into ext/pdo_mysql/tests. However, since there is a behavioral change for all PDO drivers, shouldn't we test it for all drivers?
There was a problem hiding this comment.
I'm not very familiar with other databases, so I had to use MySQL to test it.
And, I'm not quite sure why the I386 test failed.
There was a problem hiding this comment.
I think the problem the usage of PHP_INT_MIN to trigger a failure. PHP_INT_MIN is -2147483648 on 32bit architectures, and that wouldn't be problem for the database. Can't we use something else to trigger a failure? I'm thinking about trying to insert a non numeric string into an INTEGER column, or such. That should work for other drivers as well.
There was a problem hiding this comment.
I've added mysql, pgsql, sqlite tests.
Other databases I am really not familiar with and cannot test.
|
Merged with df52903ee. Thank you. |
|
Something is still not right with the tests here https://github.com/php/php-src/runs/6712417535?check_suite_focus=true I suppose DBLIB needs the same exception as ODBC. |
Fix #8626
This problem exists for both PHP 7.4, 8.0, 8.1
Reference:
php-src/ext/pdo/pdo_dbh.c
Line 1054 in af20923