Code

Opened 4 months ago

Last modified 4 weeks ago

#21696 new Bug

Allow usage of objects with add_to_query() in filter() calls

Reported by: akaariai Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Attachments (0)

Change History (7)

comment:1 Changed 4 months ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

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.

comment:2 Changed 3 months ago 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.

comment:3 Changed 3 months ago 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.

comment:4 Changed 3 months ago 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.

comment:5 Changed 3 months ago 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.

comment:6 Changed 2 months ago by mjtamlyn

  • Triage Stage changed from Ready for checkin to Accepted

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.

comment:7 Changed 4 weeks ago 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.