SearchHeadline crashes without an active connection.

[Scott Macpherson]

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.

[Scott Macpherson]

Traceback

[Tim Graham]

I guess it's fixed by 4bf4222010fd8e413963c6c873e4088614332ef9 which removed compose_sql()?

[Scott Macpherson]

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?

[Mariusz Felisiak]

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.

[Scott Macpherson]

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'

[Mariusz Felisiak]

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?

[Scott Macpherson]

I've knocked up a little project which demonstrates the issue:

​https://github.com/scottmacpherson/postgres_search

[Mariusz Felisiak]

Great catch! We should ensure an active connection in mogrify() with psycopg version 3, e.g.

django/db/backends/postgresql/psycopg_any.py

@@ -18 +18 @@
     TSTZRANGE_OID = types["tstzrange"].oid
 
     def mogrify(sql, params, connection):
-        return ClientCursor(connection.connection).mogrify(sql, params)
+        with connection.cursor() as cursor:
+            return ClientCursor(cursor.connection).mogrify(sql, params)
 
     # Adapters.
     class BaseTzLoader(TimestamptzLoader):

Would you like to prepare a patch (via GitHub PR)? a regression test is required.

[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 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.

[Mariusz Felisiak]

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 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.

Since the real issue is in compose_sql(), I'd add a backend-specific test, e.g.

tests/backends/postgresql/tests.py

@@ -420 +420 @@
         with self.assertRaisesMessage(NotSupportedError, msg):
             connection.check_database_version_supported()
         self.assertTrue(mocked_get_database_version.called)
+
+    def test_compose_sql_no_connection(self):
+        new_connection = connection.copy()
+        # compose_sql() should open a new connection.
+        try:
+            self.assertEqual(new_connection.ops.compose_sql("SELECT %s", ["test"]), "SELECT 'test'")
+        finally:
+            new_connection.close()

[Scott Macpherson]

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.

[Scott Macpherson]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 53aee470:

Fixed #34486 -- Fixed DatabaseOperations.compose_sql() crash with no existing database connection on PostgreSQL.

Regression in 09ffc5c1212d4ced58b708cbbf3dfbfb77b782ca.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 090d5cc:

[4.2.x] Fixed #34486 -- Fixed DatabaseOperations.compose_sql() crash with no existing database connection on PostgreSQL.

Regression in 09ffc5c1212d4ced58b708cbbf3dfbfb77b782ca.

Backport of 53aee470d5b35e2708864d5221d2b5655e10c091 from main