Opened 2 years ago

Closed 2 years ago

#33815 closed Bug (fixed)

last_executed_query() incorrectly substitutes parameters on Oracle.

Reported by: Brian Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: oracle
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Since Django 3.0, the variable substitution performed in the Oracle backend's last_executed_query function can incorrectly track the sql statements issued to the backend. Here's the method copied from https://github.com/django/django/blob/main/django/db/backends/oracle/operations.py#L304:

    def last_executed_query(self, cursor, sql, params):
        # https://cx-oracle.readthedocs.io/en/latest/api_manual/cursor.html#Cursor.statement
        # The DB API definition does not define this attribute.
        statement = cursor.statement
        # Unlike Psycopg's `query` and MySQLdb`'s `_executed`, cx_Oracle's
        # `statement` doesn't contain the query parameters. Substitute
        # parameters manually.
        if isinstance(params, (tuple, list)):
            for i, param in enumerate(params):
                statement = statement.replace(
                    ":arg%d" % i, force_str(param, errors="replace")
                )
        elif isinstance(params, dict):
            for key, param in params.items():
                statement = statement.replace(
                    ":%s" % key, force_str(param, errors="replace")
                )
        return statement

The problem is that statement.replace will end up replacing all matches in the statement, even those that are not a full match for the argument identifier. This can result in values that are a mashup of the subbed in value and the argument identifiers. For example, if you have values A-L that need to be substituted into a query, you'd have 12 arguments that would need to be substituted in, and the following scenario would occur:

Statement Pre-substitution:

SELECT 
    "EMPLOYEE"."ID",
    "EMPLOYEE"."USERNAME",
    "EMPLOYEE"."NAME",
    "EMPLOYEE"."EMAIL"
FROM "EMPLOYEE"
WHERE "EMPLOYEE"."ID" IN (:arg0,  :arg1,  :arg2,  :arg3,  :arg4,  :arg5,  :arg6,  :arg7, :arg8,  :arg9  :arg10,  :arg11)

Statement Post-substitution:

SELECT 
    "EMPLOYEE"."ID",
    "EMPLOYEE"."USERNAME",
    "EMPLOYEE"."NAME",
    "EMPLOYEE"."EMAIL"
FROM "EMPLOYEE"
WHERE "EMPLOYEE"."ID" IN (A, B, C, D, E, F, G, H, I , J,  B0,  B1)

Expected Output:

SELECT 
    "EMPLOYEE"."ID",
    "EMPLOYEE"."USERNAME",
    "EMPLOYEE"."NAME",
    "EMPLOYEE"."EMAIL"
FROM "EMPLOYEE"
WHERE "EMPLOYEE"."ID" IN (A, B, C, D, E, F, G, H, I , J,  K,  L)

Change History (4)

comment:1 by Mariusz Felisiak, 2 years ago

Component: UncategorizedDatabase layer (models, ORM)
Keywords: oracle added
Summary: Oracle sql tracking incorrectly substitutes in parameters when 10 or more parameters are used.last_executed_query() incorrectly substitutes parameters on Oracle.
Triage Stage: UnreviewedAccepted

Thanks for the report. I attached a small regression test:

  • tests/backends/tests.py

    diff --git a/tests/backends/tests.py b/tests/backends/tests.py
    index 9303089b51..b3e3f68335 100644
    a b class LastExecutedQueryTest(TestCase):  
    101101                pk=1,
    102102                reporter__pk=9,
    103103            ).exclude(reporter__pk__in=[2, 1]),
     104            Article.objects.filter(pk__in=list(range(20, 31))),
    104105        ):
    105106            sql, params = qs.query.sql_with_params()
    106107            with qs.query.get_compiler(DEFAULT_DB_ALIAS).execute_sql(CURSOR) as cursor:

comment:2 by Mariusz Felisiak, 2 years ago

Would you like to prepare a patch?

comment:3 by Mariusz Felisiak, 2 years ago

Has patch: set
Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

comment:4 by GitHub <noreply@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 249ecc43:

Fixed #33815 -- Fixed last_executed_query() on Oracle when parameter names overlap.

Note: See TracTickets for help on using tickets.
Back to Top