Repository navigation
Conversation
…ring fetch A collation callback cannot report an error to SQLite, so the Python exception stays set. A later error in the same step can make sqlite3_step() fail. Then pysqlite_cursor_iternext() calls set_error_from_db() with the exception still set. This causes a SystemError. In debug builds, an assertion fails. _pysqlite_query_execute() already prints or clears the pending exception before it raises the DB-API exception. Move that code into the new helper clear_callback_error() and use it in both functions.
|
All commit authors signed the Contributor License Agreement. |
The test compares the printed traceback with plain text. When colors are enabled, the traceback contains escape codes and the comparison fails. Use force_not_colorized to make the output plain.
A collation exception also stays set when the step does not fail. Then pysqlite_cursor_iternext() and executescript() return a result with an exception set. Now the collation exception propagates on these paths. executescript() stops after the statement that caused the exception. It also reports the pending exception when a step fails. Report the pending exception with PyErr_FormatUnraisable() instead of PyErr_Print(). The documentation of enable_callback_tracebacks() says this, and the other callbacks do this. Move the tests into the new class CollationErrorTests and use with_tracebacks().
The failed collation returns a wrong result to SQLite. Thus, the collation exception is the cause of any error later in the step. After each step, check PyErr_Occurred() first and propagate the exception. This also prevents execute() and executemany() from calling converters, adapters and parameter iterators while the exception is set. Other callbacks can run in the same step after the collation raised. Do not call Python code from them while the exception is set. Remove the code that printed or cleared the exception.
The authorizer can run during a step, because table-valued pragma functions prepare statements. inverse() can run after the collation raised, for example when SQLite adds a row to an index that uses the collation. Do not call Python code from these callbacks while an exception is set. final_callback() calls finalize() with the earlier exception set aside. If finalize() raised, the earlier exception was lost. Now the module sets it again, so that it propagates. Make the comments generic, because a callback other than a collation can also leave an exception set. Make the tests check the order of the calls, and add tests for the authorizer, inverse() and finalize().
With STAT4, the query planner can call a collation when SQLite prepares a statement. Check PyErr_Occurred() after sqlite3_prepare_v2() and propagate the exception. Add a control to the authorizer test, so that it shows that SQLite calls the authorizer during the step.
imarturm
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A collation callback cannot report an error to SQLite. Thus, if the callback raises,
collation_callback()keeps the exception set and returns 0 ("equal"). SQLite then continues the step with this wrong result. Some code does not expect the exception:pysqlite_cursor_iternext()(fetch methods and iteration) andexecutescript()callset_error_from_db()with the exception set if the step fails. If the step does not fail, they return a result with the exception set.execute()andexecutemany()call converters, adapters and parameter iterators with the exception set if the step does not fail.final_callback()loses the exception iffinalize()raises.pysqlite_statement_create()then returns a statement with the exception set.In release builds, this causes a
SystemError, or a result without an exception. In debug builds, an assertion fails.This change does these things:
Modules/_sqlite/cursor.c, the module checksPyErr_Occurred()first and propagates the exception. The wrong result of the failed collation is the cause of any error later in the step, for example theinteger overflowin the issue. Thus, the module raises the collation exception, and notOperationalError, also when the step fails. Before,execute()raisedOperationalErrorin this case, and it printed or cleared the collation exception.Modules/_sqlite/connection.c, each callback that calls Python code returns immediately while an exception is set. The SQL function fails, the authorizer denies, or the progress handler stops the step, so SQLite stops the step soon.final_callback()still callsfinalize()with the exception set aside, to release the aggregate. Now it sets the exception again also whenfinalize()raises.pysqlite_statement_create()checksPyErr_Occurred()aftersqlite3_prepare_v2().The new class
CollationErrorTestshas a test for each path. The tests check the order of the calls. For each of the 12 changed locations, I removed the change and made sure that at least one test fails. The STAT4 test runs only if SQLite hasSQLITE_ENABLE_STAT4.test_sqlite3passes with-R 3:3,-X dev,-r,FORCE_COLOR=1,PYTHON_LAZY_IMPORTS=all, and on a release build with AddressSanitizer. I tested with SQLite 3.54.0 (without STAT4) and 3.53.4 (with STAT4).Two questions that this PR does not change:
UPDATEor aDELETEcan change all rows, because each comparison is "equal", and thenexecute()raises. This is also the behavior on main. A fix could callsqlite3_interrupt()when the collation raises, but that stops all statements of the connection. Is this change wanted, maybe in a different issue?step_callback()leaves the exception set if it cannot build the arguments, for example for text that is not valid UTF-8. Now this exception propagates. A scalar function raisesOperationalErrorin this case. I can make this consistent in a different PR.