Fixed DOUBLE EXECUTION OF QUERIES when executing first SQLite3Result-… #8415
Conversation
…>fetcharray() Steps to fix the bug: 1. Disabled sqlite3_reset(stmt_obj->stmt) in SQLite3::query() and SQLite3Stmt::execute(). sqlite3_reset(stmt_obj->stmt) forces a new execution of the query on the first execution of SQLite3Result::fetcharray(). 2. Added 'int last_error' to struct '_php_sqlite3_result_object'. last_error is used to store the returned value of the step command executed in SQLite3::query() and SQLite3Stmt::execute(), in order to pass this step return code to SQLite3Result::fetcharray(). 3. SQLite3Result::fetcharray() reads last_error, process it and only steps if it is equal to SQLITE_ROW.
…DOUBLE QUERY EXECUTION on first SQLite3Result->fetchArray() Store the last step error code from Sqlite3::query(), SQlite3Stmt::execute() in _php_sqlite3_result_object, to passe it to SQLite3Result::fetchArray(). Also stores last step error from SQLite3Result::fetchArray() to pass it to next SQLite3Result::fetchArray().
|
Please do not submit new pull requests if you just want to add commits to an existing PR. Anyhow, this solution looks a bit like my previous attempt at fixing this (PR #5204), and like I commented there, this would break |
|
Hi Christoph, Ok, I reviewed my code and wrote a small correction that, as far as I know, does not break SQLite3::lastErrorCode() and ::lastErrorMsg(). It does not change or affect the error processing logic of the code. I guess that this last patch fixes this bug for good. It's working fine in my PHP build. As I commented, I'm new to Github. In order to have this code reviewed and merged, I only have to commit it to HochMartinez:patch-2? No pull requests?... |
Yes, that would work. |
…esult->fetchArray() This fix prevents avoids the double execution of a query when calling SQLite3Result->fetchArray(). This second fix does not alter or affect the error logic of SQLite3Result->fetchArray().
Correction to a comment.
|
Hi Christoph, I just pushed the new fix here: It affects two files, Sqlite3.c and php_sqlite3_structs.h. As I commented, the patch does not affect the error logic. It's a very simple fix, so all should work fine. Thanks and greetings from Barcelona... :-) |
|
Thanks for the update! There are still failing tests; please check that. |
|
Hi Christoph, I can't see the relationship of the errors with the changes I made. I append the errors. You have a lot more experience. Maybe you could check what happens. As we commented the patch I submitted is very simple and, as far as I know, does not affect the error handling logic. Greetings! ............................................................................... ========DIFF======== ========DIFF======== ========DIFF========Warning: symlink(): open_basedir restriction in effect. File(%s/test/bad/bad.txt) is not within the allowed path(s): (.) in %s on line %d |
|
I'm referring to the 4 failing sqlite3 tests. These are likely caused by your changes. |
|
@HochMartinez Have you been able to investigate the 4 failing sqlite3 tests that @cmb69 referenced? |
|
Hi Ben,
No, sorry, I'm super busy, lately.
And the patch I submitted is working fine for me. It solves the double
execution of queries and it is stable.
Maybe someone else can take a look at the 4 failing tests?
The patches I submitted are very, very simple.
Greetings!
El dom., 22 may. 2022 19:05, Ben Ramsey ***@***.***> escribió:
… @HochMartinez <https://github.com/HochMartinez> Have you been able to
investigate the 4 failing sqlite3 tests that @cmb69
<https://github.com/cmb69> referenced?
—
Reply to this email directly, view it on GitHub
<#8415 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANXYDDTEGCQZZO63ZCWH7ZDVLJSMVANCNFSM5T66K6AA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I’d love to get this patch into 8.2 so that it can help others. I hope you can find the time to add tests so we can merge this ahead of the first alpha release. Thanks! |
|
There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken. |
…>fetcharray()
Steps to fix the bug:
Disabled sqlite3_reset(stmt_obj->stmt) in SQLite3::query() and SQLite3Stmt::execute().
sqlite3_reset(stmt_obj->stmt) forces a new execution of the query on the first execution of SQLite3Result::fetcharray().
Added 'int last_error' to struct '_php_sqlite3_result_object'.
last_error is used to store the returned value of the step command executed in SQLite3::query() and SQLite3Stmt::execute(), in order to pass this step return code to SQLite3Result::fetcharray().
SQLite3Result::fetcharray() reads last_error, process it and only steps if it is equal to SQLITE_ROW.