#34486 closed Bug (fixed)
SearchHeadline crashes without an active connection.
Reported by: | Scott Macpherson | Owned by: | Scott Macpherson |
---|---|---|---|
Component: | contrib.postgres | Version: | 4.2 |
Severity: | Release blocker | Keywords: | |
Cc: | Florian Apolloner, Daniele Varrazzo | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Following an upgrade to Django 4.2 and psycopg3, any use of SearchVector has started returning AttributeError "'NoneType' object has no attribute 'pgconn'".
It seems to be because the instance of DatabaseWrapper passed in to as_sql doesn't have a connection: DatabaseOperations.compose_sql calls mogrify, which in turn calls .connection on the DatabaseWrapper, which returns NoneType.
I've verified that calling connection.connect() in search.py immediately before the compose_sql line resolves this, but I don't know nearly enough to know if this is the real root problem, or where in the stack the connection is supposed to be created if it is.
Attachments (1)
Change History (16)
by , 19 months ago
Attachment: | traceback.txt added |
---|
comment:1 by , 19 months ago
Type: | Uncategorized → Bug |
---|
I guess it's fixed by 4bf4222010fd8e413963c6c873e4088614332ef9 which removed compose_sql()
?
follow-up: 3 comment:2 by , 19 months ago
That commit does indeed resolve this issue. While checking that out I discovered that SearchHeadline.as_sql suffers from the same issue. Should I log a separate bug for that?
comment:3 by , 19 months ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Replying to Scott Macpherson:
While checking that out I discovered that SearchHeadline.as_sql suffers from the same issue. Should I log a separate bug for that?
Does it? In SearchHeadline.as_sql()
composed options are passed in params, so it should not be affected.
Duplicate of #34459.
follow-up: 5 comment:4 by , 19 months ago
The trouble doesn't seem to be with the structure of the call to compose_sql
, but it's called on a DatabaseOperations
object with a DatabaseWrapper
that doesn't have a connection. When mogrify
retrieves that connection it gets a NoneType back.
I can't work out why the tests in SearchHeadlineTests
don't capture this - could it have something to do with the way connections are reused in the tests, but not for a cold request?
… File "/Users/scott/Development/python_venvs/zerosleeps/lib/python3.11/site-packages/django/contrib/postgres/search.py", line 320, in as_sql ", ".join( ^ File "/Users/scott/Development/python_venvs/zerosleeps/lib/python3.11/site-packages/django/contrib/postgres/search.py", line 321, in <genexpr> connection.ops.compose_sql(f"{option}=%s", [value]) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/scott/Development/python_venvs/zerosleeps/lib/python3.11/site-packages/django/db/backends/postgresql/operations.py", line 205, in compose_sql return mogrify(sql, params, self.connection) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/scott/Development/python_venvs/zerosleeps/lib/python3.11/site-packages/django/db/backends/postgresql/psycopg_any.py", line 21, in mogrify return ClientCursor(connection.connection).mogrify(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/scott/Development/python_venvs/zerosleeps/lib/python3.11/site-packages/psycopg/cursor.py", line 672, in __init__ super().__init__(connection) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/scott/Development/python_venvs/zerosleeps/lib/python3.11/site-packages/psycopg/cursor.py", line 65, in __init__ self._pgconn = connection.pgconn ^^^^^^^^^^^^^^^^^ Exception Type: AttributeError at /search/ Exception Value: 'NoneType' object has no attribute 'pgconn'
comment:5 by , 19 months ago
Replying to Scott Macpherson:
I can't work out why the tests in
SearchHeadlineTests
don't capture this - could it have something to do with the way connections are reused in the tests, but not for a cold request?
I'm not sure how it is possible that you reach this point without a connection. Can you provide a regression test or a small project that reproduces the issue?
comment:7 by , 19 months ago
Cc: | added |
---|---|
Resolution: | duplicate |
Severity: | Normal → Release blocker |
Status: | closed → new |
Summary: | SearchVector.as_sql causes AttributeError → SearchHeadline crashes without an active connection. |
Triage Stage: | Unreviewed → Accepted |
Great catch! We should ensure an active connection in mogrify()
with psycopg
version 3, e.g.
-
django/db/backends/postgresql/psycopg_any.py
diff --git a/django/db/backends/postgresql/psycopg_any.py b/django/db/backends/postgresql/psycopg_any.py index 579104dead..1fe6b15caf 100644
a b try: 18 18 TSTZRANGE_OID = types["tstzrange"].oid 19 19 20 20 def mogrify(sql, params, connection): 21 return ClientCursor(connection.connection).mogrify(sql, params) 21 with connection.cursor() as cursor: 22 return ClientCursor(cursor.connection).mogrify(sql, params) 22 23 23 24 # Adapters. 24 25 class BaseTzLoader(TimestamptzLoader):
Would you like to prepare a patch (via GitHub PR)? a regression test is required.
follow-up: 9 comment:8 by , 19 months ago
I'd be happy to. Are you able to give me some pointers on replicating the behaviour we're seeing from inside TestCase
?
I've been mucking about with TransactionTestCase
rather than TestCase
, and I can get things into a state where connection.connection
inside mogrify
returns an idle connection rather than one that's in a transaction, but that's not far enough to replicate this.
comment:9 by , 19 months ago
Replying to Scott Macpherson:
I'd be happy to. Are you able to give me some pointers on replicating the behaviour we're seeing from inside
TestCase
?
I've been mucking about with
TransactionTestCase
rather thanTestCase
, and I can get things into a state whereconnection.connection
insidemogrify
returns an idle connection rather than one that's in a transaction, but that's not far enough to replicate this.
Since the real issue is in compose_sql()
, I'd add a backend-specific test, e.g.
-
tests/backends/postgresql/tests.py
diff --git a/tests/backends/postgresql/tests.py b/tests/backends/postgresql/tests.py index 9c4512be24..82df344139 100644
a b class Tests(TestCase): 420 420 with self.assertRaisesMessage(NotSupportedError, msg): 421 421 connection.check_database_version_supported() 422 422 self.assertTrue(mocked_get_database_version.called) 423 424 def test_compose_sql_no_connection(self): 425 new_connection = connection.copy() 426 # compose_sql() should open a new connection. 427 try: 428 self.assertEqual(new_connection.ops.compose_sql("SELECT %s", ["test"]), "SELECT 'test'") 429 finally: 430 new_connection.close()
comment:10 by , 19 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:11 by , 19 months ago
Thanks for your help - you made it too easy! Pull request 16760 created: https://github.com/django/django/pull/16760.
backends.postgres
has one new regression test - that suite had one skip before and the same skip after this change. tests.postgres_tests
produces the same output before and after this change as well.
comment:13 by , 19 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Traceback