#18176 closed Bug (fixed)
SQL queries for filtering datetime objects use incorrect format for years <1000
| Reported by: | rrotaru | Owned by: | rrotaru |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | datetime, SQL, year |
| Cc: | flo@… | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Datetime objects with year values <1000 are auto-formatted to 4-digits by adding leading
zeroes (i.e. year 1 becomes 0001). Also, according to the docs, the __year lookup takes a
four digit year (i.e., again, 0000-9999). However, when attempting to use the __year lookup on
years <1000, the SQL generated lacks the leading zeroes and as a result, no matches are made.
Example with Poll objects:
>>> poll1 = Poll(question="This",pub_date=datetime.datetime(0001,01,01)) >>> poll1.save() >>> Poll.objects.filter(pub_date__year=0001) #should return our poll1 []
Also, certain years <1000, such as 0999, or years ending in a 9 with at least one leading zero
generate "invalid token" syntax errors
>>> poll2 = Poll(question="That",pub_date=datetime.datetime(0999,01,01))
>>> poll2.save()
>>> Poll.objects.filter(pub_date__year=0999) #should return our poll2
Poll.objects.filter(pub_date__year=0999)
^
SyntaxError: invalid token
Note: This bug occurs when using sqlite3, I have not tried replicating it with other databases.
More discussion on this bug: https://groups.google.com/forum/?fromgroups#%21topic/django-developers/xPIQKbc4tiQ
Attachments (1)
Change History (12)
comment:1 by , 14 years ago
| Component: | Uncategorized → Database layer (models, ORM) |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 14 years ago
It turns out that python interprets numbers with leading zeroes as octal input (hence why anything with a leading zero that also contains an 8 or 9 causes an invalid token error). It seems like allowing strings to be passed in year lookups would solve both issues and might be the most reasonable approach.
Note: __year lookup already supports strings, however something along the lines of
Poll.objects.filter(pub_date__year='0999')
Still generates the following query in SQLite:
{'sql': u'SELECT "polls_poll"."id", "polls_poll"."question", "polls_poll"."pub_date" FROM "polls_poll" WHERE "polls_poll"."pub_date" BETWEEN 999-01-01 and 999-12-31 23:59:59.999999 LIMIT 21',
'time': '0.000'}]
(Notice how the years still lack the leading zeroes. This seems to be mainly an issue in SQLite.)
comment:3 by , 14 years ago
Tried a similar test using the postgresql_psycopg2 engine and achieved the following results:
In [28]: poll1 = Poll(question="poll1?",pub_date=datetime.datetime(0001,01,01))
In [29]: poll1.save()
In [30]: Poll.objects.filter(pub_date__year=0001)
Out[30]: []
In [31]: poll2 = Poll(question="poll2?",pub_date=datetime.datetime(1,01,01))
In [32]: poll2.save()
In [33]: Poll.objects.filter(pub_date__year=1)
Out[33]: []
In [34]: poll3 = Poll(question="poll3?",pub_date=datetime.datetime(0999,01,01))
------------------------------------------------------------
File "<ipython console>", line 1
poll3 = Poll(question="poll3?",pub_date=datetime.datetime(0999,01,01))
^
SyntaxError: invalid token
In [35]: poll4 = Poll(question="poll4?",pub_date=datetime.datetime(999,01,01))
In [36]: poll4.save()
In [37]: Poll.objects.filter(pub_date__year=999)
Out[37]: [<Poll: poll4?>]
Note that poll4 works while poll3 does not. They should create equally valid queries, but django seems to nip poll3. Here are the generated queries:
{'sql': 'INSERT INTO "polls_poll" ("question", "pub_date") VALUES (E\'poll1?\', E\'0001-01-01 00:00:00\') RETURNING "polls_poll"."id"',
'time': '0.001'},
{'sql': 'SELECT "polls_poll"."id", "polls_poll"."question", "polls_poll"."pub_date" FROM "polls_poll" WHERE "polls_poll"."pub_date" BETWEEN E\'1-01-01 00:00:00\' and E\'1-12-31 23:59:59.999999\' LIMIT 21',
'time': '0.002'},
{'sql': 'INSERT INTO "polls_poll" ("question", "pub_date") VALUES (E\'poll2?\', E\'0001-01-01 00:00:00\') RETURNING "polls_poll"."id"',
'time': '0.001'},
{'sql': 'SELECT "polls_poll"."id", "polls_poll"."question", "polls_poll"."pub_date" FROM "polls_poll" WHERE "polls_poll"."pub_date" BETWEEN E\'1-01-01 00:00:00\' and E\'1-12-31 23:59:59.999999\' LIMIT 21',
'time': '0.001'},
{'sql': 'INSERT INTO "polls_poll" ("question", "pub_date") VALUES (E\'poll4?\', E\'0999-01-01 00:00:00\') RETURNING "polls_poll"."id"',
'time': '0.001'},
{'sql': 'SELECT "polls_poll"."id", "polls_poll"."question", "polls_poll"."pub_date" FROM "polls_poll" WHERE "polls_poll"."pub_date" BETWEEN E\'999-01-01 00:00:00\' and E\'999-12-31 23:59:59.999999\' LIMIT 21',
'time': '0.001'}]
comment:4 by , 14 years ago
So, in django/db/backends/sqlite3 there is the following in base.py
def year_lookup_bounds(self, value):
first = '%s-01-01'
second = '%s-12-31 23:59:59.999999'
return [first % value, second % value]
Would it not make more sense to have something like the following?
def year_lookup_bounds(self, value):
first = datetime.datetime(value,1,1)
second = datetime.datetime(value,12,31,23,59,59,999999)
return [first, second]
This fixes the above issue and a similar patch can be applied to the base.py file in django/db/backends/mysql and django/db/backends/oracle
by , 14 years ago
| Attachment: | sql_query_fix.diff added |
|---|
comment:5 by , 14 years ago
| Has patch: | set |
|---|---|
| Needs documentation: | set |
| Needs tests: | set |
comment:6 by , 13 years ago
I think this issue is already fixed in master.
#18969 describes a similar issue and the test case provided there tests year lookups with a year < 1000. This test case worked for me with postgres and sqlite. The SQL is generated correctly. String lookups for __year work as well.
We should probably close either this ticket or #18969 as duplicate.
comment:7 by , 13 years ago
| Needs documentation: | unset |
|---|---|
| Needs tests: | unset |
| Version: | 1.4 → master |
I've created a pull request with a test and updated documentation (__year, __month,... accept integer and strings) by #18969
comment:8 by , 13 years ago
I closed #18969 as a duplicate of this one, I'll copy my comment below:
If the current implementation happens to accept strings, fantastic. But I'm against documenting it — because it means committing to support strings in these lookups forever.
Years, months, and friends are fundamentally integers and "there should be one-- and preferably only one --obvious way to do it."
In my opinion the only requirement to close this ticket is to add a test.
comment:9 by , 13 years ago
| Cc: | added |
|---|
I've moved the test from my pull request for #18969 and created a new one for this ticket, without the doc update: https://github.com/django/django/pull/847
comment:10 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
The main problem I see here is that if 999 is changed to mean '0999' instead of 1999 (or is it 2999?) this could break existing applications.
How about allowing strings in year lookups - document that what 1 means is database dependent like now(?). If you want year 0001 you need to pass '0001'. The problem with this is this can be a little confusing for users. Opinions on this API idea welcome!