Rebind row values to the statement after freeing result

[morozov]

If prepared statement is reused for multiple queries, and after one of the executions, the statement cursor is closed ($stmt->closeCursor()), all consequent executions will return the last row returned by $stmt->_fetch() before the cursor was closed.

The reason is that mysqli_stmt::free_result() also unbinds result variables from the statement, and they don't get bound back upon next execution.

[kimhemsoe]

Why not reset rowBindedValues and hereby avoid rowValuesAreBound?

[morozov]

@kimhemsoe by "reset" you mean re-bind? We can extract the procedure of binding row values to a separate method and call it from execute() and from here.

However, we don't know if the statement is going to be reused after closing cursor, so setting a flag saves us some CPU ticks. What do you say?

[kimhemsoe]

Not sure. It is just moving store_result() scares me alot. In history it have been a very picky call to get right, with all the different drivers and such.

[morozov]

I removed the store_result() part from this patch as irrelevant.

[kimhemsoe]

I think we should combine the 2 PR you have into one with one test. They are both very similar.

[kimhemsoe]

Also we need someone to test the none pdo drivers. Adding a test for this will make some of the other drivers fail.

[Ocramius]

@kimhemsoe the 2 PRs are fine as separate as they are (makes it easier to track what was done, too).

What do you mean by none pdo drivers?

[kimhemsoe]

@Ocramius All those drivers which are not PDO. Mysqli is only driver that is not PDO and is getting tested by travis.

[Ocramius]

Mysqli is only driver that is not PDO and is getting tested by travis.

Ah yes, we have very few unit tests there, I fear. Can't do much about it besides adding them, but can't ask to scope-creep this PR either.

[kimhemsoe]

No ofc not, just saying that we introducing failing tests for the other drivers :)

[morozov]

I think we should combine the 2 PR you have into one with one test. They are both very similar.

I'd say it's better to keep them separate since these are two completely independent issues. In fact, fixing this one doesn't require calling $this->_stmt->store_result(); on every execution, which is part of fixing the previous issue. I can remove this part, but it will introduce a merge conflict between the two.

[morozov]

Also we need someone to test the none pdo drivers. Adding a test for this will make some of the other drivers fail.

Right now the test skips all drivers besides mysqli. Do you want to be able to run it for all of them? I can test sqlsrv, ibm_db2 and oci8.

[kimhemsoe]

@morozov If you have the resources to test the other drivers afterwards, you are were much welcome. But for now lets drag them in here more then i already have done.

[morozov]

I checked the test case against sqlsrv, ibm_db2 and oci8 manually and reworked the test to cover all supported drivers. Have no idea how to setup the environment for SQL Anywhere.

Looks like I looked at the wrong place, since all the adapters above close the statement resource as part of closeCursor(), so the statement cannot be reused at all. Unlike those, the adapters for mysqli and sasql keep the statement in place and allow the reuse. The fact that the statement is closed contradicts the purpose of the method:

Closes the cursor, enabling the statement to be executed again.

[morozov]

So, apart from the original issue, a related issue with closing cursor and then reusing statement has been fixed for Oracle, IBM DB2 and SQL Server platforms. Can we handle them together, since they can be reproduced with a similar use case, or we should split them apart?

[morozov]

Closing in favor of #2536 and #2546.