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

Jump to bottom

sqlite3 regression: bound values are not expanded in traced statements #89301

Closed
erlend-aasland opened this issue Sep 8, 2021 · 6 comments · Fixed by #92053
Closed

sqlite3 regression: bound values are not expanded in traced statements #89301

erlend-aasland opened this issue Sep 8, 2021 · 6 comments · Fixed by #92053
Assignees
@erlend-aasland
Labels
3.10 only security fixes 3.11 only security fixes extension-modules C modules in the Modules dir topic-sqlite3 type-bug An unexpected behavior, bug, or error

Comments

@erlend-aasland
Copy link
Contributor

BPO 45138
Nosy @JelleZijlstra, @miss-islington, @erlend-aasland
PRs
  • bpo-45138: Expand traced SQL statements in sqlite3 trace callback #28240
  • bpo-45138: Fix regression in GH-28240 #31783
  • bpo-45138: Revert GH-28240: Expand traced SQL statements #31788
    		•

    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-09-08.14:05:14.995>
labels = ['extension-modules', 'type-feature', '3.11']
title = '[sqlite3] expand bound values in traced statements when possible'
updated_at = <Date 2022-03-09.17:40:07.234>
user = 'https://github.com/erlend-aasland'

    bugs.python.org fields:

    activity = <Date 2022-03-09.17:40:07.234>
actor = 'miss-islington'
assignee = 'erlendaasland'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2021-09-08.14:05:14.995>
creator = 'erlendaasland'
dependencies = []
files = []
hgrepos = []
issue_num = 45138
keywords = ['patch']
message_count = 6.0
messages = ['401383', '414784', '414785', '414787', '414793', '414806']
nosy_count = 3.0
nosy_names = ['JelleZijlstra', 'miss-islington', 'erlendaasland']
pr_nums = ['28240', '31783', '31788']
priority = 'low'
resolution = 'fixed'
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue45138'
versions = ['Python 3.11']
    @erlend-aasland
    Copy link
    Contributor Author

    For SQLite 3.14.0 and newer, we're using the v2 trace API. This means that the trace callback receives a pointer to the sqlite3_stmt object. We can use the sqlite3_stmt pointer to retrieve expanded SQL string.

    The following statement...:
    cur.executemany("insert into t values(?)", ((v,) for v in range(3)))

    ...will produce the following traces:
    insert into t values(0)
    insert into t values(1)
    insert into t values(2)

    ...instead of:
    insert into t values(?)
    insert into t values(?)
    insert into t values(?)

    @erlend-aasland erlend-aasland added the 3.11 only security fixes label Sep 8, 2021
    @erlend-aasland erlend-aasland self-assigned this Sep 8, 2021
    @erlend-aasland erlend-aasland added extension-modules C modules in the Modules dir type-feature A feature request or enhancement 3.11 only security fixes labels Sep 8, 2021
    @erlend-aasland erlend-aasland self-assigned this Sep 8, 2021
    @erlend-aasland erlend-aasland added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Sep 8, 2021
    @erlend-aasland erlend-aasland changed the title [sqlite3] expand bound values in traced statements if possible [sqlite3] expand bound values in traced statements when possible Sep 8, 2021
    @erlend-aasland erlend-aasland changed the title [sqlite3] expand bound values in traced statements if possible [sqlite3] expand bound values in traced statements when possible Sep 8, 2021
    @JelleZijlstra
    Copy link
    Member

    New changeset d177751 by Erlend Egeberg Aasland in branch 'main':
    bpo-45138: Expand traced SQL statements in sqlite3 trace callback (GH-28240)
    d177751

    @JelleZijlstra
    Copy link
    Member

    Thanks for the contribution!

    @erlend-aasland
    Copy link
    Contributor Author

    Reopening bco. broken buildbots.

    @erlend-aasland erlend-aasland reopened this Mar 9, 2022
    @erlend-aasland erlend-aasland reopened this Mar 9, 2022
    @erlend-aasland
    Copy link
    Contributor Author

    erlend-aasland commented Mar 9, 2022 •
    edited

    Ah, one of my very first contributions, gh-84498, comes back to haunt me:

    In gh-84498, we migrated from the old SQLite trace API (sqlite3_trace) to SQLite trace v2 API (sqlite3_trace_v2). #19581, which introduced this change, introduced a bug: the old trace API _implicitly_ expanded bound parameters; the new trace API does not. However, there was no tests for this behaviour, so the regression was unnoticed1.

    So, this bpo is actually a bug fix; not a feature. It should be backported to 3.10, which contains the regression.

    I'm preparing a fix for #28240, and I'll prepare a 3.10 backport including both #19581 and its upcoming fix.

    Footnotes

    1. There has been no bug reports regarding this change in behaviour, so it seems to have gone under most people's radar. ↩

    @miss-islington
    Copy link
    Contributor

    New changeset e801e88 by Erlend Egeberg Aasland in branch 'main':
    bpo-45138: Revert #72427: Expand traced SQL statements (GH-31788)
    e801e88

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @erlend-aasland erlend-aasland added type-bug An unexpected behavior, bug, or error 3.10 only security fixes and removed type-feature A feature request or enhancement labels Apr 10, 2022
    @erlend-aasland erlend-aasland changed the title [sqlite3] expand bound values in traced statements when possible sqlite3 regression: bound values are not expanded in traced statements Apr 29, 2022
    erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this issue Apr 29, 2022
    @erlend-aasland
    pythongh-89301: Fix regression with bound values in traced statements
    ab35f4b
    @erlend-aasland erlend-aasland mentioned this issue Apr 29, 2022
    Merged
    JelleZijlstra pushed a commit that referenced this issue May 2, 2022
    @erlend-aasland
    gh-89301: Fix regression with bound values in traced SQLite statements (
    721aa96 
    #92053)
    erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue May 2, 2022
    @erlend-aasland
    [3.10] pythongh-89301: Fix regression with bound values in traced SQL…
    ae48271 
    …ite statements

(cherry picked from commit 721aa96)
    JelleZijlstra pushed a commit that referenced this issue May 2, 2022
    @erlend-aasland
    [3.10] gh-89301: Fix regression with bound values in traced SQLite st…
    178d79a 
    …atements (#92147)

(cherry picked from commit 721aa96)
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @erlend-aasland erlend-aasland

    Labels
    3.10 only security fixes 3.11 only security fixes extension-modules C modules in the Modules dir topic-sqlite3 type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging a pull request may close this issue.

    gh-89301: Fix regression with bound values in traced SQLite statements erlend-aasland/cpython
    3 participants
    @JelleZijlstra @erlend-aasland @miss-islington