Fixed #36385 -- Simplified filtering in QuerySet. #19438
+92
−160
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37eddbc to
e618f01
Compare
…add_q(). This means the `can_reuse` name for `Q.add_q()` and `Q.build_filter()` follows through.
This is unnecessary complexity as we can already handle this by manually constructing a `Q()` object if the value is a `dict`, or we can simply call `.filter(**limit_choices_to)`. Another undocumented function instead of using the available public API which is perfectly sufficient makes this all just a little bit harder to understand.
…xclude(). Instead of having to pass around the deconstructed parts, construct the `Q()` object as early as possible. This allows removal of the separate `_filter_or_exclude()` and `_filter_or_exclude_inplace()` functions in favour of a simple `_filter_q()` which takes a `Q()` object directly.
Added a `defer` argument to `QuerySet._filter_q()` instead. As we're immediately performing a `.filter()` call, make use of the underlying internal `._filter_q()` method and pass a flag directly.
…sticky(). Added a `sticky` argument to `QuerySet._filter_q()` instead. As we're immediately performing a `.filter()` call, make use of the underlying internal `._filter_q()` method and pass a flag directly. When the flag is passed, we can simply set `filter_is_sticky` on the query object after cloning the queryset.
After earlier changes this is only calling `._clone()`, so remove it.
There is no need for these to be different and it just makes the usage of them inconsistent. It was also harder to understand what the value of `Query.used_aliases` was because the `.clone()` would set the value to what would be expected when `filter_is_sticky` was set which is the rare case and this was then undone in `.chain()` when the flag was not set (the common case) which would make `used_aliases` an empty set.
We pass `defer=True` in two places and `sticky=True` in one of those places. That means that in the non-deferred case we don't need to handle `sticky=True`. And given the deferred filter handling - when `.add_q()` is called will happen when `.query` is next accessed - and therefore before `Query.chain()` gets called, we can push down assigning `True` to `.filter_is_sticky` till then. Doing this will mean that it also applies to the other place that we call `._filter_q()` with `defer=True`, but that doesn't seem to be an issue - all tests passed. The handling of `.filter_is_sticky` has also been moved to `.clone()` from `.chain()` which makes it a bit clearer and will allow us to simplify further.
e618f01 to
9b28654
Compare
QuerySet.QuerySet.
QuerySet.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trac ticket number
ticket-36385
Branch description
Some of the code for filtering
QuerySetis overly complex which makes it hard to follow:QuerySet._clone()vsQuerySet._chain()andQuery.clone()vsQuery.chain()Queryset.complex_filter()which exists for a specific case but can be handled using public APIs_defer_next_filterand_sticky_filterthat affect the next call toQuerySet.filter()Q()as early as possibleklassargument toQuery.chain()I had hoped to eliminate
Query.filter_is_stickyand pass theused_aliasestoQuery.add_q(), but I think that's a non-starter as it has to apply after the next clone. (Hard to get my head around it!)Checklist
mainbranch.