Opened 5 years ago

Closed 4 years ago

#13935 closed (fixed)

QuerySet .dates() method should span relationships

Reported by: coleifer Owned by: valyagolev
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: UI/UX:

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 valyagolev 5 years ago.
tests
13935.diff (2.5 KB) - added by valyagolev 5 years ago.
patch

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by valyagolev

tests

comment:1 Changed 5 years ago by valyagolev

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to valyagolev
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

I hope I'm doing everything right

comment:2 Changed 5 years ago by valyagolev

  • 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).

Changed 5 years ago by valyagolev

patch

comment:3 Changed 5 years ago by anonymous

  • 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 Changed 5 years ago by anonymous

"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 Changed 5 years ago by anonymous

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 Changed 5 years ago by valyagolev

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

comment:7 Changed 4 years ago by va1en0k

  • Triage Stage changed from Accepted to Ready for checkin

I hope I'm not wrong

comment:8 Changed 4 years ago by Alex

  • Resolution set to fixed
  • Status changed from assigned to closed

(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