#29648: Incorrect escaping when using subqueries inside datetime truncation
functions
-------------------------------------+-------------------------------------
Reporter: Raphael | Owner: Raphael Michel
Michel |
Type: Bug | Status: assigned
Component: Database | Version: 2.1
layer (models, ORM) |
Severity: Release | Keywords: Subqueries Datetime
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
If you build a subquery that (a) has a datetime-typed result and (b)
contains parameterized values, like this:
{{{
inner = Fan.objects.filter(
author=OuterRef('pk'),
name__in=('Emma', 'Isabella', 'Tom')
).values('author').annotate(newest_fan=Max('fan_since')).values('newest_fan')
}}}
And then use one of the Trunc* methods on the result, like this:
{{{
outer = Author.objects.annotate(
newest_fan_year=TruncYear(Subquery(inner,
output_field=DateTimeField()))
)
}}}
Then Django will throw an `OperationalError` or `TypeError`, because it
internally builds an incorrect query:
{{{
SELECT "db_functions_author"."name", django_datetime_trunc('year', (SELECT
MAX(U0."fan_since") AS "newest_fan" FROM "db_functions_fan" U0 WHERE
(U0."author_id" = ("db_functions_author"."id") AND U0."name" IN (%%s, %%s,
%%s)) GROUP BY U0."author_id"), 'UTC') AS "newest_fan_year" FROM
"db_functions_author" ORDER BY "db_functions_author"."name" ASC
}}}
As you can see, this query contains `%%s` instead of `%s` in place of the
parameters in the subquery. The reason is that `TruncBase` makes an
incorrect assumption about the database backends:
{{{
# Escape any params because trunc_sql will format the string.
inner_sql = inner_sql.replace('%s', '%%s')
}}}
There's two things wrong with this:
* This assumption is wrong, because no database backend in core runs
string formatting on this value. Instead, they use this value as a
parameter in string formatting of another string, like `sql = "TRUNC(%s,
'Q')" % field_name`.
* A Transform should not make assumptions about implementation details of
the database backend in the first place. Some backends, like the MySQL
one, use `.format()` instead of `%` to format their strings, so the
assumption doesn't apply at all.
For reference, ``Extract`` (which is very similar to ``TruncBase`` in many
regards) doesn't have this assumption. I therefore recommend to remove the
``replace`` statement without substitution to resolve the issue.
I've prepared a patch with a regression test that I'll send as a PR as
soon as I have the ticket number of this bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/29648>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/051.962f9cad94c64cb5b7b670e57cb0a3f2%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
