-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix executemany() raising from the SQL panel #2437
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
_record() passed the param list straight to _last_executed_query(), but the backend's last_executed_query() expects a flat sequence of scalars. On sqlite it re-quotes them with QUOTE(?) placeholders, so a sequence of param sequences raises ProgrammingError. It happens in a finally: block after the statement has already run, so the write lands and the request still 500s. Thread a many flag through _record() and render the statement as 'N times: <sql>' in that case, which is what Django's own CursorDebugWrapper does. The select/explain/profile buttons re-execute raw_sql with params through plain cursor.execute, which fails the same way, so they are hidden for executemany queries.
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,9 @@ Pending | |
| * Stopped the history panel buttons from submitting their form when clicked | ||
| before the panel script has loaded, which navigated away from the page. | ||
| * Added support for Django 6.1. | ||
| * Fixed ``cursor.executemany()`` raising from the SQL panel instead of being | ||
| recorded. The statement is now shown as ``N times: <sql>``, matching | ||
| Django's own debug cursor. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has to be moved up to the section covering the next release now.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to Pending in |
||
|
|
||
| 7.0.0 (2026-06-17) | ||
| ------------------ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,6 +104,51 @@ def test_recording(self): | |
| # ensure the stacktrace is populated | ||
| self.assertTrue(len(query["stacktrace"]) > 0) | ||
|
|
||
| def test_executemany(self): | ||
| """ | ||
| executemany() must be recorded without raising. | ||
|
|
||
| The backend's last_executed_query() expects a flat sequence of scalar | ||
| params, so handing it a list of param sequences fails. It happens in a | ||
| finally: block after the write has already landed, so the exception | ||
| escapes into the caller. | ||
| """ | ||
| self.assertEqual(len(self.panel._queries), 0) | ||
|
|
||
| with connection.cursor() as cursor: | ||
| cursor.executemany( | ||
| "INSERT INTO tests_binary (field) VALUES (%s)", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just asking: Is there a specific reason you chose
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the only backend-agnostic model in |
||
| [(b"one",), (b"two",)], | ||
| ) | ||
|
|
||
| self.assertEqual(len(self.panel._queries), 1) | ||
| query = self.panel._queries[0] | ||
| self.assertEqual( | ||
| query["sql"], "2 times: INSERT INTO tests_binary (field) VALUES (%s)" | ||
| ) | ||
| self.assertTrue(query["many"]) | ||
| self.assertEqual(Binary.objects.count(), 2) | ||
|
|
||
| def test_executemany_with_empty_param_list(self): | ||
| """An empty param list runs no statement but must still not raise.""" | ||
| self.assertEqual(len(self.panel._queries), 0) | ||
|
|
||
| with connection.cursor() as cursor: | ||
| cursor.executemany("INSERT INTO tests_binary (field) VALUES (%s)", []) | ||
|
|
||
| self.assertEqual(len(self.panel._queries), 1) | ||
| self.assertEqual( | ||
| self.panel._queries[0]["sql"], | ||
| "0 times: INSERT INTO tests_binary (field) VALUES (%s)", | ||
| ) | ||
| self.assertEqual(Binary.objects.count(), 0) | ||
|
|
||
| def test_execute_is_not_marked_as_many(self): | ||
| sql_call() | ||
|
|
||
| self.assertEqual(len(self.panel._queries), 1) | ||
| self.assertFalse(self.panel._queries[0]["many"]) | ||
|
|
||
| def test_assert_num_queries_works(self): | ||
| """ | ||
| Confirm Django's assertNumQueries and CaptureQueriesContext works | ||
|
|
||
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.
I think this should probably be internationalized.
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.
Done in
aead842d, usingngettextso the count pluralizes, with a separate string for the?case whenparamshas nolen(). Added a singular test.