[Django] #25475: Database function srtftime with argument %W fails in 1.9a1
Django
----------------------------------------------+---------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.9a1
Severity: Normal | Keywords: func strftime
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+---------------------------
I have a database function to annotate the week of a date. It worked in
1.8 but breaks in 1.9a1, failing with a "ValueError: unsupported format
character 'W'" exception.
{{{
class Week(Func):
function = 'strftime'
template = "%(function)s('%%W',%(expressions)s)"
def data_last_n_weeks(user, week_count):
assert week_count > 0, "week_count must be greater than 0"
assert week_count < 52, "week_count must be less than 52"
today = date.today()
most_recent_monday = today - timedelta(days=(today.isoweekday()-1))
start_date = most_recent_monday - timedelta(days=7*(week_count-1))
data = user.serving_set.filter(date__range=(start_date, today)
).annotate(week=Week('date',
output_field=IntegerField())).values(
'week').annotate(amt=Sum('amount'), cost=Sum('cost'))
return data
}}}
yields
{{{
File "/Users/[me]/Projects/[proj]/ENV/lib/python3.4/site-
packages/django/db/backends/sqlite3/operations.py", line 133, in
last_executed_query
return sql % params
ValueError: unsupported format character 'W' (0x57) at index 18
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25475>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: func strftime | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by vtbassmatt):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
I think we need to re-escape % after executing `template % self.extra` in
`Func.as_sql`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I'm having trouble getting the database function you provided working on
1.8. Am I doing something wrong? I tried
`Poll.objects.annotate(week=Week('pub_date',
output_field=models.IntegerField())`.
This gives:
{{{
File "/home/tim/code/django/django/db/backends/utils.py", line 66, in
execute
return self.cursor.execute(sql, params)
IndexError: tuple index out of range
}}}
with `sql` and `params`:
`SELECT "polls_poll"."id", "polls_poll"."question",
"polls_poll"."pub_date", "polls_poll"."amount", "polls_poll"."cost",
strftime('%W',"polls_poll"."pub_date") AS "week" FROM "polls_poll" LIMIT
21`
`()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vtbassmatt):
I don't see anything wrong with the simplified case you wrote. I'm not in
a place where I can generate the sql from my repro but I'll get it
tonight.
One possible difference is that Serving.date is a DateField while I
suspect Poll.pub_date is a DateTimeField. That could matter.
Also, I've only tested with SQLite - possible that this is database
dependent?
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Looking at the excerpt of the stack trace you posted I'm pretty sure it
only happens on SQLite.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vtbassmatt):
I couldn't repro the IndexError in my setup. Here are two execution logs,
one on 1.8.4 and the other on 1.9.0a1 using the exact same code and (a
copy of) the exact same database. This time I haven't manually doctored
the output for simplification as I did earlier.
1.8.4:
{{{
(3NV) mattc ~/Projects/beer30/beersite $ ./manage.py shell
Python 3.4.2 (default, Oct 19 2014, 12:51:48)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.54)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from django.contrib.auth.models import User
>>> from api.views import beer_data_last_n_weeks
>>> u = User.objects.get(pk=1)
>>> beer_data_last_n_weeks(u, 5)
[{'cost': Decimal('41.00'), 'week': 35, 'oz': Decimal('228.0')}, {'cost':
Decimal('41.50'), 'week': 36, 'oz': Decimal('158.0')}, {'cost':
Decimal('28.93'), 'week': 37, 'oz': Decimal('198.6')}, {'cost':
Decimal('24.66'), 'week': 38, 'oz': Decimal('128.0')}, {'cost':
Decimal('3.33'), 'week': 39, 'oz': Decimal('12.0')}]
}}}
1.9.0a1:
{{{
(3NV) mattc ~/Projects/beer31/beersite $ ./manage.py shell
Python 3.4.3 (default, Aug 11 2015, 08:57:25)
[GCC 4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.53)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from django.contrib.auth.models import User
>>> from api.views import beer_data_last_n_weeks
>>> u = User.objects.get(pk=1)
>>> beer_data_last_n_weeks(u, 5)
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/Users/mattc/Projects/beer31/3NV/lib/python3.4/site-
packages/django/db/models/query.py", line 234, in __repr__
data = list(self[:REPR_OUTPUT_SIZE + 1])
File "/Users/mattc/Projects/beer31/3NV/lib/python3.4/site-
packages/django/db/models/query.py", line 258, in __iter__
self._fetch_all()
File "/Users/mattc/Projects/beer31/3NV/lib/python3.4/site-
packages/django/db/models/query.py", line 1074, in _fetch_all
self._result_cache = list(self.iterator())
File "/Users/mattc/Projects/beer31/3NV/lib/python3.4/site-
packages/django/db/models/query.py", line 112, in __iter__
for row in compiler.results_iter():
File "/Users/mattc/Projects/beer31/3NV/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 806, in results_iter
results = self.execute_sql(MULTI)
File "/Users/mattc/Projects/beer31/3NV/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 852, in execute_sql
cursor.execute(sql, params)
File "/Users/mattc/Projects/beer31/3NV/lib/python3.4/site-
packages/django/db/backends/utils.py", line 83, in execute
sql = self.db.ops.last_executed_query(self.cursor, sql, params)
File "/Users/mattc/Projects/beer31/3NV/lib/python3.4/site-
packages/django/db/backends/sqlite3/operations.py", line 133, in
last_executed_query
return sql % params
ValueError: unsupported format character 'W' (0x57) at index 18
}}}
Relevant code snippets:
api/views.py
{{{
class Week(Func):
function = 'strftime'
template = "%(function)s('%%W',%(expressions)s)"
def beer_data_last_n_weeks(user, week_count):
assert week_count > 0, "week_count must be greater than 0"
assert week_count < 52, "week_count must be less than 52"
today = date.today()
most_recent_monday = today - timedelta(days=(today.isoweekday()-1))
start_date = most_recent_monday - timedelta(days=7*(week_count-1))
beer_data = user.serving_set.filter(date__range=(start_date, today)
).annotate(week=Week('date',
output_field=IntegerField())).values(
'week').annotate(oz=Sum('amount'), cost=Sum('cost'))
return beer_data
}}}
datamodel/models.py
{{{
class Serving(models.Model):
SERVING_CHOICES = (
(0, "unknown"),
(1, "tap"),
(2, "bottle"),
(4, "can"),
(3, "growler"),
)
beer = models.ForeignKey(Beer)
amount = models.DecimalField(max_digits=4, decimal_places=1)
date = models.DateField()
type = models.SmallIntegerField(choices=SERVING_CHOICES, default=1)
owner = models.ForeignKey(User)
location = models.CharField(max_length=100, blank=True)
cost = models.DecimalField(max_digits=5,
decimal_places=2, null=True, blank=True)
def __str__(self):
return " ".join([str(self.amount), "oz of", self.beer.name])
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I'm attaching a sample app based on the models you provided (I removed the
foreign key to `Beer` since you didn't provide that model), however, the
test works without error for me. Does it work for you? If so, could you
say how to modify it to reproduce the error? If not, then perhaps it's
something like a difference between versions of SQLite.
System details:
Python 3.4.3 (default, Mar 5 2015, 15:15:09)
[GCC 4.8.2] on linux
sqlite3 2.6.0
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* Attachment "t25475.tar.gz" added.
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vtbassmatt):
Your test case does not fail on the Windows machine I currently have
available (Windows 10.0.10240 with SQLite3 2.6.0, sqlite3.sqlite_version
3.8.3.1). Will check on my Mac later tonight - that's where I've observed
the failure.
From looking at Apple's online man pages, 3.1.3 is shown in the sample
outputs.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vtbassmatt):
Argh, that test case doesn't fail on my Mac either. The real app still
exhibits the behavior, so I'll keep digging and see what it takes to make
the test case fail.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vtbassmatt):
I'm so sorry. Somewhere between my full app and your test case there's a
cutover point where this starts failing on 1.9a1, but I can't find it.
I've learned quite a bit more about Pdb tonight than I really intended but
to no avail.
1.8 and 1.9 both accept a double-escaped percent sign, so I'm going to
update my code to that.
{{{
class Week(Func):
function = 'strftime'
template = "%(function)s('%%%%W',%(expressions)s)"
}}}
I'd be happy to share the whole project with someone from the core team if
it would help further debugging.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jarshwah):
* cc: josh.smeaton@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
If you could [https://docs.djangoproject.com/en/dev/internals/contributing
/triaging-tickets/#bisecting-a-regression use git bisect] to find the
commit where the behavior changed, that might yield some insight.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
The exception is triggered in code I added in [4f6a7663]. However it's
unclear to me why this code would be incorrect.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
It may be because `_quote_params_for_last_executed_query` is bypassing the
Django cursor wrapper. When the wrapper is executing a query, it takes
care of not passing `params` when it is None. Maybe we should do the same
dance in `_quote_params_for_last_executed_query`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
I think the bug is rather that the initial version didn't break in 1.8!
You implicitly counted on the fact the the sqlite driver accepts the
following query (2 placeholder-like occurrences, one param):
{{{
cursor.execute("SELECT strftime('%W', 'date') WHERE 42=%s", (42,))
}}}
It may be because the SQLite driver only recognizes the `%s` placeholder
(a guess). A similar query with MySQL and DATE_FORMAT by example would
produce an error.
We might fix the bug by a documentation note somewhere in custom
expressions.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vtbassmatt):
Totally fair, I probably misunderstood the docs :)
So I should have double-escaped to begin with? In retrospect that makes
sense. If you guys want to doc that and close this issue, I'm good with
that.
{{{
class Week(Func):
function = 'strftime'
template = "%(function)s('%%%%W',%(expressions)s)"
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:17>
Django
--------------------------------------+------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Bug => Cleanup/optimization
* version: 1.9a1 => 1.8
* component: Database layer (models, ORM) => Documentation
* severity: Release blocker => Normal
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:18>
Django
--------------------------------------+------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Looking at this now, I'm not sure what the documentation note should say.
Can anyone suggest something or at least give a simple description of the
issue?
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:19>
Django
--------------------------------------+------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vtbassmatt):
I would create a documentation PR myself, but I don't understand the issue
enough to feel comfortable educating others. Summary of behavior is that
when I created a custom expression, something between the code I wrote and
the SQLite driver was eating some of my escape characters. I had to
express a logical "%W" as "%%%%W" (four percent characters) to get it
working properly. I've lost most of the context about the failure, but I
recall expecting to need two percents (%%W) and being surprised that it
took four.
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:20>
Django
--------------------------------------+------------------------------------
Reporter: vtbassmatt | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: func strftime | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
I would place the note here:
https://docs.djangoproject.com/en/1.10/ref/models/expressions/#django.db.models.Func.template
Something like (feel free to reformulate!):
If you need a literal `%` character to reach the database, you will need
to quadruple it in the `template` attribute because the string will be
extrapoled twice (once during the template extrapolation in `as_sql()`,
and once in the final SQL extrapolation with the query parameters in the
database cursor).
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:21>
Django
--------------------------------------+------------------------------------
Reporter: Matt Cooper | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/7355 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:22>
Django
Reporter: Matt Cooper | Owner: nobody
Cleanup/optimization |
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* stage: Accepted => Ready for checkin
Comment:
Patch looks good!
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: Matt Cooper | Owner: nobody
Cleanup/optimization |
Component: Documentation | Version: 1.8
Keywords: func strftime | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by GitHub <noreply@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"c60feb6999f83bfd1fdabff01f0dd4a26d72e158" c60feb69]:
{{{
#!CommitTicketReference repository=""
revision="c60feb6999f83bfd1fdabff01f0dd4a26d72e158"
Fixed #25475 -- Doc'd how to use a literal % in Func.template.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: Matt Cooper | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: func strftime | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"be535960b1b877da50c2b66301820f68e312463c" be535960]:
{{{
#!CommitTicketReference repository=""
revision="be535960b1b877da50c2b66301820f68e312463c"
[1.10.x] Fixed #25475 -- Doc'd how to use a literal % in Func.template.
Backport of c60feb6999f83bfd1fdabff01f0dd4a26d72e158 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25475#comment:25>