Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sqlite3] drop statement in_use field in favour of sqlite3_stmt_busy() #88239

Closed
erlend-aasland opened this issue May 8, 2021 · 3 comments · Fixed by #25984
Closed

[sqlite3] drop statement in_use field in favour of sqlite3_stmt_busy() #88239

erlend-aasland opened this issue May 8, 2021 · 3 comments · Fixed by #25984
Assignees
Labels
3.11 only security fixes easy extension-modules C modules in the Modules dir topic-sqlite3 type-feature A feature request or enhancement

Comments

@erlend-aasland
Copy link
Contributor

BPO 44073
Nosy @berkerpeksag, @serhiy-storchaka, @erlend-aasland
PRs
  • gh-88239: Use sqlite3_stmt_busy() to determine if statements are in use #25984
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/erlend-aasland'
    closed_at = None
    created_at = <Date 2021-05-08.07:47:59.789>
    labels = ['extension-modules', 'easy', 'type-feature', '3.11']
    title = '[sqlite3] drop statement in_use field in favour of sqlite3_stmt_busy()'
    updated_at = <Date 2021-08-19.20:10:44.154>
    user = 'https://github.com/erlend-aasland'

    bugs.python.org fields:

    activity = <Date 2021-08-19.20:10:44.154>
    actor = 'erlendaasland'
    assignee = 'erlendaasland'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules']
    creation = <Date 2021-05-08.07:47:59.789>
    creator = 'erlendaasland'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 44073
    keywords = ['patch', 'easy (C)']
    message_count = 3.0
    messages = ['393241', '393243', '399932']
    nosy_count = 3.0
    nosy_names = ['berker.peksag', 'serhiy.storchaka', 'erlendaasland']
    pr_nums = ['25984']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue44073'
    versions = ['Python 3.11']

    @erlend-aasland
    Copy link
    Contributor Author

    sqlite3_stmt_busy() has been around since SQLite 3.7.10. I suggest to drop the in_use field of pysqlite_Statement in favour of sqlite3_stmt_busy(); we do not need to duplicate functionality already present in SQLite.

    There was a bugfix for sqlite3_stmt_busy() in SQLite 3.8.6 regarding rollback statements, but we normally reset all our statements after use, so it should not be a problem. There are some corner cases in _pysqlite_query_execute() where a sqlite3_stmt may not be reset upon return, but that's easily fixable (and it would be a nice side-effect).

    Pro's:

    • statement objects have one less member
    • no duplication of SQLite functionality
    • cleaner exit paths from _pysqlite_query_execute()
    • less lines of code, easier to maintain

    Con's:

    • the current code works / "code churn"

    @erlend-aasland erlend-aasland added the 3.11 only security fixes label May 8, 2021
    @erlend-aasland erlend-aasland self-assigned this May 8, 2021
    @erlend-aasland erlend-aasland added extension-modules C modules in the Modules dir easy type-feature A feature request or enhancement 3.11 only security fixes labels May 8, 2021
    @erlend-aasland erlend-aasland self-assigned this May 8, 2021
    @erlend-aasland erlend-aasland added extension-modules C modules in the Modules dir easy type-feature A feature request or enhancement labels May 8, 2021
    @erlend-aasland
    Copy link
    Contributor Author

    $ git diff --stat
     Modules/_sqlite/connection.c |  1 -
     Modules/_sqlite/cursor.c     |  5 +

    Modules/_sqlite/statement.c | 14 +-------------
    Modules/_sqlite/statement.h | 2 --
    4 files changed, 2 insertions(+), 20 deletions(-)

    @erlend-aasland
    Copy link
    Contributor Author

    erlend-aasland commented Aug 19, 2021

    See also gh-89121 (bpo-44958)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes easy extension-modules C modules in the Modules dir topic-sqlite3 type-feature A feature request or enhancement
    Projects
    Status: Done
    Development

    Successfully merging a pull request may close this issue.

    1 participant