Make `index` in TermsLookup mandatory (#25753)

This change removes the leniency of having a `null` index to fetch
terms from in 6.0 onwards. This feature will be deprecated in the 5.x series
and 6.0 nodes will require the index to be set.

Closes #25750

core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java

@@ -439,14 +439,6 @@ public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
     @Override
     protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) {
         if (this.termsLookup != null) {
-            TermsLookup termsLookup = new TermsLookup(this.termsLookup);
-            if (termsLookup.index() == null) { // TODO this should go away?
-                if (queryRewriteContext.getIndexSettings() != null) {
-                    termsLookup.index(queryRewriteContext.getIndexSettings().getIndex().getName());
-                } else {
-                    return this; // can't rewrite until we have index scope on the shard
-                }
-            }
             List<Object> values = fetch(termsLookup, queryRewriteContext.getClient());
             return new TermsQueryBuilder(this.fieldName, values);
         }

core/src/main/java/org/elasticsearch/indices/TermsLookup.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.indices;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -35,17 +36,12 @@ import java.util.Objects;
  * Encapsulates the parameters needed to fetch terms.
  */
 public class TermsLookup implements Writeable, ToXContent {
-    private String index;
+    private final String index;
     private final String type;
     private final String id;
     private final String path;
     private String routing;
 
-    public TermsLookup(TermsLookup copy) {
-        this(copy.index, copy.type, copy.id, copy.path);
-        this.routing = copy.routing;
-    }
-
     public TermsLookup(String index, String type, String id, String path) {
         if (id == null) {
             throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the id.");
@@ -56,6 +52,9 @@ public class TermsLookup implements Writeable, ToXContent {
         if (path == null) {
             throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the path.");
         }
+        if (index == null) {
+            throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the index.");
+        }
         this.index = index;
         this.type = type;
         this.id = id;
@@ -69,7 +68,14 @@ public class TermsLookup implements Writeable, ToXContent {
         type = in.readString();
         id = in.readString();
         path = in.readString();
-        index = in.readOptionalString();
+        if (in.getVersion().onOrAfter(Version.V_6_0_0_beta1)) {
+            index = in.readString();
+        } else {
+            index = in.readOptionalString();
+            if (index == null) {
+                throw new IllegalStateException("index must not be null in a terms lookup");
+            }
+        }
         routing = in.readOptionalString();
     }
 
@@ -78,7 +84,11 @@ public class TermsLookup implements Writeable, ToXContent {
         out.writeString(type);
         out.writeString(id);
         out.writeString(path);
-        out.writeOptionalString(index);
+        if (out.getVersion().onOrAfter(Version.V_6_0_0_beta1)) {
+            out.writeString(index);
+        } else {
+            out.writeOptionalString(index);
+        }
         out.writeOptionalString(routing);
     }
 
@@ -86,11 +96,6 @@ public class TermsLookup implements Writeable, ToXContent {
         return index;
     }
 
-    public TermsLookup index(String index) {
-        this.index = index;
-        return this;
-    }
-
     public String type() {
         return type;
     }
@@ -126,7 +131,7 @@ public class TermsLookup implements Writeable, ToXContent {
             } else if (token.isValue()) {
                 switch (currentFieldName) {
                 case "index":
-                    index = parser.textOrNull();
+                    index = parser.text();
                     break;
                 case "type":
                     type = parser.text();
@@ -159,9 +164,7 @@ public class TermsLookup implements Writeable, ToXContent {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-        if (index != null) {
-            builder.field("index", index);
-        }
+        builder.field("index", index);
         builder.field("type", type);
         builder.field("id", id);
         builder.field("path", path);

core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java

@@ -93,7 +93,7 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase<TermsQueryBuil
     }
 
     private TermsLookup randomTermsLookup() {
-        return new TermsLookup(randomBoolean() ? randomAlphaOfLength(10) : null, randomAlphaOfLength(10), randomAlphaOfLength(10),
+        return new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10),
                 termsPath).routing(randomBoolean() ? randomAlphaOfLength(10) : null);
     }
 

core/src/test/java/org/elasticsearch/indices/TermsLookupTests.java

@@ -47,19 +47,25 @@ public class TermsLookupTests extends ESTestCase {
         String type = randomAlphaOfLength(5);
         String id = randomAlphaOfLength(5);
         String path = randomAlphaOfLength(5);
-        switch (randomIntBetween(0, 2)) {
-        case 0:
-            type = null;
-            break;
-        case 1:
-            id = null;
-            break;
-        case 2:
-            path = null;
-            break;
+        String index = randomAlphaOfLength(5);
+        switch (randomIntBetween(0, 3)) {
+            case 0:
+                type = null;
+                break;
+            case 1:
+                id = null;
+                break;
+            case 2:
+                path = null;
+                break;
+            case 3:
+                index = null;
+                break;
+            default:
+                fail("unknown case");
         }
         try {
-            new TermsLookup(null, type, id, path);
+            new TermsLookup(index, type, id, path);
         } catch (IllegalArgumentException e) {
             assertThat(e.getMessage(), containsString("[terms] query lookup element requires specifying"));
         }
@@ -80,7 +86,7 @@ public class TermsLookupTests extends ESTestCase {
 
     public static TermsLookup randomTermsLookup() {
         return new TermsLookup(
-                randomBoolean() ? randomAlphaOfLength(10) : null,
+                randomAlphaOfLength(10),
                 randomAlphaOfLength(10),
                 randomAlphaOfLength(10),
                 randomAlphaOfLength(10).replace('.', '_')

docs/reference/migration/migrate_6_0/search.asciidoc

@@ -68,6 +68,10 @@
   use an explicit quoted query instead.
   If provided, it will be ignored and issue a deprecation warning.
 
+* The `index` parameter in the terms filter, used to look up terms in a dedicated index is
+  now mandatory. Previously, the index defaulted to the index the query was executed on. Now this index
+  must be explicitly set in the request.
+
 ==== Search shards API
 
 The search shards API no longer accepts the `type` url parameter, which didn't

docs/reference/query-dsl/terms-query.asciidoc

@@ -30,8 +30,7 @@ The terms lookup mechanism supports the following options:
 
 [horizontal]
 `index`::
-    The index to fetch the term values from. Defaults to the
-    current index.
+    The index to fetch the term values from.
 
 `type`::
     The type to fetch the term values from.