Opened 6 years ago
Last modified 6 years ago
#29648 closed Bug
Incorrect escaping when using subqueries inside datetime truncation functions — at Initial Version
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
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.