#23940 closed Bug (fixed)
ORM should allow model fields named "exact"
Reported by: | zhiyajun11 | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | zhiyajun11, Yuki Izumi, info+coding@… | 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
Hi guys, when i try to access the related model of a model who has a field named exact by related-manager, a 'exact=true' limit will be add to the sql automatically. As show below:
My models is:
import datetime from django.db import models from django.utils import timezone # Create your models here. class Question(models.Model): question_text = models.CharField(max_length=200) #pub_date = models.DateTimeField('date published') pub_date = models.IntegerField() exact = models.NullBooleanField() #exact = models.IntegerField(default=0) def was_published_recently(self): return self.pub_date >= timezone.now() - datetime.timedelta(days=1) class Choice(models.Model): question = models.ForeignKey(Question, related_name = 'choices') choice_text = models.CharField(max_length=200) votes = models.IntegerField(default=0)
The shell command is:
In [1]: from polls.models import Question In [2]: Question.objects.all() Out[2]: [<Question: Question object>, <Question: Question object>] In [3]: q = Question.objects.get(pk=1) In [4]: q Out[4]: <Question: Question object> In [5]: q.choices.all() Out[5]: [] In [6]: from django.db import connection In [7]: connection.queries[-1] Out[7]: {u'sql': u'QUERY = u'SELECT "polls_choice"."id", "polls_choice"."question_id", "polls_choice"."choice_text", "polls_choice"."votes" FROM "polls_choice" INNER JOIN "polls_question" ON ( "polls_choice"."question_id" = "polls_question"."id" ) WHERE "polls_question"."exact" = %s LIMIT 21' - PARAMS = (True,)', u'time': u'0.000'}
Ps: this problem also show up on django1.6.
Attachments (1)
Change History (27)
comment:1 by , 10 years ago
Cc: | added |
---|
comment:2 by , 10 years ago
Type: | Uncategorized → Bug |
---|
comment:3 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
follow-up: 8 comment:6 by , 10 years ago
I'd be interested in trying to put together a patch that achieved this, if there was interest!
comment:7 by , 10 years ago
Cc: | added |
---|
comment:8 by , 10 years ago
Replying to kivikakk:
I'd be interested in trying to put together a patch that achieved this, if there was interest!
what you mean
comment:9 by , 10 years ago
Summary: | problem when i access related object → Disallow/warn on fields named exact |
---|---|
Type: | Bug → New feature |
Version: | 1.7 → master |
I think a system check that gives a warning or error on fields named exact
would be appropriate.
comment:10 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:11 by , 10 years ago
I created a pull request here: https://github.com/django/django/pull/3831
comment:12 by , 10 years ago
Has patch: | set |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:13 by , 10 years ago
I was thinking about a more general solution that takes all registered lookups and transforms into account.
MarkusH | wouldn't it make more sense to raise errors if fields, transforms and/or lookups clash? collinanderson | MarkusH: no idea. i haven't messed around with custom lookups MarkusH | jarshwah_: if you are around ---^ collinanderson | MarkusH: right, i was thinking that, but it's only an issue for primary_key fields, right? collinanderson | MarkusH: or any field referenced by a foreignkey MarkusH | I think it's a general problem MarkusH | any field you can possibly reference via relations MarkusH | if you have a standalone model, no FK pointing to or from, you should be safe MarkusH | collinanderson: so, what do I do with the issue now? Do I set its triage state back to accepted after I dumped my thoughts there? collinanderson | MarkusH: hah. i'm totally not a triager-expert. collinanderson | MarkusH: but let's say you have book with fk->author collinanderson | MarkusH: ohh, i get it. yeah MarkusH | ok collinanderson | MarkusH: ohh, see, you could do either book_author_id__gte=2 collinanderson | MarkusH: or book__author__id__gte=2 collinanderson | MarkusH: both of those would work with a field on author named "gte" collinanderson | right? MarkusH | yes collinanderson | and django doesn't really do that internally at all, but id _does_ do __exact a bunch collinanderson | right? (at least this ^^ is my assumption) MarkusH | but what about book__author__foo='bar' (where foo can be a field, transform or lookup) do? collinanderson | right, and in that case it should be a field, not a transform or lookup MarkusH | afaik, if no lookup is given, exact is assumed collinanderson | seems to me you if you wanted a transform/lookup in that case you could just do book__author__pk__foo='bar' collinanderson | or book__author_id__foo='bar' MarkusH | ahh, right MarkusH | thanks collinanderson | and maybe it's possible to change django so it allows for a field named "exact" in the same way MarkusH | hang on collinanderson | (i'm saying this all in theory, i haven't actually tried any of this :) MarkusH | the test case attached to the PR has a model with a field "exact" collinanderson | and i assume the bad news is it uses the lookup/transform instead of the field? MarkusH | ok. The PR is about preventing certain problems by not allowing them to happen MarkusH | it specializes the exact lookup MarkusH | it isn't about a way to find another way to access a field MarkusH | so, in practice, every lookup and transform should prevent a field of that name
comment:14 by , 10 years ago
Marcus and I were talking on IRC about this.
- Is this issue specific to exact? Are _all_ lookups a problem? Looking at the code it's probably an issue for
iexact
andisnull
also. - Would it be possible to support having a field named "exact"?
comment:15 by , 10 years ago
I think we should try to fix this rather than add a warning. It should be possible to first check if the "part" is a field and use that if so.
.filter('field__exact__exact')
For each part ("field", "exact", "exact"):
try: get_field(part) except: try: get_transform(part) except: get_lookup(part)
There's already similar code to this that first tries the transform then falls back to lookup. We should just need to introduce similar logic that first checks if a part is actually a field.
comment:16 by , 10 years ago
Cc: | added |
---|---|
Triage Stage: | Ready for checkin → Accepted |
Delaying the patch until further investigation
comment:17 by , 10 years ago
Easy pickings: | unset |
---|---|
Has patch: | unset |
Summary: | Disallow/warn on fields named exact → ORM should allow model fields named "exact" |
Type: | New feature → Bug |
comment:18 by , 10 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:19 by , 10 years ago
After doing some investigation with user phood on IRC, the problem seems local to fields called exact
only. We traced the problem to:
The related manager adds a filter called model__exact
to fetch the correct related record. Unfortunately, other parts of the code interpret this as targeting the field exact rather than an exact match for the model (usually pk).
Changing:
self.core_filters = {'%s__exact' % rel_field.name: instance} # old self.core_filters = {'%s__pk' % rel_field.name: instance.pk} # new
Fixes the this problem, but breaks the test: foreign_object.tests.MultiColumnFKTests.test_reverse_query_returns_correct_result
So, still some work to do here. If there are any multi column fk experts around, it'd be good to hear from you.
comment:21 by , 10 years ago
I suggest we open a new ticket to add field/lookup/transform collisions checks and add a test to my PR to make sure the reported issue doesn't regress.
Thoughts?
comment:22 by , 10 years ago
Has patch: | set |
---|
comment:23 by , 10 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:24 by , 10 years ago
The ticket for tracking mode field names, lookups and transforms collision is #24246.
comment:25 by , 10 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
There are several parts of the ORM that append
exact
at the end of a filter expression if no other lookup is given (like there https://github.com/django/django/blob/master/django/db/models/sql/query.py#L975).Should all lookup names be considered as reserved keywords ?