Opened 14 years ago

Closed 14 years ago

#13935 closed (fixed)

QuerySet .dates() method should span relationships

Reported by: coleifer Owned by: Valentin Golev
Component: Database layer (models, ORM) Version: 1.2
Severity: Keywords:
Cc: ego@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In the current implementation, QuerySet.dates() cannot span relationships. This limits the date-based generic views to only working if the date_field is on the same model as your queryset. The actual object lookup for the date-based generic views *works* - it is when the generic view tries to build the date_list that it fails.

# here's a silly example of what I'm talking about
>>> from django.contrib.auth.models import Group, User
>>> User.objects.dates('last_login', 'year')
[datetime.datetime(2009, 1, 1, 0, 0), datetime.datetime(2010, 1, 1, 0, 0)]

>>> Group.objects.dates('user__last_login', 'year')
FieldDoesNotExist: Group has no field named 'user__last_login'

The error is raised by this call:
http://code.djangoproject.com/browser/django/trunk/django/db/models/query.py#L991

I suspect this could be tweaked by changing something around here:
http://code.djangoproject.com/browser/django/trunk/django/db/models/sql/subqueries.py#L187

Attachments (2)

13935_tests.diff (3.4 KB ) - added by Valentin Golev 14 years ago.
tests
13935.diff (2.5 KB ) - added by Valentin Golev 14 years ago.
patch

Download all attachments as: .zip

Change History (10)

by Valentin Golev, 14 years ago

Attachment: 13935_tests.diff added

tests

comment:1 by Valentin Golev, 14 years ago

Owner: changed from nobody to Valentin Golev
Status: newassigned
Triage Stage: UnreviewedAccepted

I hope I'm doing everything right

comment:2 by Valentin Golev, 14 years ago

Has patch: set

I don't think it really needs any changes in documentation. Patch is ready.

I had to move the verification logic to the django.models.sql.subqueries.DateQuery, I'm not really sure if it was the right thing to do (but I have no better ideas and I see no harm here).

by Valentin Golev, 14 years ago

Attachment: 13935.diff added

patch

comment:3 by anonymous, 14 years ago

Cc: ego@… added

Unfortunately this patch doesn't work for Multi-Table Inheritance models, i.e. ones which have an implicit one-to-one link to a parent model.

eg:

class Post():
    date_posted = models.DateTimeField()

class BlogPost(Post):
    pass
>> BlogPost.objects.dates('date_posted', 'year')
(1054, "Unknown column 'myapp_blogpost.date_posted' in 'where clause'")

or trying a 'wrong' trick...

>> BlogPost.objects.dates('post__date_posted', 'year')
(1054, "Unknown column 'myapp_blogpost.post' in 'where clause'")

It seems like there should be some helper

comment:4 by anonymous, 14 years ago

"It seems like there should be some helper" ...I didn't finish that phrase.

What I meant to say is I have been trawling through the Django db classes to try and find... there must be some helper method somewhere which resolves the field names the 'right' way without having to write our own hack code to do so.

But so far I can't find it. I figure this ought to be easy for whoever wrote the relevant part of the ORM.

There's this bit of code on compiler.py > SQLCompiler.pre_sql_setup() that looks promising:

        if (not self.query.select and self.query.default_cols and not
                self.query.included_inherited_models):
            self.query.setup_inherited_models()

This will never happen for a DateQuery because of this, in subqueries.py > DateQuery.add_date_select():

    self.select = [select]

Unfortunately I don't know what most of this code really does, there's a bit too much going on to take it all in easily.

comment:5 by anonymous, 14 years ago

Sorry everybody, please ignore everything I just said above. It looked like I'd definitely ran into this bug, but in fact I was causing the problem with a bit of bad raw sql in my custom model manager.

The implicit one-to-one field on Multi-Table Inheritance models seems to work just fine for queryset.dates().

Sorry again for the noise.

comment:6 by Valentin Golev, 14 years ago

Anyway, I think we should add some tests for multi-table inheritance. I'll try to do it soon

comment:7 by va1en0k, 14 years ago

Triage Stage: AcceptedReady for checkin

I hope I'm not wrong

comment:8 by Alex Gaynor, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [14461]) Fixed #13935, added support for using QuerySet.dates across related fields. Thanks to valyagolev for his work on the patch.

Note: See TracTickets for help on using tickets.
Back to Top