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
commented
Sep 8, 2021
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
I'll resolve the conflicting files after GH-29327 has been merged. Fixed.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Note to self: after GH-29356 has landed, add test for too long SQL statements. Done
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
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|}
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I believe I've addressed everything here, @pablogsal. Let me know if you want further changes.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I'm aware, and I'm coming up with a fix for that test.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Apr 10, 2022
Reviewers
pablogsal
Awaiting requested review from pablogsal
Uh oh!
There was an error while loading. Please reload this page.
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.
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.
|