Debug query capturing on psycopg3 disregards execute wrappers.

[Ran Benita]

Problem

I use ​connection.execute_wrapper as a way to add comments to certain queries (see code at the end for reference). And I test the wrapper using assertNumQueries (see code at end for reference). Once I switched from psycopg2 to psycopg3, this test started failing: the unmodified query is captured, instead of the query modified by the execute wrapper. Note however that the modified query is what is actually sent to the DB, so this is only a debug issue.

I expect it's the same for django.db.backends debug logs but I haven't verified this.

Analysis

I've traced the issue to the following code in postgresql.DatabaseOperations.last_executed_query ​https://github.com/django/django/blob/66d58e77de3196404e0820a6fef0a6144186015a/django/db/backends/postgresql/operations.py#L296-L311:

    if is_psycopg3:

        def last_executed_query(self, cursor, sql, params):
            try:
                return self.compose_sql(sql, params)
            except errors.DataError:
                return None

    else:

        def last_executed_query(self, cursor, sql, params):
            # https://www.psycopg.org/docs/cursor.html#cursor.query
            # The query attribute is a Psycopg extension to the DB API 2.0.
            if cursor.query is not None:
                return cursor.query.decode()
            return None

psycopg2 uses cursor.query which ends up being the modified query. psycopg3 uses whatever's passed in which ends up being the unmodified query.

It seems like psycopg3 has an equivalent in ​cursor._query. It is documented in the API reference but with a warning "You shouldn’t consider it part of the public interface of the object: it might change without warnings. [...] If you would like to build reliable features using this object, please get in touch so we can try and design an useful interface for it.". So if this is the desired solution, will need to work with psycopg to expose a stable interface.

Reproduction code

Example execute wrapper:

def db_comment_wrapper(comment: str) -> AbstractContextManager[None]:
    def handler(execute, sql, params, many, context):
        clean_comment = escape(comment)
        return execute(f'/* {clean_comment} */ {sql}', params, many, context)
    return db_connection.execute_wrapper(handler)

Test:

with self.assertNumQueries(1) as captured:
    with db_comment_wrapper('This is a comment'):
        list(ContentType.objects.all())
sql = captured[0]['sql']
assert sql.startswith('/* This is a comment */')

[Mariusz Felisiak]

As far as I'm aware, there is no way to support it with psycopg's public API, so I'd not treat this as a bug.

Daniele, What do you think about extending psycopg's API?

[Mariusz Felisiak]

Daniele, What do you think about extending psycopg's API?

I opened an ​issue to continue a discussion with the psycopg maintainers.

[Florian Apolloner]

I think the new code was written like this because we cannot ever determine the actual query for server side bindings.

[Daniele Varrazzo]

I understand that Django use ClientCursor only, right? I think that the first iterations of the backend used to use server-side-binding cursors and maybe the current implementation of last_executed_query() was catered to that.

I have added a proposal to ​the upstream ticket.

[Florian Apolloner]

Replying to Daniele Varrazzo:

I understand that Django use ClientCursor only, right? I think that the first iterations of the backend used to use server-side-binding cursors and maybe the current implementation of last_executed_query() was catered to that.

Django defaults to ClientCursor but it can be changed to server-side-binding cursors via settings.

[Mariusz Felisiak]

Tentatively accepted. I think it's worth fixing even just for ClientCursor. Server-side binding cursors have a few other minor limitations in Django, so one more should not be a big surprise.

[Mariusz Felisiak]

Replying to Michail Chatzis:

Why you unset "needs improvement" flag without fixing my comments?

[Michail Chatzis]

Replying to Mariusz Felisiak:

Replying to Michail Chatzis:

Why you unset "needs improvement" flag without fixing my comments?

My bad, I thought you wanted me to unset the 'needs' flag when I wanted a review on Gitlab. I left a new comment on Gitlab, hence I wanted a review of it.

I am guessing I misunderstood that.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 4426b1a:

Fixed #35021 -- Fixed capturing queries when using client-side parameters binding with psycopg 3+.