diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/150_rewrite_on_coordinator.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/150_rewrite_on_coordinator.yml index be34e10ddcd74..77298cb4f61c3 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/150_rewrite_on_coordinator.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/150_rewrite_on_coordinator.yml @@ -39,7 +39,7 @@ search: rest_total_hits_as_int: true index: "search_index" - body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "type" : "_doc", "id": "1", "path": "followers"} } } } + body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "id": "1", "path": "followers"} } } } - do: indices.create: index: lookup_index @@ -64,7 +64,7 @@ search: rest_total_hits_as_int: true index: "search_index" - body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "type" : "_doc", "id": "1", "path": "followers"} } } } + body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "id": "1", "path": "followers"} } } } - match: { _shards.total: 5 } - match: { _shards.successful: 5 } diff --git a/server/src/internalClusterTest/java/org/opensearch/search/query/SearchQueryIT.java b/server/src/internalClusterTest/java/org/opensearch/search/query/SearchQueryIT.java index db87269c8ceae..c9bb746973226 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/query/SearchQueryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/query/SearchQueryIT.java @@ -1195,75 +1195,63 @@ public void testTermsLookupFilter() throws Exception { ); SearchResponse searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "type", "1", "terms"))) + .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "1", "terms"))) .get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "1", "3"); // same as above, just on the _id... - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("_id", new TermsLookup("lookup", "type", "1", "terms"))) - .get(); + searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("_id", new TermsLookup("lookup", "1", "terms"))).get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "1", "3"); // another search with same parameters... - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "type", "1", "terms"))) - .get(); + searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "1", "terms"))).get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "1", "3"); - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "type", "2", "terms"))) - .get(); + searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "2", "terms"))).get(); assertHitCount(searchResponse, 1L); assertFirstHit(searchResponse, hasId("2")); - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "type", "3", "terms"))) - .get(); + searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "3", "terms"))).get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "2", "4"); - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "type", "4", "terms"))) - .get(); + searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "4", "terms"))).get(); assertHitCount(searchResponse, 0L); searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "type", "1", "arr.term"))) + .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "1", "arr.term"))) .get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "1", "3"); searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "type", "2", "arr.term"))) + .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "2", "arr.term"))) .get(); assertHitCount(searchResponse, 1L); assertFirstHit(searchResponse, hasId("2")); searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "type", "3", "arr.term"))) + .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "3", "arr.term"))) .get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "2", "4"); searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("not_exists", new TermsLookup("lookup2", "type", "3", "arr.term"))) + .setQuery(termsLookupQuery("not_exists", new TermsLookup("lookup2", "3", "arr.term"))) .get(); assertHitCount(searchResponse, 0L); // index "lookup" type "type" id "missing" document does not exist: ignore the lookup terms searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "type", "missing", "terms"))) + .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "missing", "terms"))) .get(); assertHitCount(searchResponse, 0L); // index "lookup3" type "type" has the source disabled: ignore the lookup terms - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup3", "type", "1", "terms"))) - .get(); + searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup3", "1", "terms"))).get(); assertHitCount(searchResponse, 0L); } diff --git a/server/src/internalClusterTest/java/org/opensearch/validate/SimpleValidateQueryIT.java b/server/src/internalClusterTest/java/org/opensearch/validate/SimpleValidateQueryIT.java index 51d0a4395127a..46506e7c378b6 100644 --- a/server/src/internalClusterTest/java/org/opensearch/validate/SimpleValidateQueryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/validate/SimpleValidateQueryIT.java @@ -490,7 +490,7 @@ public void testExplainTermsQueryWithLookup() throws Exception { client().prepareIndex("twitter").setId("1").setSource("followers", new int[] { 1, 2, 3 }).get(); refresh(); - TermsQueryBuilder termsLookupQuery = QueryBuilders.termsLookupQuery("user", new TermsLookup("twitter", "_doc", "1", "followers")); + TermsQueryBuilder termsLookupQuery = QueryBuilders.termsLookupQuery("user", new TermsLookup("twitter", "1", "followers")); ValidateQueryResponse response = client().admin() .indices() .prepareValidateQuery("twitter") diff --git a/server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java index e797730ac0dff..ac29cb2cf5201 100644 --- a/server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java @@ -225,10 +225,6 @@ public TermsLookup termsLookup() { return this.termsLookup; } - public boolean isTypeless() { - return termsLookup == null || termsLookup.type() == null; - } - private static final Set> INTEGER_TYPES = new HashSet<>( Arrays.asList(Byte.class, Short.class, Integer.class, Long.class) ); diff --git a/server/src/main/java/org/opensearch/indices/TermsLookup.java b/server/src/main/java/org/opensearch/indices/TermsLookup.java index 1aa16ad5cd72c..bf6c024fa130e 100644 --- a/server/src/main/java/org/opensearch/indices/TermsLookup.java +++ b/server/src/main/java/org/opensearch/indices/TermsLookup.java @@ -32,8 +32,7 @@ package org.opensearch.indices; -import org.opensearch.LegacyESVersion; -import org.opensearch.common.Nullable; +import org.opensearch.Version; import org.opensearch.common.ParseField; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; @@ -42,13 +41,13 @@ import org.opensearch.common.xcontent.ToXContentFragment; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.index.mapper.MapperService; import org.opensearch.index.query.TermsQueryBuilder; import java.io.IOException; import java.util.Objects; import static org.opensearch.common.xcontent.ConstructingObjectParser.constructorArg; -import static org.opensearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; /** * Encapsulates the parameters needed to fetch terms. @@ -56,20 +55,11 @@ public class TermsLookup implements Writeable, ToXContentFragment { private final String index; - private @Nullable String type; private final String id; private final String path; private String routing; public TermsLookup(String index, String id, String path) { - this(index, null, id, path); - } - - /** - * @deprecated Types are in the process of being removed, use {@link TermsLookup(String, String, String)} instead. - */ - @Deprecated - 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."); } @@ -80,7 +70,6 @@ public TermsLookup(String index, String type, String id, String path) { throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the index."); } this.index = index; - this.type = type; this.id = id; this.path = path; } @@ -89,11 +78,8 @@ public TermsLookup(String index, String type, String id, String path) { * Read from a stream. */ public TermsLookup(StreamInput in) throws IOException { - if (in.getVersion().onOrAfter(LegacyESVersion.V_7_0_0)) { - type = in.readOptionalString(); - } else { - // Before 7.0, the type parameter was always non-null and serialized as a (non-optional) string. - type = in.readString(); + if (in.getVersion().before(Version.V_2_0_0)) { + in.readOptionalString(); } id = in.readString(); path = in.readString(); @@ -103,16 +89,8 @@ public TermsLookup(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { - if (out.getVersion().onOrAfter(LegacyESVersion.V_7_0_0)) { - out.writeOptionalString(type); - } else { - if (type == null) { - throw new IllegalArgumentException( - "Typeless [terms] lookup queries are not supported if any " + "node is running a version before 7.0." - ); - - } - out.writeString(type); + if (out.getVersion().before(Version.V_2_0_0)) { + out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME); } out.writeString(id); out.writeString(path); @@ -124,14 +102,6 @@ public String index() { return index; } - /** - * @deprecated Types are in the process of being removed. - */ - @Deprecated - public String type() { - return type; - } - public String id() { return id; } @@ -151,14 +121,12 @@ public TermsLookup routing(String routing) { private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("terms_lookup", args -> { String index = (String) args[0]; - String type = (String) args[1]; - String id = (String) args[2]; - String path = (String) args[3]; - return new TermsLookup(index, type, id, path); + String id = (String) args[1]; + String path = (String) args[2]; + return new TermsLookup(index, id, path); }); static { PARSER.declareString(constructorArg(), new ParseField("index")); - PARSER.declareString(optionalConstructorArg(), new ParseField("type").withAllDeprecated()); PARSER.declareString(constructorArg(), new ParseField("id")); PARSER.declareString(constructorArg(), new ParseField("path")); PARSER.declareString(TermsLookup::routing, new ParseField("routing")); @@ -170,19 +138,12 @@ public static TermsLookup parseTermsLookup(XContentParser parser) throws IOExcep @Override public String toString() { - if (type == null) { - return index + "/" + id + "/" + path; - } else { - return index + "/" + type + "/" + id + "/" + path; - } + return index + "/" + id + "/" + path; } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field("index", index); - if (type != null) { - builder.field("type", type); - } builder.field("id", id); builder.field("path", path); if (routing != null) { @@ -193,7 +154,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public int hashCode() { - return Objects.hash(index, type, id, path, routing); + return Objects.hash(index, id, path, routing); } @Override @@ -206,7 +167,6 @@ public boolean equals(Object obj) { } TermsLookup other = (TermsLookup) obj; return Objects.equals(index, other.index) - && Objects.equals(type, other.type) && Objects.equals(id, other.id) && Objects.equals(path, other.path) && Objects.equals(routing, other.routing); diff --git a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java index e37b4f1a1c39f..ea93d7a65b951 100644 --- a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java @@ -119,9 +119,7 @@ protected TermsQueryBuilder doCreateTestQueryBuilder() { private TermsLookup randomTermsLookup() { // Randomly choose between a typeless terms lookup and one with an explicit type to make sure we are - TermsLookup lookup = maybeIncludeType && randomBoolean() - ? new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath) - : new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath); + TermsLookup lookup = new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath); // testing both cases. lookup.routing(randomBoolean() ? randomAlphaOfLength(10) : null); return lookup; @@ -379,13 +377,6 @@ protected QueryBuilder parseQuery(XContentParser parser) throws IOException { try { QueryBuilder query = super.parseQuery(parser); assertThat(query, CoreMatchers.instanceOf(TermsQueryBuilder.class)); - - TermsQueryBuilder termsQuery = (TermsQueryBuilder) query; - String deprecationWarning = "Deprecated field [type] used, this field is unused and will be removed entirely"; - if (termsQuery.isTypeless() == false && !assertedWarnings.contains(deprecationWarning)) { - assertWarnings(deprecationWarning); - assertedWarnings.add(deprecationWarning); - } return query; } finally { diff --git a/server/src/test/java/org/opensearch/indices/TermsLookupTests.java b/server/src/test/java/org/opensearch/indices/TermsLookupTests.java index fb1462b500ea9..661995a22c507 100644 --- a/server/src/test/java/org/opensearch/indices/TermsLookupTests.java +++ b/server/src/test/java/org/opensearch/indices/TermsLookupTests.java @@ -45,42 +45,36 @@ public class TermsLookupTests extends OpenSearchTestCase { public void testTermsLookup() { String index = randomAlphaOfLengthBetween(1, 10); - String type = randomAlphaOfLengthBetween(1, 10); String id = randomAlphaOfLengthBetween(1, 10); String path = randomAlphaOfLengthBetween(1, 10); String routing = randomAlphaOfLengthBetween(1, 10); - TermsLookup termsLookup = new TermsLookup(index, type, id, path); + TermsLookup termsLookup = new TermsLookup(index, id, path); termsLookup.routing(routing); assertEquals(index, termsLookup.index()); - assertEquals(type, termsLookup.type()); assertEquals(id, termsLookup.id()); assertEquals(path, termsLookup.path()); assertEquals(routing, termsLookup.routing()); } public void testIllegalArguments() { - String type = randomAlphaOfLength(5); String id = randomAlphaOfLength(5); String path = randomAlphaOfLength(5); String index = randomAlphaOfLength(5); - switch (randomIntBetween(0, 3)) { + switch (randomIntBetween(0, 2)) { case 0: - type = null; - break; - case 1: id = null; break; - case 2: + case 1: path = null; break; - case 3: + case 2: index = null; break; default: fail("unknown case"); } try { - new TermsLookup(index, type, id, path); + new TermsLookup(index, id, path); } catch (IllegalArgumentException e) { assertThat(e.getMessage(), containsString("[terms] query lookup element requires specifying")); } @@ -99,35 +93,6 @@ public void testSerialization() throws IOException { } } - public void testSerializationWithTypes() throws IOException { - TermsLookup termsLookup = randomTermsLookupWithTypes(); - try (BytesStreamOutput output = new BytesStreamOutput()) { - termsLookup.writeTo(output); - try (StreamInput in = output.bytes().streamInput()) { - TermsLookup deserializedLookup = new TermsLookup(in); - assertEquals(deserializedLookup, termsLookup); - assertEquals(deserializedLookup.hashCode(), termsLookup.hashCode()); - assertNotSame(deserializedLookup, termsLookup); - } - } - } - - public void testXContentParsingWithType() throws IOException { - XContentParser parser = createParser( - JsonXContent.jsonXContent, - "{ \"index\" : \"index\", \"id\" : \"id\", \"type\" : \"type\", \"path\" : \"path\", \"routing\" : \"routing\" }" - ); - - TermsLookup tl = TermsLookup.parseTermsLookup(parser); - assertEquals("index", tl.index()); - assertEquals("type", tl.type()); - assertEquals("id", tl.id()); - assertEquals("path", tl.path()); - assertEquals("routing", tl.routing()); - - assertWarnings("Deprecated field [type] used, this field is unused and will be removed entirely"); - } - public void testXContentParsing() throws IOException { XContentParser parser = createParser( JsonXContent.jsonXContent, @@ -136,7 +101,6 @@ public void testXContentParsing() throws IOException { TermsLookup tl = TermsLookup.parseTermsLookup(parser); assertEquals("index", tl.index()); - assertNull(tl.type()); assertEquals("id", tl.id()); assertEquals("path", tl.path()); assertEquals("routing", tl.routing()); @@ -147,13 +111,4 @@ public static TermsLookup randomTermsLookup() { randomBoolean() ? randomAlphaOfLength(10) : null ); } - - public static TermsLookup randomTermsLookupWithTypes() { - return new TermsLookup( - randomAlphaOfLength(10), - randomAlphaOfLength(10), - randomAlphaOfLength(10), - randomAlphaOfLength(10).replace('.', '_') - ).routing(randomBoolean() ? randomAlphaOfLength(10) : null); - } }