classification
Title: [sqlite3] Active statements are reset twice in _pysqlite_query_execute()
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, erlendaasland, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2021-03-01 09:52 by erlendaasland, last changed 2021-03-01 13:35 by erlendaasland.

Pull Requests
URL Status Linked Edit
PR 24681 open erlendaasland, 2021-03-01 09:54
Messages (4)
msg387850 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-03-01 09:52
In _pysqlite_query_execute(), if the cursor already has a statement object, it is reset twice before the cache is queried.
msg387858 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-03-01 12:15
There are three calls of pysqlite_statement_reset() in _pysqlite_query_execute(). And all three of them can be called with the same statement object. But repeated calls are no-ops because pysqlite_statement_reset() clears flag in_use and calls sqlite3_reset() only if it was set before. So additional call of pysqlite_statement_reset() does not harm. You can call it as many times as you want. On other hand removing it can introduce race condition.

Maybe the code could be rewritten in more explicit way and call pysqlite_statement_reset() only when it is necessary, but for now I don't think that just removing one call will make the code better.
msg387859 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-03-01 12:38
> There are three calls of pysqlite_statement_reset() in _pysqlite_query_execute()

Yes, but only two before the cache is queried.

> So additional call of pysqlite_statement_reset() does not harm.

That's true, but removing redundant code makes it easier to read and maintain, IMO.

> On other hand removing it can introduce race condition.

How?

> Maybe the code could be rewritten in more explicit way and call pysqlite_statement_reset() only when it is necessary

Most of _pysqlite_query_execute() could be rewritten in order to make the code clearer. An alternative is to clean it up part by part.
msg387865 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-03-01 13:35
Hm, I guess we could use sqlite3_stmt_busy() instead of the in_use flag.
History
Date User Action Args
2021-03-01 13:35:28erlendaaslandsetmessages: + msg387865
2021-03-01 12:38:31erlendaaslandsetmessages: + msg387859
2021-03-01 12:15:06serhiy.storchakasetmessages: + msg387858
2021-03-01 09:54:16erlendaaslandsetkeywords: + patch
stage: patch review
pull_requests: + pull_request23466
2021-03-01 09:52:49erlendaaslandcreate