[issue43290] [sqlite3] remove legacy code from pysqlite_step

Change by Berker Peksag : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Berker Peksag added the comment: New changeset 91ea37c84af2dd5ea92802a4c2adad47861ac067 by Erlend Egeberg Aasland in branch 'master': bpo-43290: Remove workaround from pysqlite_step() (GH-24638) https://github.com/python/cpython/commit/91ea37c84af2dd5ea92802a4c2adad47861ac067 --

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: Addendum to msg387641: > The latter only leaves a valid Cursor->statement->st pointer (sqlite3_stmt > pointer) if the Statement object was successfully created, and the > sqlite3_stmt successfully prepared. sqlite3_prepare_v2() actually returns

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: msg387668 was a little bit hasty. I'll try again: Dong-hee Na: > Hmm by the way the current implementation returns SQLITE_OK if the statement > is NULL, but it looks like return SQLITE_MISUSE if we apply this patch. > Does it not cause any behavior

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: > int rc = sqlite3_reset(NULL); >printf("reset with NULL: %d %s\n", rc, sqlite3_errstr(rc)); Sorry, wrong test: int rc = sqlite3_step(NULL); printf("step with NULL: %d %s\n", rc, sqlite3_errstr(rc)); $ ./a.out step with NULL: 21 bad

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: Ah, at last I found the source of the confusion: The SQLite changelog. Quoting from msg387619 and https://sqlite.org/changes.html: > From the SQLite 3.5.3 changelog: > - sqlite3_step() returns SQLITE_MISUSE instead of crashing when called with a >

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Christian Heimes added the comment: Back in the day I was of several core devs that took care of syncing code between Python 2 and 3 branches with a tool called "svnmerge". Commit 380532117c2547bb0dedf6f85efa66d18a9abb88 is a svnmerge commit. The tool synced changesets in batches, which

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: Introduced by Christian Heimes 2007-12-14 in commit 380532117c2547bb0dedf6f85efa66d18a9abb88, which is a merge from SVN (?) checkin r59471 by Gerhard Häring (https://mail.python.org/pipermail/python-checkins/2007-December/063968.html) This

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: > Look at issue in which the workaround was added. Does it contain some > examples? I'll check. Thanks. -- ___ Python tracker ___

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Serhiy Storchaka added the comment: Look at issue in which the workaround was added. Does it contain some examples? -- ___ Python tracker ___

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: > I wonder if it is possible at all to reach this branch. If it is not, then > I'm pretty sure Cursor.in_use is redundant Typo: should be Statement.in_use Corner error cases may cause the _pysqlite_query_execute() loop to exit without

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: > _pysqlite_query_execute() has one coverage gap: lines 478-488 (if > (self->statement->in_use)) are never executed. I wonder if it is possible at all to reach this branch. If it is not, then I'm pretty sure Cursor.in_use is redundant, and all code

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: Regarding test coverage: _pysqlite_query_execute() has one coverage gap: lines 478-488 (if (self->statement->in_use)) are never executed. Except from that, the only missing are the hard-to-trigger goto error's (for example PyList_New and

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: pysqlite_cursor_iternext() has four users: - sqlite3.Cursor.fetchone() - sqlite3.Cursor.fetchall() - sqlite3.Cursor.fetchmany() - sqlite3.Cursor.__next__() All of these methods pass self to pysqlite_cursor_iternext(). pysqlite_cursor_iternext() starts

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: There are six users of pysqlite_step(): $ grep -nrE "\" Modules/_sqlite Modules/_sqlite/util.c:27:int pysqlite_step(sqlite3_stmt* statement, pysqlite_Connection* connection) Modules/_sqlite/connection.c:393:rc = pysqlite_step(statement, self);

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: I’ll check all uses and see if we’ve got everything covered by the test suite. This is one of the core functions of the sqlite3 module (all queries call step at least once, but often multiple times), so I’d expect coverage is pretty good. --

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Dong-hee Na added the comment: Hmm by the way the current implementation returns SQLITE_OK if the statement is NULL, but it looks like return SQLITE_MISUSE if we apply this patch. Does it not cause any behavior regression? if so we should add news also. -- nosy: +corona10

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23423 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24638 ___ Python tracker

[issue43290] [sqlite3] remove legacy code from pysqlite_step

Erlend Egeberg Aasland added the comment: >From the SQLite 3.5.3 changelog: - sqlite3_step() returns SQLITE_MISUSE instead of crashing when called with a NULL parameter. -- ___ Python tracker

[issue43290] [sqlite3] remove legacy code from pysqlite_step

New submission from Erlend Egeberg Aasland : pysqlite_step() contains a NULL check needed for SQLite 3.5 and earlier. This can be removed. -- components: Library (Lib) messages: 387476 nosy: berker.peksag, erlendaasland, serhiy.storchaka priority: normal severity: normal status: open