Fix error handling in mysqli driver #752
Conversation
|
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DBAL-1088 We use Jira to track the state of pull requests and the versions they got |
|
👍 |
|
@nicolas-grekas looks good to me but travis is failing now for mysqli. Can you please investigate? |
nicolas-grekas
changed the title
|
Tests are back to green for mysqli. Failure was related to a bug in MysqliStatement that is also fixed by this PR now. |
| @@ -300,7 +300,7 @@ public function fetchColumn($columnIndex = 0) | |||
| return false; | |||
| } | |||
|
|
|||
| return $row[$columnIndex]; | |||
| return isset($row[$columnIndex]) ? $row[$columnIndex] : null; | |||
There was a problem hiding this comment.
This other changed code-path requires a test case
There was a problem hiding this comment.
There is one: the one that was failing before the patch :)
There was a problem hiding this comment.
And it was explicitly testing for non-existing $columnIndex?
There was a problem hiding this comment.
Alright then :-)
@nicolas-grekas I'm always this paranoid just to make sure ;-)
There was a problem hiding this comment.
@Ocramius @nicolas-grekas is right, there were tests introduced in #709. What is weird is that the tests were not failing in that PR already oO.
There was a problem hiding this comment.
@deeky666 if the error handler kicked in, then of course the tests passed :)
There was a problem hiding this comment.
@Ocramius but temporarily disabling the error handler is just for grabbing connection errors, no? It is restored afterwards anyways so failing queries are not affected by this or am I getting something wrong here?
There was a problem hiding this comment.
It was not restored correctly before the patch. The side effect of this bug was that the silencing error handler was still in place. Thus, all later errors were silenced.
There was a problem hiding this comment.
@nicolas-grekas I see. Thought it was just a "cleaner" implementation. So merging then. Thanks a lot! :)
|
@deeky666 mergeable! |
Fix error handling in mysqli driver
PHP already handles an internal stack of error handlers