Open
Conversation
Previously, when validating that fields in a `filter` clause had a corresponding FilterField, `_process_filter` simply took the attname (e.g. `title` for `Character.objects.filter(novel__title="The Hobbit")`) and looked for it in the queryset model's `search_fields`, not accounting for the fact that it's a field from a different model. Fix this by introducing a new method `_get_filter_field_for_column` that considers the Col object of the filter in full, and descends into `RelatedFields` as needed. Fixing this is sufficient to make these lookups work on database backends; for Elasticsearch we still need to deal with translating this lookup into Elasticsearch query structure.
Rename `_get_filter_field_for_column` to `_get_filter_field_path_for_column`, make it track (and return) the full sequence of search fields rather than just the final entry, and make it responsible for raising FilterFieldError messages rather than returning None on not found. This allows error messages to be more specific about where the missing field is, rather than just giving the base model and final field name.
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
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.
Fixes #12
Fix the validation phase of
BaseSearchQueryCompiler._process_filterso that it correctly descends intoRelatedFieldsdefinitions when confirming that a field being filtered on has a correspondingFilterField. (Previously it only looked in thesearch_fieldsdefinition of the base model of the query, giving spurious "Please add index.FilterField" errors.) This is sufficient to make filtering by related fields work for the database search backends (because these work by annotating and filtering the original query, retaining all existing filtering logic).For Elasticsearch-based engines, we also need to rewrite the filter clause into Elasticsearch query syntax: where a filter on a field of the base model looks like:
the equivalent for a field on a related model is:
To achieve this, we revise the API of
_process_lookup(which is only implemented by the Elasticsearch backends) so that it receives the full chain ofRelatedFieldsdefinitions leading up to theFilterFieldbeing filtered on, and accepts amappingparameter so that it can work with the mapping of any model (rather than justself.mappingwhich is tied to the query's root model). We can then call_process_lookuprecursively to build the inner{"term": ...}clause, and wrap it in the{"nested": ...}clause.