#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 , 13 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 13 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.
comment:3 by , 13 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 , 13 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 , 13 years ago
Attachment: | sql_query_fix.diff added |
---|
comment:5 by , 13 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
comment:6 by , 12 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 , 12 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 , 12 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 , 12 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 , 12 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!