From 46209591fe250ffd002325b3110bbe8b34006b50 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 5 Dec 2018 14:06:32 +0100 Subject: [PATCH 1/4] Propagate a doc-value `format` with inner hits. (#36068) While sql passes a doc-value `format` when asking for doc-value fields, it doesn't do it when asking for fields via inner hits. --- .../querydsl/container/QueryContainer.java | 17 +++++++++---- .../xpack/sql/querydsl/query/BoolQuery.java | 6 ++--- .../xpack/sql/querydsl/query/LeafQuery.java | 2 +- .../xpack/sql/querydsl/query/NestedQuery.java | 22 ++++++++--------- .../xpack/sql/querydsl/query/NotQuery.java | 4 ++-- .../xpack/sql/querydsl/query/Query.java | 2 +- .../container/QueryContainerTests.java | 24 ++++++++++++------- .../sql/querydsl/query/BoolQueryTests.java | 10 +++++--- .../sql/querydsl/query/LeafQueryTests.java | 4 +++- .../sql/querydsl/query/NestedQueryTests.java | 24 +++++++++++-------- 10 files changed, 70 insertions(+), 45 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java index 5ef4689422b..8901ca75c8f 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.execution.search.FieldExtraction; import org.elasticsearch.xpack.sql.execution.search.SourceGenerator; @@ -29,8 +30,10 @@ import org.elasticsearch.xpack.sql.querydsl.query.MatchAll; import org.elasticsearch.xpack.sql.querydsl.query.NestedQuery; import org.elasticsearch.xpack.sql.querydsl.query.Query; import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.type.DataType; import java.io.IOException; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; @@ -180,8 +183,9 @@ public class QueryContainer { List nestedRefs = new ArrayList<>(); String name = aliasName(attr); + String format = attr.field().getDataType() == DataType.DATE ? "epoch_millis" : DocValueFieldsContext.USE_DEFAULT_FORMAT; Query q = rewriteToContainNestedField(query, attr.location(), - attr.nestedParent().name(), name, attr.field().isAggregatable()); + attr.nestedParent().name(), name, format, attr.field().isAggregatable()); SearchHitFieldRef nestedFieldRef = new SearchHitFieldRef(name, attr.field().getDataType(), attr.field().isAggregatable(), attr.parent().name()); @@ -190,11 +194,13 @@ public class QueryContainer { return new Tuple<>(new QueryContainer(q, aggs, columns, aliases, pseudoFunctions, scalarFunctions, sort, limit), nestedFieldRef); } - static Query rewriteToContainNestedField(@Nullable Query query, Location location, String path, String name, boolean hasDocValues) { + static Query rewriteToContainNestedField(@Nullable Query query, Location location, String path, String name, String format, + boolean hasDocValues) { if (query == null) { /* There is no query so we must add the nested query * ourselves to fetch the field. */ - return new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)); + return new NestedQuery(location, path, singletonMap(name, new AbstractMap.SimpleImmutableEntry<>(hasDocValues, format)), + new MatchAll(location)); } if (query.containsNestedField(path, name)) { // The query already has the nested field. Nothing to do. @@ -202,7 +208,7 @@ public class QueryContainer { } /* The query doesn't have the nested field so we have to ask * it to add it. */ - Query rewritten = query.addNestedField(path, name, hasDocValues); + Query rewritten = query.addNestedField(path, name, format, hasDocValues); if (rewritten != query) { /* It successfully added it so we can use the rewritten * query. */ @@ -210,7 +216,8 @@ public class QueryContainer { } /* There is no nested query with a matching path so we must * add the nested query ourselves just to fetch the field. */ - NestedQuery nested = new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)); + NestedQuery nested = new NestedQuery(location, path, + singletonMap(name, new AbstractMap.SimpleImmutableEntry<>(hasDocValues, format)), new MatchAll(location)); return new BoolQuery(location, true, query, nested); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java index 47075773b01..8b342b34c77 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java @@ -44,9 +44,9 @@ public class BoolQuery extends Query { } @Override - public Query addNestedField(String path, String field, boolean hasDocValues) { - Query rewrittenLeft = left.addNestedField(path, field, hasDocValues); - Query rewrittenRight = right.addNestedField(path, field, hasDocValues); + public Query addNestedField(String path, String field, String format, boolean hasDocValues) { + Query rewrittenLeft = left.addNestedField(path, field, format, hasDocValues); + Query rewrittenRight = right.addNestedField(path, field, format, hasDocValues); if (rewrittenLeft == left && rewrittenRight == right) { return this; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java index c66de04f550..4c0a5b4dcd6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java @@ -20,7 +20,7 @@ abstract class LeafQuery extends Query { } @Override - public Query addNestedField(String path, String field, boolean hasDocValues) { + public Query addNestedField(String path, String field, String format, boolean hasDocValues) { // No leaf queries are nested return this; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java index 03af1433f30..4224c2adc8a 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java @@ -5,11 +5,11 @@ */ package org.elasticsearch.xpack.sql.querydsl.query; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Objects; import org.apache.lucene.search.join.ScoreMode; @@ -37,14 +37,14 @@ public class NestedQuery extends Query { private static final List NO_STORED_FIELD = singletonList(StoredFieldsContext._NONE_); private final String path; - private final Map fields; + private final Map> fields; // field -> (useDocValues, format) private final Query child; public NestedQuery(Location location, String path, Query child) { this(location, path, emptyMap(), child); } - public NestedQuery(Location location, String path, Map fields, Query child) { + public NestedQuery(Location location, String path, Map> fields, Query child) { super(location); if (path == null) { throw new IllegalArgumentException("path is required"); @@ -68,10 +68,10 @@ public class NestedQuery extends Query { } @Override - public Query addNestedField(String path, String field, boolean hasDocValues) { + public Query addNestedField(String path, String field, String format, boolean hasDocValues) { if (false == this.path.equals(path)) { // I'm not at the right path so let my child query have a crack at it - Query rewrittenChild = child.addNestedField(path, field, hasDocValues); + Query rewrittenChild = child.addNestedField(path, field, format, hasDocValues); if (rewrittenChild == child) { return this; } @@ -81,9 +81,9 @@ public class NestedQuery extends Query { // I already have the field, no rewriting needed return this; } - Map newFields = new HashMap<>(fields.size() + 1); + Map> newFields = new HashMap<>(fields.size() + 1); newFields.putAll(fields); - newFields.put(field, hasDocValues); + newFields.put(field, new AbstractMap.SimpleImmutableEntry<>(hasDocValues, format)); return new NestedQuery(location(), path, unmodifiableMap(newFields), child); } @@ -113,9 +113,9 @@ public class NestedQuery extends Query { boolean noSourceNeeded = true; List sourceFields = new ArrayList<>(); - for (Entry entry : fields.entrySet()) { - if (entry.getValue()) { - ihb.addDocValueField(entry.getKey()); + for (Map.Entry> entry : fields.entrySet()) { + if (entry.getValue().getKey()) { + ihb.addDocValueField(entry.getKey(), entry.getValue().getValue()); } else { sourceFields.add(entry.getKey()); @@ -141,7 +141,7 @@ public class NestedQuery extends Query { return path; } - Map fields() { + Map> fields() { return fields; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java index d6464740c85..d44e62c230f 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java @@ -34,8 +34,8 @@ public class NotQuery extends Query { } @Override - public Query addNestedField(String path, String field, boolean hasDocValues) { - Query rewrittenChild = child.addNestedField(path, field, hasDocValues); + public Query addNestedField(String path, String field, String format, boolean hasDocValues) { + Query rewrittenChild = child.addNestedField(path, field, format, hasDocValues); if (child == rewrittenChild) { return this; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java index 057454b6fd8..7691ef42041 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java @@ -44,7 +44,7 @@ public abstract class Query { * @return a new query if we could add the nested field, the same query * instance otherwise */ - public abstract Query addNestedField(String path, String field, boolean hasDocValues); + public abstract Query addNestedField(String path, String field, String format, boolean hasDocValues); /** * Attach the one and only one matching nested query's filter to this diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java index f943e325cf7..84cb62a5ada 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java @@ -5,6 +5,9 @@ */ package org.elasticsearch.xpack.sql.querydsl.container; +import java.util.AbstractMap.SimpleImmutableEntry; + +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.sql.querydsl.query.BoolQuery; import org.elasticsearch.xpack.sql.querydsl.query.MatchAll; @@ -21,18 +24,21 @@ public class QueryContainerTests extends ESTestCase { private Location location = LocationTests.randomLocation(); private String path = randomAlphaOfLength(5); private String name = randomAlphaOfLength(5); + private String format = DocValueFieldsContext.USE_DEFAULT_FORMAT; private boolean hasDocValues = randomBoolean(); public void testRewriteToContainNestedFieldNoQuery() { - Query expected = new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)); - assertEquals(expected, QueryContainer.rewriteToContainNestedField(null, location, path, name, hasDocValues)); + Query expected = new NestedQuery(location, path, singletonMap(name, new SimpleImmutableEntry<>(hasDocValues, format)), + new MatchAll(location)); + assertEquals(expected, QueryContainer.rewriteToContainNestedField(null, location, path, name, format, hasDocValues)); } public void testRewriteToContainsNestedFieldWhenContainsNestedField() { Query original = new BoolQuery(location, true, - new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)), + new NestedQuery(location, path, singletonMap(name, new SimpleImmutableEntry<>(hasDocValues, format)), + new MatchAll(location)), new RangeQuery(location, randomAlphaOfLength(5), 0, randomBoolean(), 100, randomBoolean())); - assertSame(original, QueryContainer.rewriteToContainNestedField(original, location, path, name, randomBoolean())); + assertSame(original, QueryContainer.rewriteToContainNestedField(original, location, path, name, format, randomBoolean())); } public void testRewriteToContainsNestedFieldWhenCanAddNestedField() { @@ -41,16 +47,18 @@ public class QueryContainerTests extends ESTestCase { new NestedQuery(location, path, emptyMap(), new MatchAll(location)), buddy); Query expected = new BoolQuery(location, true, - new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)), + new NestedQuery(location, path, singletonMap(name, new SimpleImmutableEntry<>(hasDocValues, format)), + new MatchAll(location)), buddy); - assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, hasDocValues)); + assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, format, hasDocValues)); } public void testRewriteToContainsNestedFieldWhenDoesNotContainNestedFieldAndCantAdd() { Query original = new RangeQuery(location, randomAlphaOfLength(5), 0, randomBoolean(), 100, randomBoolean()); Query expected = new BoolQuery(location, true, original, - new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location))); - assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, hasDocValues)); + new NestedQuery(location, path, singletonMap(name, new SimpleImmutableEntry<>(hasDocValues, format)), + new MatchAll(location))); + assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, format, hasDocValues)); } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java index 76c582f236b..5837b069654 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.sql.querydsl.query; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.sort.NestedSortBuilder; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.sql.tree.Location; @@ -12,6 +13,7 @@ import org.elasticsearch.xpack.sql.tree.LocationTests; import java.util.Arrays; import java.util.List; +import java.util.AbstractMap.SimpleImmutableEntry; import java.util.function.Function; import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; @@ -50,14 +52,15 @@ public class BoolQueryTests extends ESTestCase { public void testAddNestedField() { Query q = boolQueryWithoutNestedChildren(); - assertSame(q, q.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), randomBoolean())); + assertSame(q, q.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), DocValueFieldsContext.USE_DEFAULT_FORMAT, + randomBoolean())); String path = randomAlphaOfLength(5); String field = randomAlphaOfLength(5); q = boolQueryWithNestedChildren(path, field); String newField = randomAlphaOfLength(5); boolean hasDocValues = randomBoolean(); - Query rewritten = q.addNestedField(path, newField, hasDocValues); + Query rewritten = q.addNestedField(path, newField, DocValueFieldsContext.USE_DEFAULT_FORMAT, hasDocValues); assertNotSame(q, rewritten); assertTrue(rewritten.containsNestedField(path, newField)); } @@ -83,7 +86,8 @@ public class BoolQueryTests extends ESTestCase { private Query boolQueryWithNestedChildren(String path, String field) { NestedQuery match = new NestedQuery(LocationTests.randomLocation(), path, - singletonMap(field, randomBoolean()), new MatchAll(LocationTests.randomLocation())); + singletonMap(field, new SimpleImmutableEntry<>(randomBoolean(), DocValueFieldsContext.USE_DEFAULT_FORMAT)), + new MatchAll(LocationTests.randomLocation())); Query matchAll = new MatchAll(LocationTests.randomLocation()); Query left; Query right; diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java index 232040b79bc..52043003ab5 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.querydsl.query; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.sort.NestedSortBuilder; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.sql.tree.Location; @@ -52,7 +53,8 @@ public class LeafQueryTests extends ESTestCase { public void testAddNestedField() { Query query = new DummyLeafQuery(LocationTests.randomLocation()); // Leaf queries don't contain nested fields. - assertSame(query, query.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), randomBoolean())); + assertSame(query, query.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), DocValueFieldsContext.USE_DEFAULT_FORMAT, + randomBoolean())); } public void testEnrichNestedSort() { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java index 7cd53bc73de..3e228954949 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java @@ -5,12 +5,14 @@ */ package org.elasticsearch.xpack.sql.querydsl.query; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.sort.NestedSortBuilder; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.tree.LocationTests; +import java.util.AbstractMap.SimpleImmutableEntry; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -38,11 +40,11 @@ public class NestedQueryTests extends ESTestCase { return new NestedQuery(LocationTests.randomLocation(), randomAlphaOfLength(5), randomFields(), randomQuery(depth)); } - private static Map randomFields() { + private static Map> randomFields() { int size = between(0, 5); - Map fields = new HashMap<>(size); + Map> fields = new HashMap<>(size); while (fields.size() < size) { - fields.put(randomAlphaOfLength(5), randomBoolean()); + fields.put(randomAlphaOfLength(5), new SimpleImmutableEntry<>(randomBoolean(), DocValueFieldsContext.USE_DEFAULT_FORMAT)); } return fields; } @@ -77,20 +79,20 @@ public class NestedQueryTests extends ESTestCase { NestedQuery q = randomNestedQuery(0); for (String field : q.fields().keySet()) { // add does nothing if the field is already there - assertSame(q, q.addNestedField(q.path(), field, randomBoolean())); + assertSame(q, q.addNestedField(q.path(), field, DocValueFieldsContext.USE_DEFAULT_FORMAT, randomBoolean())); String otherPath = randomValueOtherThan(q.path(), () -> randomAlphaOfLength(5)); // add does nothing if the path doesn't match - assertSame(q, q.addNestedField(otherPath, randomAlphaOfLength(5), randomBoolean())); + assertSame(q, q.addNestedField(otherPath, randomAlphaOfLength(5), DocValueFieldsContext.USE_DEFAULT_FORMAT, randomBoolean())); } // if the field isn't in the list then add rewrites to a query with all the old fields and the new one String newField = randomValueOtherThanMany(q.fields()::containsKey, () -> randomAlphaOfLength(5)); boolean hasDocValues = randomBoolean(); - NestedQuery added = (NestedQuery) q.addNestedField(q.path(), newField, hasDocValues); + NestedQuery added = (NestedQuery) q.addNestedField(q.path(), newField, DocValueFieldsContext.USE_DEFAULT_FORMAT, hasDocValues); assertNotSame(q, added); - assertThat(added.fields(), hasEntry(newField, hasDocValues)); + assertThat(added.fields(), hasEntry(newField, new SimpleImmutableEntry<>(hasDocValues, DocValueFieldsContext.USE_DEFAULT_FORMAT))); assertTrue(added.containsNestedField(q.path(), newField)); - for (Map.Entry field : q.fields().entrySet()) { + for (Map.Entry> field : q.fields().entrySet()) { assertThat(added.fields(), hasEntry(field.getKey(), field.getValue())); assertTrue(added.containsNestedField(q.path(), field.getKey())); } @@ -129,7 +131,9 @@ public class NestedQueryTests extends ESTestCase { } public void testToString() { - NestedQuery q = new NestedQuery(new Location(1, 1), "a.b", singletonMap("f", true), new MatchAll(new Location(1, 1))); - assertEquals("NestedQuery@1:2[a.b.{f=true}[MatchAll@1:2[]]]", q.toString()); + NestedQuery q = new NestedQuery(new Location(1, 1), "a.b", + singletonMap("f", new SimpleImmutableEntry<>(true, DocValueFieldsContext.USE_DEFAULT_FORMAT)), + new MatchAll(new Location(1, 1))); + assertEquals("NestedQuery@1:2[a.b.{f=true=use_field_mapping}[MatchAll@1:2[]]]", q.toString()); } } From 3d85e8cd0dab332b1635020cfe3b1079278ee64b Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 5 Dec 2018 14:12:38 +0100 Subject: [PATCH 2/4] [TEST] Use different name for remote cluster connection To avoid collisions with other tests. --- .../src/test/java/org/elasticsearch/client/CCRIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java index 2c63835af8c..b6fa146701d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java @@ -69,7 +69,7 @@ public class CCRIT extends ESRestHighLevelClientTestCase { String transportAddress = (String) nodesResponse.get("transport_address"); ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest(); - updateSettingsRequest.transientSettings(Collections.singletonMap("cluster.remote.local.seeds", transportAddress)); + updateSettingsRequest.transientSettings(Collections.singletonMap("cluster.remote.local_cluster.seeds", transportAddress)); ClusterUpdateSettingsResponse updateSettingsResponse = highLevelClient().cluster().putSettings(updateSettingsRequest, RequestOptions.DEFAULT); assertThat(updateSettingsResponse.isAcknowledged(), is(true)); @@ -77,7 +77,7 @@ public class CCRIT extends ESRestHighLevelClientTestCase { assertBusy(() -> { Map localConnection = (Map) toMap(client() .performRequest(new Request("GET", "/_remote/info"))) - .get("local"); + .get("local_cluster"); assertThat(localConnection, notNullValue()); assertThat(localConnection.get("connected"), is(true)); }); @@ -91,7 +91,7 @@ public class CCRIT extends ESRestHighLevelClientTestCase { CreateIndexResponse response = highLevelClient().indices().create(createIndexRequest, RequestOptions.DEFAULT); assertThat(response.isAcknowledged(), is(true)); - PutFollowRequest putFollowRequest = new PutFollowRequest("local", "leader", "follower"); + PutFollowRequest putFollowRequest = new PutFollowRequest("local_cluster", "leader", "follower"); PutFollowResponse putFollowResponse = execute(putFollowRequest, ccrClient::putFollow, ccrClient::putFollowAsync); assertThat(putFollowResponse.isFollowIndexCreated(), is(true)); assertThat(putFollowResponse.isFollowIndexShardsAcked(), is(true)); @@ -165,7 +165,7 @@ public class CCRIT extends ESRestHighLevelClientTestCase { public void testAutoFollowing() throws Exception { CcrClient ccrClient = highLevelClient().ccr(); PutAutoFollowPatternRequest putAutoFollowPatternRequest = - new PutAutoFollowPatternRequest("pattern1", "local", Collections.singletonList("logs-*")); + new PutAutoFollowPatternRequest("pattern1", "local_cluster", Collections.singletonList("logs-*")); putAutoFollowPatternRequest.setFollowIndexNamePattern("copy-{{leader_index}}"); AcknowledgedResponse putAutoFollowPatternResponse = execute(putAutoFollowPatternRequest, ccrClient::putAutoFollowPattern, ccrClient::putAutoFollowPatternAsync); From a3c1c6938a374dda7de0ad48ae35d3842c6805fe Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 5 Dec 2018 14:41:07 +0000 Subject: [PATCH 3/4] Mute ForecastIT.testSingleSeries Due to https://github.com/elastic/elasticsearch/issues/36258 --- .../java/org/elasticsearch/xpack/ml/integration/ForecastIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ForecastIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ForecastIT.java index 2d8c6a4128b..c80ec704820 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ForecastIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ForecastIT.java @@ -41,6 +41,7 @@ public class ForecastIT extends MlNativeAutodetectIntegTestCase { cleanUp(); } + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/36258") public void testSingleSeries() throws Exception { Detector.Builder detector = new Detector.Builder("mean", "value"); From 068c856e884b59ab9630a428845fe04d5e97c7e3 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 5 Dec 2018 08:11:47 -0700 Subject: [PATCH 4/4] Rename internal repository actions to be internal (#36244) This is a follow-up to #36086. It renames the internal repository actions to be prefixed by "internal". This allows the system user to execute the actions. Additionally, this PR stops casting Client to NodeClient. The client we have is a NodeClient so executing the actions will be local. --- .../src/main/java/org/elasticsearch/xpack/ccr/Ccr.java | 3 +-- .../elasticsearch/xpack/ccr/CcrRepositoryManager.java | 10 +++++----- .../DeleteInternalCcrRepositoryAction.java | 2 +- .../repositories/PutInternalCcrRepositoryAction.java | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index 6e09d29ba4f..8494f72be0a 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -10,7 +10,6 @@ import org.apache.lucene.util.SetOnce; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.Client; -import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; @@ -151,7 +150,7 @@ public class Ccr extends Plugin implements ActionPlugin, PersistentTaskPlugin, E return emptyList(); } - this.repositoryManager.set(new CcrRepositoryManager(settings, clusterService, (NodeClient) client)); + this.repositoryManager.set(new CcrRepositoryManager(settings, clusterService, client)); return Arrays.asList( ccrLicenseChecker, diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index f86789a880e..a1504ff2f8a 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -8,7 +8,7 @@ package org.elasticsearch.xpack.ccr; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.support.PlainActionFuture; -import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.client.Client; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.transport.RemoteClusterAware; @@ -22,9 +22,9 @@ import java.util.List; class CcrRepositoryManager extends RemoteClusterAware { - private final NodeClient client; + private final Client client; - CcrRepositoryManager(Settings settings, ClusterService clusterService, NodeClient client) { + CcrRepositoryManager(Settings settings, ClusterService clusterService, Client client) { super(settings); this.client = client; listenForUpdates(clusterService.getClusterSettings()); @@ -36,12 +36,12 @@ class CcrRepositoryManager extends RemoteClusterAware { if (addresses.isEmpty()) { DeleteInternalCcrRepositoryRequest request = new DeleteInternalCcrRepositoryRequest(repositoryName); PlainActionFuture f = PlainActionFuture.newFuture(); - client.executeLocally(DeleteInternalCcrRepositoryAction.INSTANCE, request, f); + client.execute(DeleteInternalCcrRepositoryAction.INSTANCE, request, f); assert f.isDone() : "Should be completed as it is executed synchronously"; } else { ActionRequest request = new PutInternalCcrRepositoryRequest(repositoryName, CcrRepository.TYPE); PlainActionFuture f = PlainActionFuture.newFuture(); - client.executeLocally(PutInternalCcrRepositoryAction.INSTANCE, request, f); + client.execute(PutInternalCcrRepositoryAction.INSTANCE, request, f); assert f.isDone() : "Should be completed as it is executed synchronously"; } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java index e85ce65858e..93d432fe93f 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java @@ -23,7 +23,7 @@ import java.io.IOException; public class DeleteInternalCcrRepositoryAction extends Action { public static final DeleteInternalCcrRepositoryAction INSTANCE = new DeleteInternalCcrRepositoryAction(); - public static final String NAME = "cluster:admin/ccr/internal_repository/delete"; + public static final String NAME = "internal:admin/ccr/internal_repository/delete"; private DeleteInternalCcrRepositoryAction() { super(NAME); diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java index 2d12cc4d77a..397137ffb49 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java @@ -23,7 +23,7 @@ import java.io.IOException; public class PutInternalCcrRepositoryAction extends Action { public static final PutInternalCcrRepositoryAction INSTANCE = new PutInternalCcrRepositoryAction(); - public static final String NAME = "cluster:admin/ccr/internal_repository/put"; + public static final String NAME = "internal:admin/ccr/internal_repository/put"; private PutInternalCcrRepositoryAction() { super(NAME);