Term query on _index field does not take aliases into account

[uschindler]

While trying to remove indicesQuery (deprecated by #17710) in my own code base, I noticed the following problem: The indicesQuery supports to pass any name of an index to the query, so also aliases work (of course it does not work with filtered aliases, doesn't it? - but that does not matter for the problem).

In my case I needed a query that returns results of a query for a specific index only (only alias name known, not its real name) and a matchAll query for the other indexes. Setup was:

• alias named "allMyStuff" with 3 indexes. Each of the three indexes have an internal, timestamped name, but there also exists an alias pointing to the most recent index. The client code only knows the alias, not the real index name.
• query executes on "allMyStaff" alias and filters the indices to query using indicesQuery.

This works with indicesQuery, specifiying the alias to only run the specific query on and return matchAll for all the others. With a term query on "_index" field, you cannot use the alias anymore. It only return documents if the index matches completely, so termQuery("_index", "alias") does not match anything.

So I don't agree with deprecating and removing indicesQuery unless this can be fixed.

[clintongormley]

Good point @uschindler - thanks

[uschindler]

I think the fix would be quite easy by using the same matching algorithm like indicesQuery does on the doToQuery() method:

@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
    if (context.matchesIndices(indices)) {
        return innerQuery.toQuery(context);
    }
    return noMatchQuery.toQuery(context);
}

I think, IndexFieldType should do something similar, here the example for termQuery method (similar in the other methods like termsQuery):

@Override
public Query termQuery(Object value, @Nullable QueryShardContext context) {
    if (context.matchesIndices(new String[] { value.toString() /*or some other conversion of the term*/))) {
        return Queries.newMatchAllQuery();
    } else {
        return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.index().getName() + " vs. " + value);
    }
}

I can try to provide a PR about this!

[uschindler]

There is still the question what to do with "filtered" aliases. Those may already broken with indicesQuery (not verified - if the filter of alias is applied afterwards, it would work). The "correct" way would be to apply the filter instead of matchAllDocs.

[uschindler]

...not so easy to fix because context.matchesIndices disappeared in master branch.

[javanna]

hey @uschindler you raise a good point which we have overlooked. The indices query bugged us for quite a while and we weren't sure how many people were using it. We need to fix the alias problem, but that doesn't require restoring the indices query. The fact that you'd need the matchesIndices method back for your fix means that part of the code removed should be restored, see 103984a . The method was removed as it was left unused after the indices query removal.

Let us discuss first though whether we come up with different ideas to address this.

[uschindler]

What was the problem with the indices query? Quite simple and straight-forward!

[sjmc7]

We were also using the indices query with aliases so would be good to see this fixed.

[carrino]

+1 Was using indices query with aliases, also. Ran into this when migrating to 6.0.0-rc1

[carrino]

In addition to the alias problem even if you pass all the indexes the alias could resolve to, I found another issue.

Before, the IndicesQuery would fully encapsulate the inner query and it would not get parsed on an index of the wrong type. This means you could do a query one index with some fields and another index with another set of fields wrapping each in an IndicesQuery.

If you convert this to a boolean with must clauses, all the boolean query is parsed even if the must index clause would fail. This parsing can lead to number_format_exceptions.

[uschindler]

Hi @carrino: I agree with you. I had the same problem. I don't fully understand why the Elasticsearch team deprecated the indices query. It is very useful and a "term query on the index field" is by far no replacement!!!

I'd suggest to unddo the deprecation and keep the query as is. Otherwise I have to write a plugin that restores the query, which is way stupid. The whole class is only a few lines of code and brings so much phantastic usage opportunities. And there is no reason to remove it from the perspective of the backend.

[carrino]

I was able to work around this limitation by changing bool query to short circuit if one of it's filter or must clauses finds a MatchNoDocsQuery. I don't see any other query types that can be used to encapsulate a query on an index besides boolean being modified in this way. Any other ideas?

diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/qu
index ac57c2a..11c714e 100644
--- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java
+++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java
@@ -23,6 +23,7 @@ import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.BooleanClause.Occur;
 import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.MatchAllDocsQuery;
+import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
@@ -376,10 +377,16 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> {
     @Override
     protected Query doToQuery(QueryShardContext context) throws IOException {
         BooleanQuery.Builder booleanQueryBuilder = new BooleanQuery.Builder();
-        addBooleanClauses(context, booleanQueryBuilder, mustClauses, BooleanClause.Occur.MUST);
+        if (!addBooleanClauses(context, booleanQueryBuilder, filterClauses, BooleanClause.Occur.FILTER)) {
+            return new MatchNoDocsQuery();
+        }
+
+        if (!addBooleanClauses(context, booleanQueryBuilder, mustClauses, BooleanClause.Occur.MUST)) {
+            return new MatchNoDocsQuery();
+        }
+
         addBooleanClauses(context, booleanQueryBuilder, mustNotClauses, BooleanClause.Occur.MUST_NOT);
         addBooleanClauses(context, booleanQueryBuilder, shouldClauses, BooleanClause.Occur.SHOULD);
-        addBooleanClauses(context, booleanQueryBuilder, filterClauses, BooleanClause.Occur.FILTER);
         BooleanQuery booleanQuery = booleanQueryBuilder.build();
         if (booleanQuery.clauses().isEmpty()) {
             return new MatchAllDocsQuery();
@@ -395,7 +402,7 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> {
         return adjustPureNegative ? fixNegativeQueryIfNeeded(query) : query;
     }
 
-    private static void addBooleanClauses(QueryShardContext context, BooleanQuery.Builder booleanQueryBuilder,
+    private static boolean addBooleanClauses(QueryShardContext context, BooleanQuery.Builder booleanQueryBuilder,
                                           List<QueryBuilder> clauses, Occur occurs) throws IOException {
         for (QueryBuilder query : clauses) {
             Query luceneQuery = null;
@@ -409,8 +416,12 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> {
                     luceneQuery = query.toFilter(context);
                     break;
             }
+            if ((occurs == Occur.FILTER || occurs == Occur.MUST) && luceneQuery.equals(new MatchNoDocsQuery())) {
+                return false;
+            }
             booleanQueryBuilder.add(new BooleanClause(luceneQuery, occurs));
         }
+        return true;
     }
 
     @Override