GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-45138: Expand traced SQL statements in `sqlite3` trace callback by erlend-aasland · Pull Request #28240 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Sep 8, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue45138

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Erlend E. Aasland added 3 commits September 8, 2021 16:06
Erlend E. Aasland
Use expanded SQL when tracing using v2 API
a018384
Erlend E. Aasland
Fall back to unexpanded sql if expanded sql is too long
0d59fd9
Erlend E. Aasland
Remove unneeded indent
654b89f
@erlend-aasland
erlend-aasland requested a review from berkerpeksag as a code owner September 8, 2021 14:19
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Sep 8, 2021
@bedevere-bot bedevere-bot added the awaiting review label Sep 8, 2021
Erlend E. Aasland added 2 commits September 8, 2021 16:21
Erlend E. Aasland
Add NEWS
d7dcbd4
Erlend E. Aasland
Revert spurious print traceback change
bc71d99
@erlend-aasland
erlend-aasland requested a review from pablogsal September 10, 2021 07:29
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
e22f58d
@erlend-aasland erlend-aasland closed this Sep 14, 2021
@erlend-aasland erlend-aasland reopened this Sep 14, 2021
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
d603841
@erlend-aasland erlend-aasland closed this Sep 19, 2021
@erlend-aasland erlend-aasland reopened this Sep 19, 2021
pablogsal
pablogsal reviewed Sep 19, 2021
View reviewed changes
Comment thread Modules/_sqlite/connection.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Erlend E. Aasland added 3 commits October 6, 2021 22:53
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
b3c7cd7
Erlend E. Aasland
Chain exceptions if multiple exceptions happen
b91fc81
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
3fa1dfa
@erlend-aasland

erlend-aasland commented Nov 1, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I'll resolve the conflicting files after GH-29327 has been merged. Fixed.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
bc1ac3a
@erlend-aasland

erlend-aasland commented Nov 2, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Note to self: after GH-29356 has landed, add test for too long SQL statements. Done

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Erlend E. Aasland added 5 commits November 2, 2021 20:23
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
5683a9a
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
7e8d2be
Erlend E. Aasland
Improve coverage
ec19cf7
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
b3940df
Erlend E. Aasland
Expand test
f773a85
@erlend-aasland

erlend-aasland commented Nov 10, 2021

Copy link
Copy Markdown
Contributor Author

Coverage for trace_callback() is now pretty good.

Details
 1056|       |static int
 1057|       |trace_callback(unsigned int type, void *ctx, void *stmt, void *sql)
 1058|       |#else
 1059|       |static void
 1060|       |trace_callback(void *ctx, const char *sql)
 1061|       |#endif
 1062|     20|{
 1063|     20|#ifdef HAVE_TRACE_V2
 1064|     20|    if (type != SQLITE_TRACE_STMT) {
 1065|      0|        return 0;
 1066|      0|    }
 1067|     20|#endif
 1068|       |
 1069|     20|    PyGILState_STATE gilstate = PyGILState_Ensure();
 1070|       |
 1071|     20|    assert(ctx != NULL);
 1072|     20|    PyObject *py_statement = NULL;
 1073|     20|#ifdef HAVE_TRACE_V2
 1074|     20|    assert(stmt != NULL);
 1075|     20|    const char *expanded_sql = sqlite3_expanded_sql((sqlite3_stmt *)stmt);
 1076|     20|    if (expanded_sql == NULL) {
 1077|      2|        sqlite3 *db = sqlite3_db_handle((sqlite3_stmt *)stmt);
 1078|      2|        if (sqlite3_errcode(db) == SQLITE_NOMEM) {
 1079|      0|            (void)PyErr_NoMemory();
 1080|      0|            goto exit;
 1081|      0|        }
 1082|       |
 1083|      2|        pysqlite_state *state = ((callback_context *)ctx)->state;
 1084|      2|        assert(state != NULL);
 1085|      2|        PyErr_SetString(state->DataError,
 1086|      2|                        "Expanded SQL string exceeds the maximum string "
 1087|      2|                        "length");
 1088|       |
 1089|       |        // Fall back to unexpanded sql
 1090|      2|        py_statement = PyUnicode_FromString((const char *)sql);
 1091|      2|    }
 1092|     18|    else {
 1093|     18|        py_statement = PyUnicode_FromString(expanded_sql);
 1094|     18|        sqlite3_free((void *)expanded_sql);
 1095|     18|    }
 1096|       |#else
 1097|       |    py_statement = PyUnicode_FromString(sql);
 1098|       |#endif
 1099|     20|    if (py_statement) {
 1100|     20|        PyObject *exc, *val, *tb;
 1101|     20|        PyErr_Fetch(&exc, &val, &tb);
 1102|       |
 1103|     20|        PyObject *callable = ((callback_context *)ctx)->callable;
 1104|     20|        PyObject *ret = PyObject_CallOneArg(callable, py_statement);
 1105|     20|        Py_DECREF(py_statement);
 1106|     20|        Py_XDECREF(ret);
 1107|       |
 1108|     20|        _PyErr_ChainExceptions(exc, val, tb);
 1109|     20|    }
 1110|       |
 1111|     20|    if (PyErr_Occurred()) {
 1112|      4|        print_or_clear_traceback((callback_context *)ctx);
 1113|      4|    }
 1114|       |
 1115|     20|exit:
 1116|     20|    PyGILState_Release(gilstate);
 1117|     20|#ifdef HAVE_TRACE_V2
 1118|     20|    return 0;
 1119|     20|#endif
 1120|     20|}
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland
erlend-aasland requested a review from pablogsal November 10, 2021 20:08
Erlend E. Aasland
We can't rely on sqlite_schema being present
a83a945
@erlend-aasland

erlend-aasland commented Jan 4, 2022

Copy link
Copy Markdown
Contributor Author

I believe I've addressed everything here, @pablogsal. Let me know if you want further changes.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Erlend E. Aasland added 2 commits January 18, 2022 23:20
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
a1c1424
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
6ea2605
@erlend-aasland
erlend-aasland requested a review from corona10 January 20, 2022 12:53
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
24ef8a9
@erlend-aasland
erlend-aasland requested a review from pablogsal February 26, 2022 21:51
Erlend E. Aasland
Merge branch 'main' into sqlite-bound-trace
4d9c9ec
@erlend-aasland
erlend-aasland requested review from JelleZijlstra and removed request for corona10 March 2, 2022 08:30
@JelleZijlstra JelleZijlstra self-assigned this Mar 2, 2022
JelleZijlstra
JelleZijlstra reviewed Mar 3, 2022
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, a few small comments

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/test/test_sqlite3/test_hooks.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_sqlite3/test_hooks.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_sqlite3/test_hooks.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_sqlite/connection.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Erlend Egeberg Aasland and others added 3 commits March 3, 2022 09:27
Erlend Egeberg Aasland @JelleZijlstra
Update Lib/test/test_sqlite3/test_hooks.py
beff1d5
Co-authored-by: Jelle Zijlstra <[email protected]>
Erlend Egeberg Aasland @JelleZijlstra
PEP 8
9920131
Co-authored-by: Jelle Zijlstra <[email protected]>
Erlend E. Aasland
Address review: clean up namespace
f086604
JelleZijlstra
JelleZijlstra approved these changes Mar 8, 2022
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! @gvanrossum I'm planning to merge this change, another SQLite enhancement by Erlend.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gvanrossum reacted with thumbs up emoji erlend-aasland reacted with hooray emoji
All reactions
  • 👍 1 reaction
  • 🎉 1 reaction
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Mar 8, 2022
@JelleZijlstra
JelleZijlstra merged commit d177751 into python:main Mar 9, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 9, 2022
@erlend-aasland

erlend-aasland commented Mar 9, 2022

Copy link
Copy Markdown
Contributor Author

Thanks, @JelleZijlstra!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303

kumaraditya303 commented Mar 9, 2022

Copy link
Copy Markdown
Contributor

This change broke several buildbots See https://buildbot.python.org/all/#/builders/58/builds/1850

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland
erlend-aasland deleted the sqlite-bound-trace branch March 9, 2022 08:22
@erlend-aasland

erlend-aasland commented Mar 9, 2022

Copy link
Copy Markdown
Contributor Author

I'm aware, and I'm coming up with a fix for that test.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

asvetlov pushed a commit that referenced this pull request Mar 9, 2022
Erlend Egeberg Aasland @asvetlov
bpo-45138: Expand traced SQL statements in sqlite3 trace callback (G…
57e428a
…H-28240)
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Mar 9, 2022
Erlend E. Aasland
bpo-45138: Revert pythonGH-28240: Expand traced SQL statements
53e0fb4
This reverts commit d177751.
miss-islington pushed a commit that referenced this pull request Mar 9, 2022
Erlend Egeberg Aasland
bpo-45138: Revert GH-28240: Expand traced SQL statements (GH-31788)
e801e88
This reverts commit d177751.

Automerge-Triggered-By: GH:JelleZijlstra
This was referenced Apr 10, 2022
sqlite3 regression: bound values are not expanded in traced statements #89301
Closed
gh-89301: Fix regression with bound values in traced SQLite statements #92053
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag
@pablogsal pablogsal Awaiting requested review from pablogsal

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@JelleZijlstra JelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@erlend-aasland @kumaraditya303 @JelleZijlstra @pablogsal @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.