New predicate functionality for Q objects

95 views
Skip to first unread message

ptone

unread,
Sep 22, 2012, 3:55:26 PM9/22/12
to django-d...@googlegroups.com
I've implemented predicate test like functionality for Q objects.


In brief, this lets you define a condition in a Q object and then test whether a model instance matches the condition.

I believe this to be a relatively complete patch, and would appreciate any review people can offer.

To be clear, a review of the documentation as a user is also helpful, so don't be shy ;-)

I'm hoping to land this for 1.5

Thanks,

-Preston

ptone

unread,
Sep 27, 2012, 1:51:39 PM9/27/12
to django-d...@googlegroups.com
So a number of issues have come up in the review of this feature that I'd like to summarize here.

The first boils down to feature basically being another case of an:


That is, the idea of a lookup matching against an instance in memory, will never completely mimic the behavior of that lookup when generating a DB query through the ORM.  It is a situation where most of the common use cases can probably be handled - but there are probably a number of edge cases lurking that would add some degree of maintenance burden were this feature to land.

The second issue is that this adds more code that relies on a currently imperfect lookup system.  Current query_terms, and now, for predicates, matching_functions live as part of the query object - a non-public object that, in part, determines what is a valid lookup.  This all could be refactored to be more associated with the fields themselves.  This is mostly an internal issue, as I feel the feature could always be refactored to take advantage of improvements to the internals without changing the public API of the predicate feature.  While explicit manager passing would probably no longer be required with improved internals - it could still be supported/ignored, and the explicit use of a manager is already in and of itself a pretty uncommon edge case requirement in the current implementation.

For a ticket that tracks the lookup refactoring ideas, see: https://code.djangoproject.com/ticket/16187

Finally - along with the ORM mismatch issue, there exists some potential for abuse of this feature that would result in very poor performance.  This is a case of: can the documentation make this clear enough that reasonable people won't shoot themselves in the foot. I'm not overly concerned about giving people enough rope to hurt themselves - but even a reasonable dev not doing any profiling could find themselves using this feature with either large arrays of instances, or with extensive conditions that follow deep relationships where the ORM and querysets would do a better job leveraging your database.  Exactly where that line exists depends on too many factors to lay out out clearly in documentation.

So that is an update on where this feature stands

If it does not make it into 1.5 - I can probably factor out most of the improvements back into the external django-predicate package, with a couple somewhat ugly hacks to support the GIS related matches.  There was some brief debate on IRC about how Django can best unearth edge case bugs in features that are candidates for inclusion into core, without the exposure that being in core offers.

Thanks to everyone who has participated in the discussion so far - most of which is occurring on the PR, not the ticket:


-Preston
Reply all
Reply to author
Forward
0 new messages