Re: [Django] #10414: select_related ignores unrecognized fields
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: trebor74hr
trebor74hr | Status: assigned
Type: Bug | Component: Database layer
Milestone: | (models, ORM)
Version: | Severity: Normal
Resolution: | Keywords: select_related
Triage Stage: Design | Has patch: 1
decision needed | Needs tests: 0
Needs documentation: 0 | Easy pickings: 0
Patch needs improvement: 0 |
-------------------------------------+-------------------------------------
Changes (by trebor74hr):
* cc: trebor74hr@… (added)
* easy: => 0
--
Ticket URL: <http://code.djangoproject.com/ticket/10414#comment:15>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | trebor74hr
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* ui_ux: => 0
* stage: Design decision needed => Accepted
Comment:
I've seen this bite people (including myself) many times. I don't think
silently dropping invalid code is acceptable.
Given that the only concern was about performance, and dates back to 5
years ago, I'm going to accept this ticket.
A quick benchmark to prove that the patch doesn't incur a performance hit
would be appreciated.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner:
Type: Bug | trebor74hr
Component: Database layer | Status: assigned
(models, ORM) | Version:
Severity: Normal | Resolution:
Keywords: select_related | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: timograham@… (added)
* needs_better_patch: 0 => 1
Comment:
I don't think the patch is correct as it causes several tests to fail,
including some in `select_related_onetoone` and `known_related_objects`.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner:
Type: Bug | trebor74hr
Component: Database layer | Status: assigned
(models, ORM) | Version:
Severity: Normal | Resolution:
Keywords: select_related | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anonymous):
Can anyone explain in plain simple words why fixing this issue could cause
some sort of performance problems? When you call select_related() without
any parameters django knows the structure of tables to be joined. Is it
that big performance problem to parse the string
'table__nexttable__blabla' and check it against this structure?
This issue can be annoying, when you use select_related to avoid
bombarding the DB with additional selects, and you make a small typo then
your code WILL actually bombard the db with these unnecessary selects and
you won't even know about it.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner:
Type: Bug | trebor74hr
Component: Database layer | Status: assigned
(models, ORM) | Version:
Severity: Normal | Resolution:
Keywords: select_related | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timo):
I think the lack of a patch that works is the main reason this isn't
fixed. After we have that we can evaluate any performance problems, but
like you (and I'm not an expert in the ORM), I feel they are likely to be
minor.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:19>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by nip3o):
* owner: trebor74hr => nip3o
Comment:
I will try to adopt this since there has been no activity for a while.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by nip3o):
* needs_better_patch: 1 => 0
Comment:
New patch.
https://github.com/django/django/pull/2986
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Keywords: select_related | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Keywords: select_related | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* stage: Ready for checkin => Accepted
Comment:
This still allows using `select_related` on fields other than relations
(e.g. a `Charfield`), IMO this needs a little more work.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
I have forgotten about this for quite a while, but the patch was updated
to support using non-relational fields quite a while ago (including tests
for this case, of course). I just updated the pull-request to the latest
version of master, so it should be ready to merge if all tests pass at the
CI server. Can I trig a CI build myself or can only core developers do
that?
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by charettes):
Only people on the white list can trigger builds. I just did for your PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* easy: 0 => 1
Comment:
I'll mark patch needs improvement as there needs to be tests for the
following cases:
- .select_related('tags') where tags is a GenericRelation
- .select_related('content_object') where content_object is a
GenericForeignKey()
- .select_related('m2m_field') where m2m_field is a ManyToManyField
- .select_related('reverse_foreign_key') where reverse_foreign_key is a
non-unique reverse side of a foreign key
These are easy additions to write to the test suite, so I'll mark this as
easy pickings (the whole patch isn't easy, but the needed additions to the
current patch are).
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* easy: 1 => 0
Comment:
The patch is updated with tests for the mentioned cases. I have never
worked with generic relations previously, but I hope that I managed to
implement the tests as intended anyway.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:28>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
You can bump the ticket to "ready for checkin" after addressing my
comments, thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:29>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Keywords: select_related | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by nip3o):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:30>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Type: Bug | Status: assigned
Component: Database layer | Version:
(models, ORM) | Resolution:
Keywords: select_related | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
There are some new comments on the pull request and also some test
failures.
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:31>
Django
-------------------------------------+-------------------------------------
Reporter: trebor74hr | Owner: nip3o
Component: Database layer | Version:
(models, ORM) |
Keywords: select_related | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3daa9d60be6672841ceed5bb812b6fb25950dc16"]:
{{{
#!CommitTicketReference repository=""
revision="3daa9d60be6672841ceed5bb812b6fb25950dc16"
Fixed #10414 -- Made select_related() fail on invalid field names.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/10414#comment:32>