Opened 17 years ago
Closed 11 years ago
#7312 closed Uncategorized (fixed)
QuerySet.complex_filter() chokes on custom Q objects (with add_to_query() method)
Reported by: | Johannes Dollinger | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | qsrf-cleanup |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
To reproduce, try:
class DoNothingQ: def add_to_query(self, query, aliases): pass MyModel.objects.complex_filter(DoNothingQ())
A side effect: My patch prevents builtin Q objects from being wrapped in another Q object when passed to complex_filter(). I'm not sure that's ok.
The cause ist the same as in http://groups.google.com/group/django-users/msg/3e6d910eacb8b542.
Attachments (2)
Change History (10)
by , 17 years ago
Attachment: | complex_filter_q.diff added |
---|
by , 17 years ago
Attachment: | 7312.regressiontest.diff added |
---|
comment:1 by , 17 years ago
Keywords: | qsrf-cleanup added |
---|
comment:2 by , 17 years ago
milestone: | → 1.0 |
---|
comment:3 by , 17 years ago
comment:4 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:6 by , 11 years ago
Easy pickings: | unset |
---|---|
Resolution: | fixed |
Severity: | → Normal |
Status: | closed → new |
Type: | → Uncategorized |
UI/UX: | unset |
Seems broken again in django 1.6.1
*** AttributeError: DoNothingQ instance has no attribute '__getitem__'
this happens b/c we lost in django.db.models.sql.query.Query.add_q method
if hasattr(q_object, 'add_to_query'): # Complex custom objects are responsible for adding themselves. q_object.add_to_query(self, used_aliases) else: ...
comment:7 by , 11 years ago
I'm not sure the example in the ticket is valid anymore. add_to_query
is not a documented API. Could you clarify? There seems to be a typo in your comment: "we lost in django.db.models.sql.query.Query.add_q method" -- I'm not sure what that means.
comment:8 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I created a separate ticket for this, see #21696.
The patch looks fine.
complex_filter()
is a bit of an ugly method, though, and definitely not part of the public API, so I'm going to leave out the test. We'll add tests when we fixlimit_choices_to
, but I'm not sure locking down the behaviour ofcomplex_filter()
just yet is the right thing to do. Non-internal code should really only be callingfilter()
andexclude()
(or calling.query.add_q()
directly, I guess).