[POC] Fix #64531: SQLite3Result::fetchArray runs the query again #5204
Conversation
`SQLite3::query()` and `SQLite3Stmt::execute()` call `sqlite3_step()` to determine whether to return a `SQLite3Result` or `false`. Then they call `sqlite3_reset()`, so that `SQLite3Result::fetchArray()` can fetch values from the first row. This obviously leads to issues if the SQL query causes any side effects. We solve this by not calling `sqlite_reset()` anymore, but instead storing the last fetch result code on the statement object, and to use this in `::fetchArray()` when the first row is to be fetched. This also allows us to prevent an SQLite3 quirk which automatically resets a statement when `sqlite_fetch()` is called after the last fetch returned anything else than `SQLITE_ROW`; thus fixing bug #79293.
|
I'm closing this, because it would merely mitigate some symptoms, but not the root cause, which is the unfortunate introduction of SQLite3Result, a concept that can't be cleanly mapped to the SQLite3 API (see also https://bugs.php.net/73530). Only allowing at most a single result for any statement would mostly solve the root cause (but would be an API break), but would still require us to fiddle around regarding some details (such as this issue). Users are likely better off to use PDO_SQLite which doesn't suffer from this defect, although it does not support some SQLite3 specific functionality like the backup API (and it is doubtful that such specific stuff will ever make it into PDO_SQLite3; cf. https://wiki.php.net/rfc/implement_sqlite_openblob_in_pdo). |
Fixes an existing issue tha was surfaced in php#5204 where re-using a variable to get the result of a statement on the second round of execution would cause a spurious reset call to be made to SQLite between execute() and fetchArray() being called. This was previously not changing the behaviour, as reset was already being called too many times, but it did mean the bug was not fixed for all potential scenarios.
Fixes an existing issue tha was surfaced in php#5204 where re-using a variable to get the result of a statement on the second round of execution would cause a spurious reset call to be made to SQLite between execute() and fetchArray() being called. This was previously not changing the behaviour, as reset was already being called too many times, but it did mean the bug was not fixed for all potential scenarios.
SQLite3::query()andSQLite3Stmt::execute()callsqlite3_step()to determine whether to return a
SQLite3Resultorfalse. Then theycall
sqlite3_reset(), so thatSQLite3Result::fetchArray()can fetchvalues from the first row. This obviously leads to issues if the SQL
query causes any side effects. We solve this by not calling
sqlite_reset()anymore, but instead storing the last fetch resultcode on the statement object, and to use this in
::fetchArray()whenthe first row is to be fetched.
This also allows us to prevent an SQLite3 quirk which automatically
resets a statement when
sqlite_fetch()is called after the last fetchreturned anything else than
SQLITE_ROW; thus fixing bug #79293.As is, this would break
SQLite3::lastErrorCode()and::lastErrorMsg(), so should not really go into PHP-7.3. However, these are brittle anyway (and actually not necessarily correct), since almost any SQLite3 API can change the error code, but PHP users are likely only interested in relevant API calls.