Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite queries on unmapped fields as MatchNoDocsQueryBuilder #49254

Closed
jpountz opened this issue Nov 18, 2019 · 6 comments · Fixed by #52486
Closed

Rewrite queries on unmapped fields as MatchNoDocsQueryBuilder #49254

jpountz opened this issue Nov 18, 2019 · 6 comments · Fixed by #52486
Labels
>enhancement good first issue low hanging fruit :Search/Search Search-related issues that do not fall into other categories

Comments

@jpountz
Copy link
Contributor

jpountz commented Nov 18, 2019

It's probably more easily explained with an example: TermQueryBuilber#doToQuery looks like this today

    protected Query doToQuery(QueryShardContext context) throws IOException {
        Query query = null;
        MappedFieldType mapper = context.fieldMapper(this.fieldName);
        if (mapper != null) {
            query = mapper.termQuery(this.value, context);
        }
        if (query == null) {
            query = new TermQuery(new Term(this.fieldName, BytesRefs.toBytesRef(this.value)));
        }
        return query;
    }

So if the field is unmapped, we create a TermQuery on a field that doesn't exist, which is no different from returning a MatchNoDocsQuery.

It's becoming more and more common that multiple indices that have different mappings get queried together, e.g. as part of an alias or an index pattern, so a more efficient way to handle these cases would be to rewrite to a MatchNoDocsQueryBuilder in the rewrite() method. This would help figure out that the shard cannot possibly match in the can_match phase and skip the query phase completely.

@jpountz jpountz added >enhancement :Search/Search Search-related issues that do not fall into other categories labels Nov 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@jpountz jpountz added the good first issue low hanging fruit label Nov 18, 2019
@ejmiers
Copy link
Contributor

ejmiers commented Nov 18, 2019

Could I jump on this issue? Fairly new to elasticsearch.

@jpountz
Copy link
Contributor Author

jpountz commented Nov 18, 2019

@ejmiers Sure, can you only focus on TermQueryBuilder for the first iteration of your PR, add me as a reviewer, and once I'm good with the approach for TermQueryBuilder we will generalize to other queries that need the same change like TermsQueryBuilder, MatchQueryBuilder and MultiMatchQueryBuilder?

@ejmiers
Copy link
Contributor

ejmiers commented Nov 29, 2019

Hi @jpountz, I am having difficulty locating the rewrite() function referenced above. Could you point me in the right direction?

@jpountz
Copy link
Contributor Author

jpountz commented Dec 3, 2019

Apologies @ejmiers I had to work on this as part of another change, so this issue should soon no longer be relevant.

@singhprincejeet
Copy link

@jpountz Should this issue be closed then?

jpountz added a commit that referenced this issue Feb 26, 2020
#49713 aims at introducing a new constant_keyword field which, like _index, always rewrites queries to a MatchAllQueryBuilder or a MatchNoneQueryBuilder in order to skip shards in the can_match phase. This change introduces a new ConstantFieldType marker class that helps get this functionality with any field and not just _index.

Since this change refactors rewrites, I also took it as an opportunity to adrress #49254: instead of returning the same queries you would get on a keyword field when a field is unmapped, queries get rewritten to a MatchNoDocsQueryBuilder.

This change exposed a couple bugs, like the fact that the percolator doesn't rewrite queries at query time, or that the significant_terms aggregation doesn't rewrite its inner filter, which I fixed.

Closes #49254
jpountz added a commit to jpountz/elasticsearch that referenced this issue Feb 26, 2020
…ic#52486)

Since this change refactors rewrites, I also took it as an opportunity to adrress elastic#49254: instead of returning the same queries you would get on a keyword field when a field is unmapped, queries get rewritten to a MatchNoDocsQueryBuilder.

This change exposed a couple bugs, like the fact that the percolator doesn't rewrite queries at query time, or that the significant_terms aggregation doesn't rewrite its inner filter, which I fixed.

Closes elastic#49254
jpountz added a commit that referenced this issue Feb 26, 2020
Generalize how queries on `_index` are handled at rewrite time (#52486)

Since this change refactors rewrites, I also took it as an opportunity to adrress #49254: instead of returning the same queries you would get on a keyword field when a field is unmapped, queries get rewritten to a MatchNoDocsQueryBuilder.

This change exposed a couple bugs, like the fact that the percolator doesn't rewrite queries at query time, or that the significant_terms aggregation doesn't rewrite its inner filter, which I fixed.

Closes #49254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement good first issue low hanging fruit :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants