[Django] #23940: problem when i access related object
Django
----------------------------------------------+--------------------
Reporter: zhiyajun11 | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23940>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: zhiyajun11 | Owner: nobody
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by zhiyajun11):
* cc: zhiyajun11 (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:1>
Django
Reporter: zhiyajun11 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by zhiyajun11):
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:2>
Django
Reporter: zhiyajun11 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by tchaumeny):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:3>
Django
Reporter: zhiyajun11 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by tchaumeny):
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 ?
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:4>
Django
Reporter: zhiyajun11 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by bmispelon):
Attached a testcase to reproduce the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:5>
Django
Reporter: zhiyajun11 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by kivikakk):
I'd be interested in trying to put together a patch that achieved this, if
there was interest!
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:6>
Django
Reporter: zhiyajun11 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
* cc: kivikakk (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:7>
Django
Reporter: zhiyajun11 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by zhiyajun11):
Replying to [comment:6 kivikakk]:
> I'd be interested in trying to put together a patch that achieved this,
if there was interest!
what you mean
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* version: 1.7 => master
* type: Bug => New feature
Comment:
I think a system check that gives a warning or error on fields named
`exact` would be appropriate.
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:9>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by nicwest):
* owner: nobody => nicwest
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: nicwest
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by nicwest):
I created a pull request here: https://github.com/django/django/pull/3831
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: nicwest
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by berkerpeksag):
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: nicwest
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by MarkusH):
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: nicwest
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by collinanderson):
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` and `isnull` also.
- Would it be possible to support having a field named "exact"?
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: nicwest
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by jarshwah):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: nicwest
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by MarkusH):
* cc: info+coding@… (added)
* stage: Ready for checkin => Accepted
Comment:
Delaying the patch until further investigation
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: nicwest
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* has_patch: 1 => 0
* type: New feature => Bug
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner:
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nicwest =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by jarshwah):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by charettes):
Removing the explicit `__exact` lookup
[https://github.com/django/django/pull/3969 seems to work].
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by charettes):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by charettes):
The ticket for tracking mode field names, lookups and transforms collision
is #24246.
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:24>
Django
-------------------------------------+-------------------------------------
| Charette <charette.s@…>
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: => Simon Charette <charette.s@…>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"eb4cdfbdd64a95b303eaaa40a070521aa58362fd"]:
{{{
#!CommitTicketReference repository=""
revision="eb4cdfbdd64a95b303eaaa40a070521aa58362fd"
Fixed #23940 -- Allowed model fields to be named `exact`.
An explicit `__exact` lookup in the related managers filters
was interpreted as a reference to a foreign `exact` field.
Thanks to Trac alias zhiyajun11 for the report, Josh for the
investigation,
Loïc for the test name and Tim for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: zhiyajun11 | Owner: Simon
| Charette <charette.s@…>
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette <charette.s@…>):
In [changeset:"a301061f88268255fc2660ca993a84b8bf12f9d2"]:
{{{
#!CommitTicketReference repository=""
revision="a301061f88268255fc2660ca993a84b8bf12f9d2"
[1.8.x] Fixed #23940 -- Allowed model fields to be named `exact`.
An explicit `__exact` lookup in the related managers filters
was interpreted as a reference to a foreign `exact` field.
Thanks to Trac alias zhiyajun11 for the report, Josh for the
investigation,
Loïc for the test name and Tim for the review.
Backport of eb4cdfbdd64a95b303eaaa40a070521aa58362fd from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23940#comment:26>