#8076 closed (fixed)
get_previous(next)_by_foo problem with parent's datetime field
Reported by: | Owned by: | jan_oberst | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Keywords: | 1.0-blocker | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
model_a has datetime field as t
model_b inherited model_a
when calling modelb.get_next(or previous)_by_t, it was failed.
Attachments (3)
Change History (13)
comment:1 by , 16 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 16 years ago
milestone: | → 1.0 |
---|---|
Resolution: | invalid |
Status: | closed → reopened |
Triage Stage: | Unreviewed → Accepted |
My mistake, this is a valid bug :) Or :( depending on how you look at it.
by , 16 years ago
Attachment: | 8076_get_prevnext_by_foo.diff added |
---|
Save, with '.diff' extension for proper highlighting...
comment:3 by , 16 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Patch needs improvement: | set |
Status: | reopened → new |
Turns out fields/related.py only checks for 'exact', 'in' or 'isnull' lookup_types, but returns a type error otherwise. get_previous_by_FOO and get_next_by_FOO have 'gt' and 'lt' lookup_types respectively.
Leaving this as 'Patch needs improvement' as it only addresses this specific examples; are there other lookup_types that might be sent to related?
comment:4 by , 16 years ago
Owner: | changed from | to
---|
comment:5 by , 16 years ago
Owner: | changed from | to
---|
comment:6 by , 16 years ago
My hackish little patch doesn't solve the actual issue here which lies in primary key lookups on child models.
Django tries to get the first other object with a date that's greater than the current object's date. If there is none it tries to get the object with the same date but highest ID. That's why it calls pk_gt. Usually this just sorts objects of the called object's class by primary key. On inheriting classes it will try to order the related field (the pointer to the superclass). Since related fields don't like to be ordered it fails.
#3246 adds the possibility to do lookups on a RelatedField by passing an object instead of primary key. It also limits lookups on RelatedField to rather 'exact' ID queries. I guess it tries to distinguish from the different querytypes ('exact' and 'in') if it needs to get PKs from more than one object. If there's more than one the pk_trace function is called on every passed object. The resulting list of primary keys is then used to get the objects with the WHERE IN statement.
When called with lt/gt we only have one object (or primary key), so we do not have to call the pk_trace more than once. I don't see why queries for 'lt','gt' shouldn't work.
On the other hand querying on primary keys of related fields isn't really straightforward. The API could behave in a way one wouldn't necessarily expect. Here's just one arbitrary example for this:
# Return all publications that have articles with IDs smaller than 3 Publications.objects.filter(article__id__lt=3) # This is prohibited by the 'Related Field has invalid lookup' block, but would do exactly the same: Publications.objects.filter(article__lt=3) # Now let's have the Article class ordered via model Meta: class Meta: ordering = ('-pub_date') # Now one might think that the following two statements do the same: Publications.objects.filter(article__lt=datetime.now()) Publications.objects.filter(article__pub_date__lt=datetime.now())
by , 16 years ago
Attachment: | patch_for_getnext_with_subclasstests.diff added |
---|
Added tests for get_next/previous_FIELD / fixed by adding an additional check if the model is a subclass
comment:7 by , 16 years ago
Keywords: | 1.0-blocker added |
---|
comment:8 by , 16 years ago
I don't think it's clear what the exact problem is here. My patch fixes the specific issue (as far as my testing goes), but Jan's goes a completely different route.
I can't really follow what his does, but I'm assuming mine fixes a symptom while he's aiming at the cause.
Maybe I just don't get it (actually, I don't), but perhaps a clearer bug description would be helpful to move this one forward?
comment:9 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
get_next_by_FOO only works for a field FOO on the current model.
In your example:
If Model_B has a ForeignKey relation to Model_A, do a get_next_by_t on Model_A, and then loop through Model_A's model_b_set. That way you'll get all the Model_Bs ordered by Model_A's datetime.