gh-89121: Keep the number of pending SQLite statements to a minimum by erlend-aasland · Pull Request #30379 · python/cpython (original) (raw)

All sqlite tests passed. Looks ok.

Thank for you interest in improving CPython, by reviewing PRs! Could you also review that all statement "exit paths" are covered? This is the important part of the PR.

A statement should be reset if:

• it is replaced by another statement in the cursor (for example: cur.execute("select 1"); cur.execute("select 2"))
• it is removed from a cursor because the cursor is closed
• it is removed from a cursor because the cursor is deallocated
• it is completely stepped through (sqlite3_step returned SQLITE_DONE)
• stepping through the statement returned an error (sqlite3_step returned something else than SQLITE_ROW and SQLITE_DONE)
• a Python API call returned an error while processing the statement (typically PyErr_Occurred() is true during statement execution)

Did I forget an exit path?

Relevant C code to check:

• Modules/_sqlite/cursor.c: all paths taken in _pysqlite_query_execute
• Modules/_sqlite/cursor.c: all paths taken in pysqlite_cursor_iternext
• Cursor deallocate
• Cursor close
• Statement deallocate

Also, we need to triple check all these:

$ grep -rn reset Modules/_sqlite Modules/_sqlite/connection.h:71: * reset to 0 at certain intervals / Modules/_sqlite/statement.h:47:int pysqlite_statement_reset(pysqlite_Statement self); Modules/_sqlite/cursor.c:105: pysqlite_statement_reset(self->statement); Modules/_sqlite/cursor.c:511: /* reset description and rowcount / Modules/_sqlite/cursor.c:518: (void)pysqlite_statement_reset(self->statement); Modules/_sqlite/cursor.c:542: pysqlite_statement_reset(self->statement); Modules/_sqlite/cursor.c:628: (void)pysqlite_statement_reset(self->statement); Modules/_sqlite/cursor.c:653: (void)pysqlite_statement_reset(self->statement); Modules/_sqlite/cursor.c:797: (void)pysqlite_statement_reset(self->statement); Modules/_sqlite/cursor.c:969: (void)pysqlite_statement_reset(self->statement); Modules/_sqlite/statement.c:369:int pysqlite_statement_reset(pysqlite_Statement self) Modules/_sqlite/statement.c:377: rc = sqlite3_reset(self->st); $ grep -rn "Py_CLEAR.*statement" Modules/_sqlite Modules/_sqlite/connection.c:325: Py_CLEAR(self->statement_cache); Modules/_sqlite/connection.c:540: Py_CLEAR(self->statement_cache); Modules/_sqlite/cursor.c:53: Py_CLEAR(self->statement); Modules/_sqlite/cursor.c:106: Py_CLEAR(self->statement); Modules/_sqlite/cursor.c:654: Py_CLEAR(self->statement); Modules/_sqlite/cursor.c:660: Py_CLEAR(self->statement); Modules/_sqlite/cursor.c:798: Py_CLEAR(self->statement); Modules/_sqlite/cursor.c:970: Py_CLEAR(self->statement);

IOW, none of the statements in the LRU cache that do not belong to an active cursor should be pending statements; they should all be reset statements. Correct me if I'm wrong.

Note; statement execution includes all the C interfaces to the execute* methods, and also all the C interfaces to the fetch* (and iter) methods.