id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
29648	Incorrect escaping when using subqueries inside datetime truncation functions	Raphael Michel	Raphael Michel	"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."	Bug	closed	Database layer (models, ORM)	dev	Normal	fixed	Subqueries Datetime		Ready for checkin	1	0	0	0	0	0