Skip to content

Commit 30cebbc

Browse files
committed
Append the caller data to each SQL query
This PR appends an SQL comment to each query containing the Python function/file/line calling Django to run that command. We experienced a busy database server blocked by one very expensive query. Another situation caused too many connections to the database server. Identifying the troublemaker within the source code wasn't easy. This PR adds the caller of each query to `SELECT query FROM pg_stat_activity` as a SQL comment: ``` INSERT INTO "tests_47ee19d1" ("id", "title") VALUES (1, 'Test') RETURNING "tests_47ee19d1"."id" /* 998020 test_append_caller_to_sql_crud .../django-postgres-extra/tests/test_append_caller_to_sql.py 55 */ SELECT "tests_47ee19d1"."id", "tests_47ee19d1"."title" FROM "tests_47ee19d1" WHERE "tests_47ee19d1"."id" = 1 LIMIT 1 /* 998020 test_append_caller_to_sql_crud .../django-postgres-extra/tests/test_append_caller_to_sql.py 69 */ UPDATE "tests_47ee19d1" SET "title" = 'success' WHERE "tests_47ee19d1"."id" = 1 /* 998020 test_append_caller_to_sql_crud .../django-postgres-extra/tests/test_append_caller_to_sql.py 64 */ DELETE FROM "tests_47ee19d1" WHERE "tests_47ee19d1"."id" IN (1) /* 998020 test_append_caller_to_sql_crud .../django-postgres-extra/tests/test_append_caller_to_sql.py 74 */ ``` The comment contains: 1. The system process id (PID). `pg_stat_activity` has a column `client_addr`. The server IP and the process id let you track down the originating process, even if PgBouncer is used inbetween. 2. The function name of the last function or method calling Django. 3. The file name and path of the source file where the call to Django happened. 4. The line number within the source file. Djangos internal commands show the process ID and name for easy identification of migrations and other maintenance operations. [#181799346]
1 parent 70b1442 commit 30cebbc

File tree

8 files changed

+196
-26
lines changed

8 files changed

+196
-26
lines changed

manage.py

100644100755
File mode changed.

psqlextra/backend/operations.py

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,25 @@
1-
from importlib import import_module
1+
from psqlextra.compiler import (
2+
PostgresAggregateCompiler,
3+
PostgresCompiler,
4+
PostgresDeleteCompiler,
5+
PostgresInsertCompiler,
6+
PostgresUpdateCompiler,
7+
)
28

39
from . import base_impl
410

511

612
class PostgresOperations(base_impl.operations()):
713
"""Simple operations specific to PostgreSQL."""
814

15+
compiler_map = {
16+
"SQLCompiler": PostgresCompiler,
17+
"SQLInsertCompiler": PostgresInsertCompiler,
18+
"SQLUpdateCompiler": PostgresUpdateCompiler,
19+
"SQLDeleteCompiler": PostgresDeleteCompiler,
20+
"SQLAggregateCompiler": PostgresAggregateCompiler,
21+
}
22+
923
def __init__(self, *args, **kwargs):
1024
super().__init__(*args, **kwargs)
1125

@@ -14,14 +28,9 @@ def __init__(self, *args, **kwargs):
1428
def compiler(self, compiler_name: str):
1529
"""Gets the SQL compiler with the specified name."""
1630

17-
# first let django try to find the compiler
18-
try:
19-
return super().compiler(compiler_name)
20-
except AttributeError:
21-
pass
22-
23-
# django can't find it, look in our own module
24-
if self._compiler_cache is None:
25-
self._compiler_cache = import_module("psqlextra.compiler")
31+
postgres_compiler = self.compiler_map.get(compiler_name)
32+
if postgres_compiler:
33+
return postgres_compiler
2634

27-
return getattr(self._compiler_cache, compiler_name)
35+
# Let Django try to find the compiler. Better run without caller comment than break
36+
return super().compiler(compiler_name)

psqlextra/compiler.py

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
import inspect
2+
import os
3+
import sys
4+
15
from collections.abc import Iterable
26
from typing import Tuple, Union
37

@@ -6,13 +10,59 @@
610
from django.core.exceptions import SuspiciousOperation
711
from django.db.models import Expression, Model, Q
812
from django.db.models.fields.related import RelatedField
9-
from django.db.models.sql.compiler import SQLInsertCompiler, SQLUpdateCompiler
13+
from django.db.models.sql.compiler import (
14+
SQLAggregateCompiler,
15+
SQLCompiler,
16+
SQLDeleteCompiler,
17+
SQLInsertCompiler,
18+
SQLUpdateCompiler,
19+
)
1020
from django.db.utils import ProgrammingError
1121

1222
from .expressions import HStoreValue
1323
from .types import ConflictAction
1424

1525

26+
def append_caller_to_sql(sql):
27+
try:
28+
# Search for the first non-Django caller
29+
stack = inspect.stack()
30+
i = 0
31+
for stack_frame in stack[1:]:
32+
i += 1
33+
frame_filename = stack_frame[1]
34+
frame_line = stack_frame[2]
35+
frame_function = stack_frame[3]
36+
if "/django/" in frame_filename or "/psqlextra/" in frame_filename:
37+
continue
38+
39+
return f"{sql} /* {os.getpid()} {frame_function} {frame_filename} {frame_line} */"
40+
41+
# Django internal commands (like migrations) end up here
42+
return f"{sql} /* {os.getpid()} {sys.argv[0]} */"
43+
except Exception:
44+
# Don't break anything because this convinence function runs into an unexpected situation
45+
return sql
46+
47+
48+
class PostgresCompiler(SQLCompiler):
49+
def as_sql(self):
50+
sql, params = super().as_sql()
51+
return append_caller_to_sql(sql), params
52+
53+
54+
class PostgresDeleteCompiler(SQLDeleteCompiler):
55+
def as_sql(self):
56+
sql, params = super().as_sql()
57+
return append_caller_to_sql(sql), params
58+
59+
60+
class PostgresAggregateCompiler(SQLAggregateCompiler):
61+
def as_sql(self):
62+
sql, params = super().as_sql()
63+
return append_caller_to_sql(sql), params
64+
65+
1666
class PostgresUpdateCompiler(SQLUpdateCompiler):
1767
"""Compiler for SQL UPDATE statements that allows us to use expressions
1868
inside HStore values.
@@ -24,7 +74,8 @@ class PostgresUpdateCompiler(SQLUpdateCompiler):
2474

2575
def as_sql(self):
2676
self._prepare_query_values()
27-
return super().as_sql()
77+
sql, params = super().as_sql()
78+
return append_caller_to_sql(sql), params
2879

2980
def _prepare_query_values(self):
3081
"""Extra prep on query values by converting dictionaries into.
@@ -72,19 +123,32 @@ def _does_dict_contain_expression(data: dict) -> bool:
72123
class PostgresInsertCompiler(SQLInsertCompiler):
73124
"""Compiler for SQL INSERT statements."""
74125

75-
def __init__(self, *args, **kwargs):
76-
"""Initializes a new instance of :see:PostgresInsertCompiler."""
126+
def as_sql(self, return_id=False):
127+
"""Builds the SQL INSERT statement."""
128+
queries = [
129+
(append_caller_to_sql(sql), params)
130+
for sql, params in super().as_sql()
131+
]
132+
133+
return queries
134+
77135

136+
class PostgresInsertOnConflictCompiler(SQLInsertCompiler):
137+
"""Compiler for SQL INSERT statements."""
138+
139+
def __init__(self, *args, **kwargs):
140+
"""Initializes a new instance of
141+
:see:PostgresInsertOnConflictCompiler."""
78142
super().__init__(*args, **kwargs)
79143
self.qn = self.connection.ops.quote_name
80144

81145
def as_sql(self, return_id=False):
82146
"""Builds the SQL INSERT statement."""
83-
84147
queries = [
85148
self._rewrite_insert(sql, params, return_id)
86149
for sql, params in super().as_sql()
87150
]
151+
print(f"InsertConflictCompiler {queries}")
88152

89153
return queries
90154

@@ -132,9 +196,12 @@ def _rewrite_insert(self, sql, params, return_id=False):
132196
self.qn(self.query.model._meta.pk.attname) if return_id else "*"
133197
)
134198

135-
return self._rewrite_insert_on_conflict(
136-
sql, params, self.query.conflict_action.value, returning
137-
)
199+
if hasattr(self.query, "conflict_action"):
200+
(sql, params) = self._rewrite_insert_on_conflict(
201+
sql, params, self.query.conflict_action.value, returning
202+
)
203+
204+
return (append_caller_to_sql(sql), params)
138205

139206
def _rewrite_insert_on_conflict(
140207
self, sql, params, conflict_action: ConflictAction, returning

psqlextra/sql.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from django.db.models import sql
99
from django.db.models.constants import LOOKUP_SEP
1010

11-
from .compiler import PostgresInsertCompiler, PostgresUpdateCompiler
11+
from .compiler import PostgresInsertOnConflictCompiler, PostgresUpdateCompiler
1212
from .expressions import HStoreColumn
1313
from .fields import HStoreField
1414
from .types import ConflictAction
@@ -179,7 +179,7 @@ def values(self, objs: List, insert_fields: List, update_fields: List = []):
179179
def get_compiler(self, using=None, connection=None):
180180
if using:
181181
connection = connections[using]
182-
return PostgresInsertCompiler(self, connection, using)
182+
return PostgresInsertOnConflictCompiler(self, connection, using)
183183

184184

185185
class PostgresUpdateQuery(sql.UpdateQuery):

tests/test_append_caller_to_sql.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import pytest
2+
3+
from django.db import connection, models
4+
from django.test.utils import CaptureQueriesContext
5+
6+
from psqlextra.compiler import append_caller_to_sql
7+
8+
from .fake_model import get_fake_model
9+
10+
11+
class psqlextraSimulated:
12+
def callMockedClass(self):
13+
return MockedClass().mockedMethod()
14+
15+
16+
class MockedClass:
17+
def mockedMethod(self):
18+
return append_caller_to_sql("sql")
19+
20+
21+
def mockedFunction():
22+
return append_caller_to_sql("sql")
23+
24+
25+
@pytest.mark.parametrize(
26+
"entry_point",
27+
[
28+
MockedClass().mockedMethod,
29+
psqlextraSimulated().callMockedClass,
30+
],
31+
)
32+
def test_append_caller_to_sql_class(entry_point):
33+
commented_sql = entry_point()
34+
assert commented_sql.startswith("sql /* ")
35+
assert "mockedMethod" in commented_sql
36+
assert __file__ in commented_sql
37+
38+
39+
def test_append_caller_to_sql_function():
40+
commented_sql = mockedFunction()
41+
assert commented_sql.startswith("sql /* ")
42+
assert "mockedFunction" in commented_sql
43+
assert __file__ in commented_sql
44+
45+
46+
def test_append_caller_to_sql_crud():
47+
model = get_fake_model(
48+
{
49+
"title": models.CharField(max_length=255, null=True),
50+
}
51+
)
52+
53+
obj = None
54+
with CaptureQueriesContext(connection) as queries:
55+
obj = model.objects.create(
56+
id=1,
57+
title="Test",
58+
)
59+
assert "test_append_caller_to_sql_crud " in queries[0]["sql"]
60+
61+
obj.title = "success"
62+
with CaptureQueriesContext(connection) as queries:
63+
obj.save()
64+
assert "test_append_caller_to_sql_crud " in queries[0]["sql"]
65+
66+
with CaptureQueriesContext(connection) as queries:
67+
assert model.objects.filter(id=obj.id)[0].id == obj.id
68+
assert "test_append_caller_to_sql_crud " in queries[0]["sql"]
69+
70+
with CaptureQueriesContext(connection) as queries:
71+
obj.delete()
72+
assert "test_append_caller_to_sql_crud " in queries[0]["sql"]

tests/test_on_conflict.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from django.core.exceptions import SuspiciousOperation
55
from django.db import connection, models
6+
from django.test.utils import CaptureQueriesContext
67
from django.utils import timezone
78

89
from psqlextra.fields import HStoreField
@@ -23,9 +24,11 @@ def test_on_conflict(conflict_action):
2324
}
2425
)
2526

26-
obj = model.objects.on_conflict(
27-
[("title", "key1")], conflict_action
28-
).insert_and_get(title={"key1": "beer"}, cookies="cheers")
27+
with CaptureQueriesContext(connection) as queries:
28+
obj = model.objects.on_conflict(
29+
[("title", "key1")], conflict_action
30+
).insert_and_get(title={"key1": "beer"}, cookies="cheers")
31+
assert " test_on_conflict " in queries[0]["sql"]
2932

3033
model.objects.on_conflict(
3134
[("title", "key1")], conflict_action

tests/test_query.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
from django.db import models
1+
from django.db import connection, models
22
from django.db.models import Case, F, Q, Value, When
3+
from django.test.utils import CaptureQueriesContext
34

45
from psqlextra.expressions import HStoreRef
56
from psqlextra.fields import HStoreField
@@ -134,3 +135,20 @@ def test_query_hstore_value_update_escape():
134135

135136
inst = model.objects.all().first()
136137
assert inst.title.get("en") == "console.log('test')"
138+
139+
140+
def test_query_comment():
141+
"""Tests whether the query is commented."""
142+
143+
model = get_fake_model(
144+
{
145+
"name": models.CharField(max_length=10),
146+
"value": models.IntegerField(),
147+
}
148+
)
149+
150+
with CaptureQueriesContext(connection) as queries:
151+
qs = model.objects.all()
152+
assert " test_query_comment " in str(qs.query)
153+
list(qs)
154+
assert " test_query_comment " in queries[0]["sql"]

tests/test_view_models.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ def test_view_model_meta_query_set(model_base):
2626
expected_sql = 'SELECT "{0}"."id", "{0}"."name" FROM "{0}"'.format(
2727
model._meta.db_table
2828
)
29-
assert view_model._view_meta.query == (expected_sql, tuple())
29+
assert view_model._view_meta.query[0].startswith(expected_sql + " /* ")
30+
assert view_model._view_meta.query[1] == tuple()
3031

3132

3233
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)