Opened 6 years ago
Last modified 6 years ago
#29648 closed Bug
Incorrect escaping when using subqueries inside datetime truncation functions — at Version 3
Reported by: | Raphael Michel | Owned by: | Raphael Michel |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | Subqueries Datetime |
Cc: | 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 (last modified by )
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.
Change History (3)
comment:1 by , 6 years ago
Has patch: | set |
---|
comment:2 by , 6 years ago
comment:3 by , 6 years ago
Description: | modified (diff) |
---|
Patch submitted at https://github.com/django/django/pull/10274