From 0166e4d011df94b42198188d9f339d0f52999835 Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Mon, 18 Apr 2022 17:13:15 +0300 Subject: [PATCH 1/6] Fix confusing variable shadowing --- debug_toolbar/panels/sql/tracking.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/debug_toolbar/panels/sql/tracking.py b/debug_toolbar/panels/sql/tracking.py index a85ac51ad..e844d9a7b 100644 --- a/debug_toolbar/panels/sql/tracking.py +++ b/debug_toolbar/panels/sql/tracking.py @@ -182,7 +182,7 @@ def _record(self, method, sql, params): else: sql = str(sql) - params = { + kwargs = { "vendor": vendor, "alias": alias, "sql": self._last_executed_query(sql, params), @@ -228,7 +228,7 @@ def _record(self, method, sql, params): else: trans_id = None - params.update( + kwargs.update( { "trans_id": trans_id, "trans_status": conn.info.transaction_status, @@ -237,7 +237,7 @@ def _record(self, method, sql, params): ) # We keep `sql` to maintain backwards compatibility - self.logger.record(**params) + self.logger.record(**kwargs) def callproc(self, procname, params=None): return self._record(super().callproc, procname, params) From 98b42f0cd869497b51067f7b83a5ab9dbd3f8d62 Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Mon, 18 Apr 2022 17:15:27 +0300 Subject: [PATCH 2/6] Remove unused query attributes --- debug_toolbar/panels/sql/tracking.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/debug_toolbar/panels/sql/tracking.py b/debug_toolbar/panels/sql/tracking.py index e844d9a7b..6ac88c091 100644 --- a/debug_toolbar/panels/sql/tracking.py +++ b/debug_toolbar/panels/sql/tracking.py @@ -191,8 +191,6 @@ def _record(self, method, sql, params): "params": _params, "raw_params": params, "stacktrace": get_stack_trace(skip=2), - "start_time": start_time, - "stop_time": stop_time, "is_slow": ( duration > dt_settings.get_config()["SQL_WARNING_THRESHOLD"] ), From 8ed8124aa75bbd219c1d8e0635dfe3068eabaf25 Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Tue, 19 Apr 2022 14:18:47 +0300 Subject: [PATCH 3/6] Post-process two query attributes There is no reason for them to be computed when recording, so move them to SQLPanel.generate_stats(). --- debug_toolbar/panels/sql/panel.py | 9 +++++++++ debug_toolbar/panels/sql/tracking.py | 5 ----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/debug_toolbar/panels/sql/panel.py b/debug_toolbar/panels/sql/panel.py index c8576e16f..9cdb1003a 100644 --- a/debug_toolbar/panels/sql/panel.py +++ b/debug_toolbar/panels/sql/panel.py @@ -6,6 +6,7 @@ from django.urls import path from django.utils.translation import gettext_lazy as _, ngettext +from debug_toolbar import settings as dt_settings from debug_toolbar.forms import SignedDataForm from debug_toolbar.panels import Panel from debug_toolbar.panels.sql import views @@ -204,6 +205,8 @@ def generate_stats(self, request, response): duplicate_query_groups = defaultdict(list) if self._queries: + sql_warning_threshold = dt_settings.get_config()["SQL_WARNING_THRESHOLD"] + width_ratio_tally = 0 factor = int(256.0 / (len(self._databases) * 2.5)) for n, db in enumerate(self._databases.values()): @@ -261,6 +264,12 @@ def generate_stats(self, request, response): if query["sql"]: query["sql"] = reformat_sql(query["sql"], with_toggle=True) + + query["is_slow"] = query["duration"] > sql_warning_threshold + query["is_select"] = ( + query["raw_sql"].lower().lstrip().startswith("select") + ) + query["rgb_color"] = self._databases[alias]["rgb_color"] try: query["width_ratio"] = (query["duration"] / self._sql_time) * 100 diff --git a/debug_toolbar/panels/sql/tracking.py b/debug_toolbar/panels/sql/tracking.py index 6ac88c091..35f0806f1 100644 --- a/debug_toolbar/panels/sql/tracking.py +++ b/debug_toolbar/panels/sql/tracking.py @@ -7,7 +7,6 @@ from django.db.backends.utils import CursorWrapper from django.utils.encoding import force_str -from debug_toolbar import settings as dt_settings from debug_toolbar.utils import get_stack_trace, get_template_info try: @@ -191,10 +190,6 @@ def _record(self, method, sql, params): "params": _params, "raw_params": params, "stacktrace": get_stack_trace(skip=2), - "is_slow": ( - duration > dt_settings.get_config()["SQL_WARNING_THRESHOLD"] - ), - "is_select": sql.lower().strip().startswith("select"), "template_info": template_info, } From 79afff439d5d110a263660a75985837acb29b92c Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Tue, 31 May 2022 17:22:58 +0300 Subject: [PATCH 4/6] Remove unused SQLPanel._offset attribute Not used since 5aff6ee75f8af3dd46254953b0b0de7c8e19c8e2 (March 2011). --- debug_toolbar/panels/sql/panel.py | 1 - 1 file changed, 1 deletion(-) diff --git a/debug_toolbar/panels/sql/panel.py b/debug_toolbar/panels/sql/panel.py index 9cdb1003a..5b450a358 100644 --- a/debug_toolbar/panels/sql/panel.py +++ b/debug_toolbar/panels/sql/panel.py @@ -111,7 +111,6 @@ class SQLPanel(Panel): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self._offset = {k: len(connections[k].queries) for k in connections} self._sql_time = 0 self._num_queries = 0 self._queries = [] From 031dfc678a5396259db2657ea7888ac8124ac48d Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Tue, 31 May 2022 17:01:35 +0300 Subject: [PATCH 5/6] Drop unneeded SQLPanel._num_queries attribute The same information can be retrieved from len(self._queries). --- debug_toolbar/panels/sql/panel.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/debug_toolbar/panels/sql/panel.py b/debug_toolbar/panels/sql/panel.py index 5b450a358..984a2074a 100644 --- a/debug_toolbar/panels/sql/panel.py +++ b/debug_toolbar/panels/sql/panel.py @@ -112,7 +112,6 @@ class SQLPanel(Panel): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._sql_time = 0 - self._num_queries = 0 self._queries = [] self._databases = {} # synthetic transaction IDs, keyed by DB alias @@ -151,7 +150,6 @@ def record(self, **kwargs): self._databases[alias]["time_spent"] += kwargs["duration"] self._databases[alias]["num_queries"] += 1 self._sql_time += kwargs["duration"] - self._num_queries += 1 # Implement the Panel API @@ -159,12 +157,13 @@ def record(self, **kwargs): @property def nav_subtitle(self): + query_count = len(self._queries) return ngettext( "%(query_count)d query in %(sql_time).2fms", "%(query_count)d queries in %(sql_time).2fms", - self._num_queries, + query_count, ) % { - "query_count": self._num_queries, + "query_count": query_count, "sql_time": self._sql_time, } From 165ce704432cd4c613539779ca813d0f7198132e Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Mon, 15 May 2023 14:35:03 +0300 Subject: [PATCH 6/6] Improve comment in wrap_cursor() More fully explain the reasoning for skipping monkey patching when running under a Django SimpleTestCase. --- debug_toolbar/panels/sql/tracking.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/debug_toolbar/panels/sql/tracking.py b/debug_toolbar/panels/sql/tracking.py index 35f0806f1..ee75f8e06 100644 --- a/debug_toolbar/panels/sql/tracking.py +++ b/debug_toolbar/panels/sql/tracking.py @@ -33,8 +33,14 @@ class SQLQueryTriggered(Exception): def wrap_cursor(connection): - # If running a Django SimpleTestCase, which isn't allowed to access the database, - # don't perform any monkey patching. + # When running a SimpleTestCase, Django monkey patches some DatabaseWrapper + # methods, including .cursor() and .chunked_cursor(), to raise an exception + # if the test code tries to access the database, and then undoes the monkey + # patching when the test case is finished. If we monkey patch those methods + # also, Django's process of undoing those monkey patches will fail. To + # avoid this failure, and because database access is not allowed during a + # SimpleTextCase anyway, skip applying our instrumentation monkey patches if + # we detect that Django has already monkey patched DatabaseWrapper.cursor(). if isinstance(connection.cursor, django.test.testcases._DatabaseFailure): return if not hasattr(connection, "_djdt_cursor"):