.dates(...) only spitting out a single date, bug in queryset order

[Mathijs de Bruin]

Due to a bug in the add_date_select function (db/models/sql/subqueries.py:400) I am only seeing the latest date (when using SQLite).

In [2]: Event.objects.all()
Out[2]: [<Event: fsddf on 2009-01-03>, <Event: Fietsen on 2008-10-12>, <Event: Kaas on 2008-10-11>, <Event: Woei on 2008-06-03>]

In [3]: Event.objects.dates('event_date', 'day')
Out[3]: [datetime.datetime(2008, 6, 3, 0, 0)]

Where the latter produces the following SQL code:

SELECT DISTINCT django_date_trunc("day", "agenda_event"."event_date") FROM "agenda_event" ORDER BY 1 ASC LIMIT 21

Here, the mysterious ORDER BY 1 clause gets SQLite in confusion. It comes straight from:

    def add_date_select(self, field, lookup_type, order='ASC'):
        <snap>
        self.distinct = True
        self.order_by = order == 'ASC' and [1] or [-1]

We can see that this is indeed the cause by running:

In [31]: Event.objects.dates('event_date', 'day').order_by('event_date')
Out[31]: [datetime.datetime(2008, 6, 3, 0, 0), datetime.datetime(2008, 10, 11, 0, 0), datetime.datetime(2008, 10, 12, 0, 0), datetime.datetime(2009, 1, 3, 0, 0)]

I propose to change the named line into:

        self.order_by = order == 'ASC' and [field.name] or ['-%s' % field.name]

After which the code is working just fine:

In [3]: Event.objects.all().dates('event_date', 'day')
Out[3]: [datetime.datetime(2008, 6, 3, 0, 0), datetime.datetime(2008, 10, 11, 0, 0), datetime.datetime(2008, 10, 12, 0, 0), datetime.datetime(2009, 1, 3, 0, 0)]

FYI: Django version 1.1 pre-alpha, r9231 of trunk

[Malcolm Tredinnick]

"Order by 1" is perfectly valid SQL and not at all mysterious. We need some more details here to work out if it's a bug in a particular version of SQLite or in SQLite in general and what a reasonable way to work around it will be. Ordering by event_date, as you suggest is not, from memory, going to be valid SQL, since all columns that appear in an ordering statement must appear as output columns in the select statement (and we're not selecting event_date, but something derived from event_date, so it's an input column and not an output column).

What version of SQLite are you using? On what platform?

[Mathijs de Bruin]

I am running sqlite-3.6.2 and pysqlite-2.4.1 on Gentoo prefix on Leopard.

About the patch: I agree with you that it shouldn't work, but for me it does. Better than the original code.
Remembering from my SQL days, correct code would be something like:

SELECT DISTINCT django_date_trunc("day", "agenda_event"."event_date") AS django_date FROM "agenda_event" ORDER BY django_date ASC

[Malcolm Tredinnick]

As I'm sure you realise, "works for me on one particular database known for taking a bunch of shortcuts" isn't a way to validate a patch, since we have to generate correct SQL so that we run on all databases.

Looks like a nice recent version of SQLite, though, so we need to work around it until SQLite learns that part of the spec.

[Mathijs de Bruin]

I'll see if I can find the time to look up the 'official' way we're supposed to do this stuff in SQLite.

For now I think other users might actually be helped by my works-for-me patch, allthough I should have realized it's not proper SQL. :)

[Malcolm Tredinnick]

The point is that I don't want to have code that does things way for SQLite and some other way for other databases. The ordering and column selections pieces of the query construction are relatively separated and having to put part of the ordering construction into the db/backends/* part of the code is a bit fugly.

Fortunately, we can avoid it. As you note, we can use a named alias for the output column gets around this, so I'll add that into the query. That will also avoid a problem with "order by 1" problems that we've seen on older SQLites on Windows (particularly the SQLite shipped with the Python 2.5 binary).

Short version: no need to worry about an SQLite-specific workaround. Assuming the aliased query actually works on SQLite (does it?), I'm going to take that approach.

[drbob@…]

I have just done a few hours of messing around with the Django db code and it seems to me that naming the date column will require an fairly fundamental change in the way .order_by() parses its parameters (right now it only looks in model.opts and extra_select). I can confirm however that working with named aliases DOES work. Perfectly. :)

This again gave me the idea to put the Date() in extra_select instead of select, which then automatically does the naming and also allows for named sorting. The trouble here though is that:

• Setting select to [None] (line 381) causes the code not to query the database at all. This seems like bad behaviour to me (sometimes you only want extra data) but this is design decision which seems not related to the current bug.
  
• Putting the Date() function directly in extra_select does not call its as_sql method. Directly calling the as_sql function from add_date_select would get us into trouble with pickling, I suppose.

It is very likely that similar problems will occur with sorting on aggregate and other 'intelligent' columns as well.

Possible solutions for the problem include:

• Upgrading to an SQLite 3.6.4 (maybe even 3.6.3). In this version the described problem is fixed. This seems a good workaround solution for now.
  
• Enabling queries for empty selects and using named extra_select columns for all kind of 'intelligence' like this. This means we have to call .as_sql() in extra_select columns. Hereby we implement this kind of functionality in a consistent way (alas, extra_select is what we would have used before .dates() existed), so we would not have to do any .order_by() workaround. Here .dates would simply be a piece of DB abstraction upon the extra_select clausule.
  

django/db/models/sql/subqueries.py:

   372      def add_date_select(self, field, lookup_type, order='ASC'):
   373          """
   374          Converts the query into a date extraction query.
   375          """
   376          result = self.setup_joins([field.name], self.get_meta(),
   377                  self.get_initial_alias(), False)
   378          alias = result[3][-1]
   379          select = Date((alias, field.column), lookup_type,
   380                  self.connection.ops.date_trunc_sql)
   381          self.select = [select]
   382          self.select_fields = [None]
   383          self.select_related = False # See #7097.
   384          self.extra_select = {}
   385          self.distinct = True
   386          self.order_by = order == 'ASC' and [1] or [-1]

[Malcolm Tredinnick]

(In [9540]) Added a note to the SQLite database documentation mentioning that version 3.6.2
is to be avoided like the plague. Fortunately, it was only the latest release
for three weeks, so avoidance is easy (as is upgrading).

Refs #9358.

[Malcolm Tredinnick]

(In [9542]) [1.0.X] Added a note to the SQLite database documentation mentioning that
version 3.6.2 is to be avoided like the plague. Fortunately, it was only the
latest release for three weeks, so avoidance is easy (as is upgrading).

Refs #9358.

Backport of r9540 from trunk.

[Malcolm Tredinnick]

In view of the fact that this only affects one release of SQLite and a fixed version was brought out specifically for this bug three weeks later, I'm happy with just documenting around that version of SQLite. The current code would require some reasonable upheaval to avoid this bug in the upstream code just for that one version in one backend. It's not worth it at the moment.

[(none)]

Milestone post-1.0 deleted