Skip to content

list values incorrectly adapted in SQL panel #1831

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

Closed
tkoschnick opened this issue Sep 14, 2023 · 3 comments · Fixed by #1832
Closed

list values incorrectly adapted in SQL panel #1831

tkoschnick opened this issue Sep 14, 2023 · 3 comments · Fixed by #1832

Comments

@tkoschnick
Copy link
Contributor

I have a sql query containing ... WHERE id = ANY(%s) and the param being a list, e.g. [1, 2].

Adapted, the query looks like id = ANY('{1,2}'::int2[]) , but in the SQL panel, it shows up as id = ANY('[1,2]'). This is due to the parameters being quoted already before being passed to mogrify. Dropping the extra quoting step, the query shows up correctly in the SQL panel.

diff --git a/debug_toolbar/panels/sql/tracking.py b/debug_toolbar/panels/sql/tracking.py
index 0c53dc2c..bcff5905 100644
--- a/debug_toolbar/panels/sql/tracking.py
+++ b/debug_toolbar/panels/sql/tracking.py
@@ -115,15 +115,6 @@ class NormalCursorMixin(DjDTCursorWrapperMixin):
         else:
             return repr(element)
 
-    def _quote_params(self, params):
-        if not params:
-            return params
-        if isinstance(params, dict):
-            return {key: self._quote_expr(value) for key, value in params.items()}
-        if isinstance(params, tuple):
-            return tuple(self._quote_expr(p) for p in params)
-        return [self._quote_expr(p) for p in params]
-
     def _decode(self, param):
         if PostgresJson and isinstance(param, PostgresJson):
             # psycopg3
@@ -157,9 +148,7 @@ class NormalCursorMixin(DjDTCursorWrapperMixin):
         # process during the .last_executed_query() call.
         self.db._djdt_logger = None
         try:
-            return self.db.ops.last_executed_query(
-                self.cursor, sql, self._quote_params(params)
-            )
+            return self.db.ops.last_executed_query(self.cursor, sql, params)
         finally:
             self.db._djdt_logger = self.logger
 

Note that I've tested only with psycopg2/3, I don't know if other backends require the extra quoting.

tracking.txt

@tkoschnick
Copy link
Contributor Author

Patch adjusted, drop _quote_expr() as well:

tracking.txt

@tim-schilling
Copy link
Member

@tkoschnick can you create a PR please?

I'm pretty skeptical we can drop that quoting logic though. I'm curious how that change handles the test suite.

@tkoschnick
Copy link
Contributor Author

I've investigated a bit further. The current quoting uses the Python representation, which happens to work for simple data types like ints and strings. For anything more complex, only the database driver's mogrify function is able to provide the correct adaptation. Take a more involved example, Postgres' range type. Currently, it's displayed as follows in the SQL panel:

... WHERE "log_logentry"."block" && 'Range(datetime.datetime(2022, 1, 1, 0, 0, tzinfo=zoneinfo.ZoneInfo(key=''UTC'')), datetime.datetime(2023, 1, 1, 0, 0, tzinfo=zoneinfo.ZoneInfo(key=''UTC'')), ''[)'')')

correct as produced with the patch applied:

... WHERE "log_logentry"."block" && '["2022-01-01 00:00:00+00:00","2023-01-01 00:00:00+00:00")'::tstzrange)

With the original example, list adaptation, the correct way depends on the database driver in use. Not patched:

... WHERE log_logentry.id = ANY('[1895]')

psycopg3:

... WHERE log_logentry.id = ANY('{1895}'::int2[])

psycopg2:

... WHERE log_logentry.id = ANY(ARRAY[1895])

I don't think there is a way to get this right without just passing it to mogrify.

Tests all pass with the patch applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants