[Django] #21696: Allow usage of objects with add_to_query() in filter() calls
Django
----------------------------------------------+--------------------
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
There was a private method add_to_query() for objects in add_q(). Using
this it was possible for the objects to add themselves to the query's
where/having condition. This capability was removed from 1.6 as part of
ORM refactorings.
I have seen at least half a dozen complaints/questions about this removal.
So, clearly this feature was used more than I anticipated. It will be
relatively easy to add similar capability to 1.6, though the API needs to
change a little. This will be around 5 lines of code change with
practically no risk of regressions. So, I am going to add the capability
back unless somebody objects.
The API will likely be add_to_query(query, used_aliases, where_node) where
it was previously add_to_query(query, used_aliases). The where_node
addition is needed due to other changes in the ORM.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akaariai):
* needs_better_patch: => 0
* stage: Unreviewed => Ready for checkin
* needs_tests: => 0
* needs_docs: => 0
Comment:
A pull request implementing add_to_query() support here:
https://github.com/django/django/pull/2125. This needs release notes, but
is otherwise ready for checkin. I'll mark it so that this will not be
forgotten.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:1>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
Actually, we likely don't want to add release notes for this one.
add_to_query() was internal API and it will remain so. Hopefully we will
get official API for .filter(MyCustomFilterCondition(...)) in Django 1.8.
If that happens we will likely remove the add_to_query() hack again.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:2>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
I updated PR 2125 with some more code. Now using add_to_query() objects is
also possible and at least somewhat working in HAVING clauses, too.
Using the new version requires some changes to code, but it allows users
who relied on add_to_query() to use 1.6 and 1.7. Currently upgrading to
1.6 or 1.7 seems really hard as there doesn't seem to be any viable
upgrade path.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:3>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
For some strange reason I get these errors after test suite has ran
(likely on Python exit time):
OK (skipped=347, expected failures=8)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
Exception KeyError: (<weakref at 0xeeb7578; dead>,) in <function remove at
0x28f9848> ignored
Exception KeyError: (<weakref at 0xeeb7418; dead>,) in <function remove at
0x27ecb90> ignored
This happens with the patch only on py27. py33 seems to be unaffected, and
I don't get this error on master. I can't see anything in the patch that
should cause this. Another pair of eyes could help spot what is happening
here.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:4>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by loic84):
@akaariai I seem to get these weakref errors intermittently as well on the
current master, so it may not be your patch.
I suspect it has to do with the recent changes to bring Python 3.4 compat
on signals.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:5>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by mjtamlyn):
* stage: Ready for checkin => Accepted
Comment:
I'm also seeing those weakref teardown errors on this patch, but I've not
seen them otherwise. This might be resolved if the patch is brought up to
speed with master though.
As the patch does not apply cleanly, I'm going to remove the RFC flag.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:6>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(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: 0 |
-------------------------------------+-------------------------------------
Comment (by manfre):
I've gotten weakref errors at the end of the test suite at various points
in time without having applied this patch. I don't think the weakref
errors should hold back this case.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:7>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:8>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.6
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
Time to merge this has passed. We will hopefully get official support for
something similar in 1.8.
--
Ticket URL: <https://code.djangoproject.com/ticket/21696#comment:9>