From 3fab741e1876bad2887496326a423edb3dc51b8d Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Thu, 13 Aug 2020 16:26:45 +0300 Subject: [PATCH 1/7] Integrate "fields" API into QL --- .../eql/execution/search/SourceGenerator.java | 11 +- .../container/FieldExtractorRegistry.java | 24 -- .../querydsl/container/SearchHitFieldRef.java | 6 +- .../ql/execution/search/QlSourceBuilder.java | 24 +- .../extractor/AbstractFieldHitExtractor.java | 163 +++++++++--- .../xpack/ql/querydsl/query/NestedQuery.java | 19 +- x-pack/plugin/sql/qa/mixed-node/build.gradle | 60 +++++ .../xpack/sql/qa/mixed_node/SqlSearchIT.java | 246 ++++++++++++++++++ .../sql/qa/single_node/CliExplainIT.java | 29 ++- .../xpack/sql/qa/FieldExtractorTestCase.java | 111 ++++---- .../xpack/sql/qa/rest/RestSqlTestCase.java | 16 +- .../src/main/resources/docs/geo.csv-spec | 2 +- .../src/main/resources/geo/geosql.csv-spec | 138 +++++----- .../sql/action/SqlQueryResponseTests.java | 3 +- .../xpack/sql/proto/ColumnInfo.java | 2 +- .../sql/action/SqlTranslateActionIT.java | 23 +- .../xpack/sql/execution/PlanExecutor.java | 6 +- .../xpack/sql/execution/search/Querier.java | 18 +- .../sql/execution/search/SourceGenerator.java | 3 - .../search/extractor/FieldHitExtractor.java | 3 +- .../xpack/sql/plugin/RestSqlQueryAction.java | 1 - .../sql/plugin/TransportSqlQueryAction.java | 63 ++++- .../querydsl/container/QueryContainer.java | 26 +- .../querydsl/container/SearchHitFieldRef.java | 6 +- .../search/SqlSourceBuilderTests.java | 27 +- .../extractor/FieldHitExtractorTests.java | 61 ++--- 26 files changed, 734 insertions(+), 357 deletions(-) create mode 100644 x-pack/plugin/sql/qa/mixed-node/build.gradle create mode 100644 x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/SourceGenerator.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/SourceGenerator.java index b783605216c7b..c19c22ecf2c01 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/SourceGenerator.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/SourceGenerator.java @@ -5,10 +5,8 @@ */ package org.elasticsearch.xpack.eql.execution.search; -import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; -import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.sort.FieldSortBuilder; import org.elasticsearch.search.sort.NestedSortBuilder; import org.elasticsearch.search.sort.ScriptSortBuilder.ScriptSortType; @@ -59,13 +57,8 @@ public static SearchSourceBuilder sourceBuilder(QueryContainer container, QueryB sorting(container, source); - // disable the source if there are no includes - if (source.fetchSource() == null || CollectionUtils.isEmpty(source.fetchSource().includes())) { - source.fetchSource(FetchSourceContext.DO_NOT_FETCH_SOURCE); - } else { - // use true to fetch only the needed bits from the source - source.fetchSource(true); - } + // disable the source, as we rely on "fields" API + source.fetchSource(false); if (container.limit() != null) { // add size and from diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/FieldExtractorRegistry.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/FieldExtractorRegistry.java index e57ddc3d63efa..05a20b5f20e5e 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/FieldExtractorRegistry.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/FieldExtractorRegistry.java @@ -12,7 +12,6 @@ import org.elasticsearch.xpack.ql.expression.Expressions; import org.elasticsearch.xpack.ql.expression.FieldAttribute; import org.elasticsearch.xpack.ql.expression.gen.pipeline.ConstantInput; -import org.elasticsearch.xpack.ql.type.DataTypes; import java.util.HashMap; import java.util.Map; @@ -49,29 +48,6 @@ private FieldExtraction topHitFieldExtractor(FieldAttribute fieldAttr) { FieldAttribute rootField = fieldAttr; StringBuilder fullFieldName = new StringBuilder(fieldAttr.field().getName()); - // Only if the field is not an alias (in which case it will be taken out from docvalue_fields if it's isAggregatable()), - // go up the tree of parents until a non-object (and non-nested) type of field is found and use that specific parent - // as the field to extract data from, from _source. We do it like this because sub-fields are not in the _source, only - // the root field to which those sub-fields belong to, are. Instead of "text_field.keyword_subfield" for _source extraction, - // we use "text_field", because there is no source for "keyword_subfield". - /* - * "text_field": { - * "type": "text", - * "fields": { - * "keyword_subfield": { - * "type": "keyword" - * } - * } - * } - */ - if (fieldAttr.field().isAlias() == false) { - while (actualField.parent() != null - && actualField.parent().field().getDataType() != DataTypes.OBJECT - && actualField.parent().field().getDataType() != DataTypes.NESTED - && actualField.field().getDataType().hasDocValues() == false) { - actualField = actualField.parent(); - } - } while (rootField.parent() != null) { fullFieldName.insert(0, ".").insert(0, rootField.parent().field().getName()); rootField = rootField.parent(); diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/SearchHitFieldRef.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/SearchHitFieldRef.java index 727df6a6047c6..67a3f012e03fb 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/SearchHitFieldRef.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/SearchHitFieldRef.java @@ -65,11 +65,7 @@ public void collectFields(QlSourceBuilder sourceBuilder) { if (hitName != null) { return; } - if (docValue) { - sourceBuilder.addDocField(name, format(dataType)); - } else { - sourceBuilder.addSourceField(name); - } + sourceBuilder.addFetchField(name, format(dataType)); } @Override diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java index 728711df05581..dcb4f517e755d 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.ql.execution.search; -import org.elasticsearch.common.Strings; import org.elasticsearch.script.Script; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.fetch.subphase.FieldAndFormat; @@ -22,8 +21,7 @@ */ public class QlSourceBuilder { // The LinkedHashMaps preserve the order of the fields in the response - private final Set sourceFields = new LinkedHashSet<>(); - private final Set docFields = new LinkedHashSet<>(); + private final Set fetchFields = new LinkedHashSet<>(); private final Map scriptFields = new LinkedHashMap<>(); boolean trackScores = false; @@ -39,17 +37,10 @@ public void trackScores() { } /** - * Retrieve the requested field from the {@code _source} of the document + * Retrieve the requested field using the "fields" API */ - public void addSourceField(String field) { - sourceFields.add(field); - } - - /** - * Retrieve the requested field from doc values (or fielddata) of the document - */ - public void addDocField(String field, String format) { - docFields.add(new FieldAndFormat(field, format)); + public void addFetchField(String field, String format) { + fetchFields.add(new FieldAndFormat(field, format)); } /** @@ -65,14 +56,11 @@ public void addScriptField(String name, Script script) { */ public void build(SearchSourceBuilder sourceBuilder) { sourceBuilder.trackScores(this.trackScores); - if (!sourceFields.isEmpty()) { - sourceBuilder.fetchSource(sourceFields.toArray(Strings.EMPTY_ARRAY), null); - } - docFields.forEach(field -> sourceBuilder.docValueField(field.field, field.format)); + fetchFields.forEach(field -> sourceBuilder.fetchField(new FieldAndFormat(field.field, field.format, null))); scriptFields.forEach(sourceBuilder::scriptField); } public boolean noSource() { - return sourceFields.isEmpty(); + return true; } } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java index b04f075545ea5..96af119a69599 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java @@ -9,6 +9,8 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.index.mapper.IgnoredFieldMapper; @@ -38,6 +40,7 @@ public abstract class AbstractFieldHitExtractor implements HitExtractor { private static final Version SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION = Version.V_7_4_0; + private static final Version SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API = Version.V_7_10_0; /** * Source extraction requires only the (relative) field name, without its parent path. @@ -54,6 +57,8 @@ private static String[] sourcePath(String name, boolean useDocValue, String hitN private final boolean useDocValue; private final boolean arrayLeniency; private final String[] path; + private final InternalHitExtractor hitExtractorDelegate; + protected final GeoPointParser geoPointParserDelegate; protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue) { this(name, null, dataType, zoneId, useDocValue, null, false); @@ -80,22 +85,37 @@ protected AbstractFieldHitExtractor(String name, String fullFieldName, DataType } this.path = sourcePath(fieldName, useDocValue, hitName); + hitExtractorDelegate = new FieldsApiFieldHitExtractor(); + geoPointParserDelegate = new FieldsApiGeoPointParser(); } protected AbstractFieldHitExtractor(StreamInput in) throws IOException { fieldName = in.readString(); - if (in.getVersion().onOrAfter(SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION)) { + if (in.getVersion().onOrAfter(SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION) + && in.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { fullFieldName = in.readOptionalString(); + hitExtractorDelegate = new SourceDocValuesFieldHitExtractor(); + geoPointParserDelegate = new SourceDocValuesGeoPointParser(); } else { fullFieldName = null; + hitExtractorDelegate = new FieldsApiFieldHitExtractor(); + geoPointParserDelegate = new FieldsApiGeoPointParser(); } String typeName = in.readOptionalString(); dataType = typeName != null ? loadTypeFromName(typeName) : null; - useDocValue = in.readBoolean(); + if (in.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { + useDocValue = in.readBoolean(); + } else { + useDocValue = false; // for "fields" API usage, extraction from _source or from docvalues doesn't matter + } hitName = in.readOptionalString(); arrayLeniency = in.readBoolean(); - path = sourcePath(fieldName, useDocValue, hitName); zoneId = readZoneId(in); + if (in.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { + path = sourcePath(fieldName, useDocValue, hitName); + } else { + path = null; + } } protected DataType loadTypeFromName(String typeName) { @@ -107,45 +127,52 @@ protected DataType loadTypeFromName(String typeName) { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(fieldName); - if (out.getVersion().onOrAfter(SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION)) { + if (out.getVersion().onOrAfter(SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION) + && out.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { out.writeOptionalString(fullFieldName); } out.writeOptionalString(dataType == null ? null : dataType.typeName()); - out.writeBoolean(useDocValue); + if (out.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { + out.writeBoolean(useDocValue); + } out.writeOptionalString(hitName); out.writeBoolean(arrayLeniency); } @Override public Object extract(SearchHit hit) { - Object value = null; - if (useDocValue) { - DocumentField field = hit.field(fieldName); - if (field != null) { - value = unwrapMultiValue(field.getValues()); - } + if (hitExtractorDelegate != null) { + return hitExtractorDelegate.extract(hit); } else { - // if the field was ignored because it was malformed and ignore_malformed was turned on - if (fullFieldName != null - && hit.getFields().containsKey(IgnoredFieldMapper.NAME) - && isFromDocValuesOnly(dataType) == false) { - /* - * We check here the presence of the field name (fullFieldName including the parent name) in the list - * of _ignored fields (due to malformed data, which was ignored). - * For example, in the case of a malformed number, a "byte" field with "ignore_malformed: true" - * with a "text" sub-field should return "null" for the "byte" parent field and the actual malformed - * data for the "text" sub-field. - */ - if (hit.getFields().get(IgnoredFieldMapper.NAME).getValues().contains(fullFieldName)) { - return null; + return new FieldsApiFieldHitExtractor().extract(hit); + } + } + + protected Object unwrapFieldsMultiValue(Object values) { + if (values == null) { + return null; + } + if (values instanceof List) { + List list = (List) values; + if (list.isEmpty()) { + return null; + } else { + if (isPrimitive(list) == false) { + if (list.size() == 1 || arrayLeniency) { + return unwrapFieldsMultiValue(list.get(0)); + } else { + throw new QlIllegalArgumentException("Arrays (returned by [{}]) are not supported", fieldName); + } } } - Map source = hit.getSourceAsMap(); - if (source != null) { - value = extractFromSource(source); - } } - return value; + + Object unwrapped = unwrapCustomValue(values); + if (unwrapped != null) { + return unwrapped; + } + + return values; } protected Object unwrapMultiValue(Object values) { @@ -336,4 +363,82 @@ public boolean equals(Object obj) { public int hashCode() { return Objects.hash(fieldName, useDocValue, hitName, arrayLeniency); } + + /* + * Logic specific to extraction preferably from _source when possible, falling back to extraction from doc_values otherwise. + * This has been introduced in 7.4.0 and not needed anymore starting with 7.10.0 + */ + private class SourceDocValuesFieldHitExtractor implements InternalHitExtractor { + public Object extract(SearchHit hit) { + Object value = null; + if (useDocValue) { + DocumentField field = hit.field(fieldName); + if (field != null) { + value = unwrapMultiValue(field.getValues()); + } + } else { + // if the field was ignored because it was malformed and ignore_malformed was turned on + if (fullFieldName != null + && hit.getFields().containsKey(IgnoredFieldMapper.NAME) + && isFromDocValuesOnly(dataType) == false + && dataType.isNumeric()) { + /* + * ignore_malformed makes sense for extraction from _source for numeric fields only. + * And we check here that the data type is actually a numeric one to rule out + * any non-numeric sub-fields (for which the "parent" field should actually be extracted from _source). + * For example, in the case of a malformed number, a "byte" field with "ignore_malformed: true" + * with a "text" sub-field should return "null" for the "byte" parent field and the actual malformed + * data for the "text" sub-field. Also, the _ignored section of the response contains the full field + * name, thus the need to do the comparison with that and not only the field name. + */ + if (hit.getFields().get(IgnoredFieldMapper.NAME).getValues().contains(fullFieldName)) { + return null; + } + } + Map source = hit.getSourceAsMap(); + if (source != null) { + value = extractFromSource(source); + } + } + return value; + } + } + + /* + * Logic specific to extraction from "fields" API (introduced in 7.10.0) + */ + private class FieldsApiFieldHitExtractor implements InternalHitExtractor { + public Object extract(SearchHit hit) { + Object value = null; + DocumentField field = hit.field(fieldName); + if (field != null) { + value = unwrapFieldsMultiValue(field.getValues()); + } + return value; + } + } + + private interface InternalHitExtractor { + Object extract(SearchHit hit); + } + + private class SourceDocValuesGeoPointParser implements GeoPointParser { + @Override + public GeoPoint parse(Object values) { + return GeoUtils.parseGeoPoint(values, true); + } + } + + private class FieldsApiGeoPointParser implements GeoPointParser { + @Override + public GeoPoint parse(Object values) { + @SuppressWarnings("unchecked") + Map map = (Map) values; + return GeoUtils.parseGeoPoint(map.get("coordinates"), true); + } + } + + public interface GeoPointParser { + GeoPoint parse(Object values); + } } \ No newline at end of file diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/querydsl/query/NestedQuery.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/querydsl/query/NestedQuery.java index 0ee7326977a44..8b6904a0f63dc 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/querydsl/query/NestedQuery.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/querydsl/query/NestedQuery.java @@ -15,7 +15,6 @@ import org.elasticsearch.xpack.ql.tree.Source; import java.util.AbstractMap; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -115,26 +114,16 @@ public QueryBuilder asBuilder() { ihb.setSize(MAX_INNER_HITS); ihb.setName(path + "_" + COUNTER++); - boolean noSourceNeeded = true; - List sourceFields = new ArrayList<>(); - for (Map.Entry> entry : fields.entrySet()) { if (entry.getValue().getKey()) { - ihb.addDocValueField(entry.getKey(), entry.getValue().getValue()); + ihb.addFetchField(entry.getKey(), entry.getValue().getValue()); } else { - sourceFields.add(entry.getKey()); - noSourceNeeded = false; + ihb.addFetchField(entry.getKey()); } } - - if (noSourceNeeded) { - ihb.setFetchSourceContext(FetchSourceContext.DO_NOT_FETCH_SOURCE); - ihb.setStoredFieldNames(NO_STORED_FIELD); - } - else { - ihb.setFetchSourceContext(new FetchSourceContext(true, sourceFields.toArray(new String[sourceFields.size()]), null)); - } + ihb.setFetchSourceContext(FetchSourceContext.DO_NOT_FETCH_SOURCE); + ihb.setStoredFieldNames(NO_STORED_FIELD); query.innerHit(ihb); } diff --git a/x-pack/plugin/sql/qa/mixed-node/build.gradle b/x-pack/plugin/sql/qa/mixed-node/build.gradle new file mode 100644 index 0000000000000..fded43d44955c --- /dev/null +++ b/x-pack/plugin/sql/qa/mixed-node/build.gradle @@ -0,0 +1,60 @@ +apply plugin: 'elasticsearch.testclusters' +apply plugin: 'elasticsearch.standalone-rest-test' +apply from : "$rootDir/gradle/bwc-test.gradle" +apply plugin: 'elasticsearch.rest-test' + +import org.elasticsearch.gradle.Version +import org.elasticsearch.gradle.VersionProperties +import org.elasticsearch.gradle.info.BuildParams +import org.elasticsearch.gradle.testclusters.StandaloneRestIntegTestTask + +dependencies { + testImplementation project(':x-pack:qa') +} + +tasks.named("integTest").configure{ enabled = false} + +for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it.onOrAfter('7.8.0') }) { + if (bwcVersion == VersionProperties.getElasticsearchVersion()) { + // Not really a mixed cluster + continue; + } + + String baseName = "v${bwcVersion}" + + testClusters { + "${baseName}" { + versions = [bwcVersion.toString(), project.version] + numberOfNodes = 3 + testDistribution = 'DEFAULT' + setting 'xpack.security.enabled', 'false' + setting 'xpack.watcher.enabled', 'false' + setting 'xpack.ml.enabled', 'false' + setting 'xpack.license.self_generated.type', 'trial' + } + } + + tasks.register("${baseName}#mixedClusterTest", StandaloneRestIntegTestTask) { + useCluster testClusters."${baseName}" + mustRunAfter("precommit") + doFirst { + // Getting the endpoints causes a wait for the cluster + println "Endpoints are: ${-> testClusters."${baseName}".allHttpSocketURI.join(",")}" + println "Upgrading one node to create a mixed cluster" + testClusters."${baseName}".nextNodeToNextVersion() + + println "Upgrade complete, endpoints are: ${-> testClusters."${baseName}".allHttpSocketURI.join(",")}" + nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}".allHttpSocketURI.join(",")}") + nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}") + } + onlyIf { project.bwc_tests_enabled } + } + + tasks.register(bwcTaskName(bwcVersion)) { + dependsOn "${baseName}#mixedClusterTest" + } + + tasks.named("check").configure { + dependsOn "${baseName}#mixedClusterTest" + } +} diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java new file mode 100644 index 0000000000000..73ac7394aeaa3 --- /dev/null +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java @@ -0,0 +1,246 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.qa.mixed_node; + +import org.apache.http.HttpHost; +import org.elasticsearch.Version; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.test.NotEqualMessageBuilder; +import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.test.rest.yaml.ObjectPath; +import org.junit.Before; + +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static java.util.Collections.singletonList; +import static java.util.Collections.unmodifiableMap; + +public class SqlSearchIT extends ESRestTestCase { + + private static String index = "test_sql_mixed_versions"; + private static int numShards; + private static int numReplicas = 1; + private static int numDocs; + private static Nodes nodes; + private static List allNodes; + private static List newNodes; + private static List bwcNodes; + private static Version bwcVersion; + private static Version newVersion; + private static Map expectedResponse; + + @Before + public void prepareTestData() throws IOException { + nodes = buildNodeAndVersions(client()); + numShards = nodes.size(); + numDocs = randomIntBetween(numShards, 16); + allNodes = new ArrayList<>(); + allNodes.addAll(nodes.getBWCNodes()); + allNodes.addAll(nodes.getNewNodes()); + newNodes = new ArrayList<>(); + newNodes.addAll(nodes.getNewNodes()); + bwcNodes = new ArrayList<>(); + bwcNodes.addAll(nodes.getBWCNodes()); + bwcVersion = nodes.getBWCNodes().get(0).getVersion(); + newVersion = nodes.getNewNodes().get(0).getVersion(); + + if (indexExists(index) == false) { + expectedResponse = new HashMap<>(); + expectedResponse.put("columns", singletonList(columnInfo("test", "text"))); + List> rows = new ArrayList<>(numDocs); + expectedResponse.put("rows", rows); + + createIndex( + index, + Settings.builder() + .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() + ); + for (int i = 0; i < numDocs; i++) { + String randomValue = "test_" + randomAlphaOfLength(3); + Request request = new Request("PUT", index + "/_doc/" + i); + request.setJsonEntity("{\"test\": \"" + randomValue + "\",\"id\":" + i + "}"); + assertOK(client().performRequest(request)); + rows.add(singletonList(randomValue)); + } + } + } + + public void testQueryToUpgradedNodes() throws Exception { + try ( + RestClient client = buildClient(restClientSettings(), newNodes.stream().map(Node::getPublishAddress).toArray(HttpHost[]::new)) + ) { + Request request = new Request("POST", "_sql"); + request.setJsonEntity( + "{\"mode\":\"jdbc\",\"version\":\"" + + newVersion.toString() + + "\",\"binary_format\":\"false\",\"query\":\"SELECT test FROM " + + index + + " ORDER BY id\"}" + ); + assertBusy(() -> { assertResponse(expectedResponse, runSql(client, request)); }); + } + } + + public void testQueryToOldNodes() throws Exception { + try ( + RestClient client = buildClient(restClientSettings(), bwcNodes.stream().map(Node::getPublishAddress).toArray(HttpHost[]::new)) + ) { + Request request = new Request("POST", "_sql"); + String versionSupport = bwcVersion.onOrAfter(Version.V_7_7_0) ? ",\"version\":\"" + newVersion.toString() + "\"" : ""; + String binaryFormatSupport = bwcVersion.onOrAfter(Version.V_7_6_0) ? ",\"binary_format\":\"false\"" : ""; + request.setJsonEntity( + "{\"mode\":\"jdbc\"" + versionSupport + binaryFormatSupport + ",\"query\":\"SELECT test FROM " + + index + + " ORDER BY id\"}" + ); + assertBusy(() -> { assertResponse(expectedResponse, runSql(client, request)); }); + } + } + + private Map columnInfo(String name, String type) { + Map column = new HashMap<>(); + column.put("name", name); + column.put("type", type); + if (bwcVersion.onOrAfter(Version.V_7_2_0)) { + column.put("display_size", 2147483647); + } else { + column.put("display_size", 0); + } + return unmodifiableMap(column); + } + + private void assertResponse(Map expected, Map actual) { + if (false == expected.equals(actual)) { + NotEqualMessageBuilder message = new NotEqualMessageBuilder(); + message.compareMaps(actual, expected); + fail("Response does not match:\n" + message.toString()); + } + } + + private Map runSql(RestClient client, Request request) throws IOException { + Response response = client.performRequest(request); + try (InputStream content = response.getEntity().getContent()) { + return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false); + } + } + + static Nodes buildNodeAndVersions(RestClient client) throws IOException { + Response response = client.performRequest(new Request("GET", "_nodes")); + ObjectPath objectPath = ObjectPath.createFromResponse(response); + Map nodesAsMap = objectPath.evaluate("nodes"); + Nodes nodes = new Nodes(); + for (String id : nodesAsMap.keySet()) { + nodes.add( + new Node( + id, + objectPath.evaluate("nodes." + id + ".name"), + Version.fromString(objectPath.evaluate("nodes." + id + ".version")), + HttpHost.create(objectPath.evaluate("nodes." + id + ".http.publish_address")) + ) + ); + } + response = client.performRequest(new Request("GET", "_cluster/state")); + nodes.setMasterNodeId(ObjectPath.createFromResponse(response).evaluate("master_node")); + return nodes; + } + + static final class Nodes extends HashMap { + + private String masterNodeId = null; + + public Node getMaster() { + return get(masterNodeId); + } + + public void setMasterNodeId(String id) { + if (get(id) == null) { + throw new IllegalArgumentException("node with id [" + id + "] not found. got:" + toString()); + } + masterNodeId = id; + } + + public void add(Node node) { + put(node.getId(), node); + } + + public List getNewNodes() { + Version bwcVersion = getBWCVersion(); + return values().stream().filter(n -> n.getVersion().after(bwcVersion)).collect(Collectors.toList()); + } + + public List getBWCNodes() { + Version bwcVersion = getBWCVersion(); + return values().stream().filter(n -> n.getVersion().equals(bwcVersion)).collect(Collectors.toList()); + } + + public Version getBWCVersion() { + if (isEmpty()) { + throw new IllegalStateException("no nodes available"); + } + return Version.fromId(values().stream().map(node -> node.getVersion().id).min(Integer::compareTo).get()); + } + + @Override + public String toString() { + return "Nodes{" + + "masterNodeId='" + + masterNodeId + + "'\n" + + values().stream().map(Node::toString).collect(Collectors.joining("\n")) + + '}'; + } + } + + static final class Node { + private final String id; + private final String nodeName; + private final Version version; + private final HttpHost publishAddress; + + Node(String id, String nodeName, Version version, HttpHost publishAddress) { + this.id = id; + this.nodeName = nodeName; + this.version = version; + this.publishAddress = publishAddress; + } + + public String getId() { + return id; + } + + public String getNodeName() { + return nodeName; + } + + public HttpHost getPublishAddress() { + return publishAddress; + } + + public Version getVersion() { + return version; + } + + @Override + public String toString() { + return "Node{" + "id='" + id + '\'' + ", nodeName='" + nodeName + '\'' + ", version=" + version + '}'; + } + } +} diff --git a/x-pack/plugin/sql/qa/server/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/CliExplainIT.java b/x-pack/plugin/sql/qa/server/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/CliExplainIT.java index a54cf3f944097..c0897488b78c5 100644 --- a/x-pack/plugin/sql/qa/server/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/CliExplainIT.java +++ b/x-pack/plugin/sql/qa/server/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/CliExplainIT.java @@ -39,12 +39,12 @@ public void testExplainBasic() throws IOException { assertThat(command("EXPLAIN (PLAN EXECUTABLE) SELECT * FROM test"), containsString("plan")); assertThat(readLine(), startsWith("----------")); assertThat(readLine(), startsWith("EsQueryExec[test,{")); - assertThat(readLine(), startsWith(" \"_source\" : {")); - assertThat(readLine(), startsWith(" \"includes\" : [")); - assertThat(readLine(), startsWith(" \"test_field\"")); - assertThat(readLine(), startsWith(" ],")); - assertThat(readLine(), startsWith(" \"excludes\" : [ ]")); - assertThat(readLine(), startsWith(" },")); + assertThat(readLine(), startsWith(" \"_source\" : false,")); + assertThat(readLine(), startsWith(" \"fields\" : [")); + assertThat(readLine(), startsWith(" {")); + assertThat(readLine(), startsWith(" \"field\" : \"test_field\"")); + assertThat(readLine(), startsWith(" }")); + assertThat(readLine(), startsWith(" ],")); assertThat(readLine(), startsWith(" \"sort\" : [")); assertThat(readLine(), startsWith(" {")); assertThat(readLine(), startsWith(" \"_doc\" :")); @@ -96,13 +96,15 @@ public void testExplainWithWhere() throws IOException { assertThat(readLine(), startsWith(" }")); assertThat(readLine(), startsWith(" }")); assertThat(readLine(), startsWith(" },")); - assertThat(readLine(), startsWith(" \"_source\" : {")); - assertThat(readLine(), startsWith(" \"includes\" : [")); - assertThat(readLine(), startsWith(" \"i\"")); - assertThat(readLine(), startsWith(" \"test_field\"")); - assertThat(readLine(), startsWith(" ],")); - assertThat(readLine(), startsWith(" \"excludes\" : [ ]")); - assertThat(readLine(), startsWith(" },")); + assertThat(readLine(), startsWith(" \"_source\" : false,")); + assertThat(readLine(), startsWith(" \"fields\" : [")); + assertThat(readLine(), startsWith(" {")); + assertThat(readLine(), startsWith(" \"field\" : \"i\"")); + assertThat(readLine(), startsWith(" },")); + assertThat(readLine(), startsWith(" {")); + assertThat(readLine(), startsWith(" \"field\" : \"test_field\"")); + assertThat(readLine(), startsWith(" }")); + assertThat(readLine(), startsWith(" ],")); assertThat(readLine(), startsWith(" \"sort\" : [")); assertThat(readLine(), startsWith(" {")); assertThat(readLine(), startsWith(" \"_doc\" :")); @@ -142,7 +144,6 @@ public void testExplainWithCount() throws IOException { assertThat(readLine(), startsWith("EsQueryExec[test,{")); assertThat(readLine(), startsWith(" \"size\" : 0,")); assertThat(readLine(), startsWith(" \"_source\" : false,")); - assertThat(readLine(), startsWith(" \"stored_fields\" : \"_none_\",")); assertThat(readLine(), startsWith(" \"sort\" : [")); assertThat(readLine(), startsWith(" {")); assertThat(readLine(), startsWith(" \"_doc\" :")); diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java index e294fdea67497..e2e3677282cb5 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java @@ -48,7 +48,7 @@ public void testTextField() throws IOException { String query = "SELECT text_field FROM test"; String text = randomAlphaOfLength(20); boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level Map indexProps = new HashMap<>(1); indexProps.put("_source", enableSource); @@ -73,10 +73,10 @@ public void testTextField() throws IOException { * } */ public void testKeywordField() throws IOException { + String query = "SELECT keyword_field FROM test"; String keyword = randomAlphaOfLength(20); - // _source for `keyword` fields doesn't matter, as they should be taken from docvalue_fields boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean ignoreAbove = randomBoolean(); Map indexProps = new HashMap<>(1); @@ -93,10 +93,14 @@ public void testKeywordField() throws IOException { createIndexWithFieldTypeAndProperties("keyword", fieldProps, explicitSourceSetting ? indexProps : null); index("{\"keyword_field\":\"" + keyword + "\"}"); - Map expected = new HashMap<>(); - expected.put("columns", Arrays.asList(columnInfo("plain", "keyword_field", "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE))); - expected.put("rows", singletonList(singletonList(ignoreAbove ? null : keyword))); - assertResponse(expected, runSql("SELECT keyword_field FROM test")); + if (explicitSourceSetting == false || enableSource) { + Map expected = new HashMap<>(); + expected.put("columns", Arrays.asList(columnInfo("plain", "keyword_field", "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE))); + expected.put("rows", singletonList(singletonList(ignoreAbove ? null : keyword))); + assertResponse(expected, runSql(query)); + } else { + expectSourceDisabledError(query); + } } /* @@ -106,10 +110,10 @@ public void testKeywordField() throws IOException { * } */ public void testConstantKeywordField() throws IOException { + String query = "SELECT constant_keyword_field FROM test"; String value = randomAlphaOfLength(20); - // _source for `constant_keyword` fields doesn't matter, as they should be taken from docvalue_fields boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level Map indexProps = new HashMap<>(1); indexProps.put("_source", enableSource); @@ -125,13 +129,17 @@ public void testConstantKeywordField() throws IOException { createIndexWithFieldTypeAndProperties("constant_keyword", fieldProps, explicitSourceSetting ? indexProps : null); index("{\"constant_keyword_field\":\"" + value + "\"}"); - Map expected = new HashMap<>(); - expected.put( - "columns", - Arrays.asList(columnInfo("plain", "constant_keyword_field", "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE)) - ); - expected.put("rows", singletonList(singletonList(value))); - assertResponse(expected, runSql("SELECT constant_keyword_field FROM test")); + if (explicitSourceSetting == false || enableSource) { + Map expected = new HashMap<>(); + expected.put( + "columns", + Arrays.asList(columnInfo("plain", "constant_keyword_field", "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE)) + ); + expected.put("rows", singletonList(singletonList(value))); + assertResponse(expected, runSql(query)); + } else { + expectSourceDisabledError(query); + } } /* @@ -141,10 +149,10 @@ public void testConstantKeywordField() throws IOException { * } */ public void testWildcardField() throws IOException { + String query = "SELECT wildcard_field FROM test"; String wildcard = randomAlphaOfLength(20); - // _source for `wildcard` fields doesn't matter, as they should be taken from docvalue_fields boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean ignoreAbove = randomBoolean(); Map indexProps = new HashMap<>(1); @@ -161,10 +169,14 @@ public void testWildcardField() throws IOException { createIndexWithFieldTypeAndProperties("wildcard", fieldProps, explicitSourceSetting ? indexProps : null); index("{\"wildcard_field\":\"" + wildcard + "\"}"); - Map expected = new HashMap<>(); - expected.put("columns", Arrays.asList(columnInfo("plain", "wildcard_field", "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE))); - expected.put("rows", singletonList(singletonList(ignoreAbove ? null : wildcard))); - assertResponse(expected, runSql("SELECT wildcard_field FROM test")); + if (explicitSourceSetting == false || enableSource) { + Map expected = new HashMap<>(); + expected.put("columns", Arrays.asList(columnInfo("plain", "wildcard_field", "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE))); + expected.put("rows", singletonList(singletonList(ignoreAbove ? null : wildcard))); + assertResponse(expected, runSql(query)); + } else { + expectSourceDisabledError(query); + } } /* @@ -221,14 +233,7 @@ public void testCoerceForFloatingPointTypes() throws IOException { // because "coerce" is true, a "123.456" floating point number STRING should be converted to 123.456 as number // and converted to 123.5 for "scaled_float" type - expected.put( - "rows", - singletonList( - singletonList( - isScaledFloat ? 123.5 : (fieldType != "double" ? Double.valueOf(123.456f) : Double.valueOf(floatingPointNumber)) - ) - ) - ); + expected.put("rows", singletonList(singletonList(isScaledFloat ? 123.5 : 123.456d))); assertResponse(expected, runSql("SELECT " + fieldType + "_field FROM test")); } @@ -281,7 +286,7 @@ private void testField(String fieldType, Object value) throws IOException { String query = "SELECT " + fieldName + " FROM test"; Object actualValue = value; boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean ignoreMalformed = randomBoolean(); // ignore_malformed is true, thus test a non-number value Map indexProps = new HashMap<>(1); @@ -319,7 +324,7 @@ public void testBooleanField() throws IOException { String query = "SELECT boolean_field FROM test"; boolean booleanField = randomBoolean(); boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean asString = randomBoolean(); // pass true or false as string "true" or "false Map indexProps = new HashMap<>(1); @@ -336,7 +341,7 @@ public void testBooleanField() throws IOException { Map expected = new HashMap<>(); expected.put("columns", Arrays.asList(columnInfo("plain", "boolean_field", "boolean", JDBCType.BOOLEAN, Integer.MAX_VALUE))); // adding the boolean as a String here because parsing the response will yield a "true"/"false" String - expected.put("rows", singletonList(singletonList(asString ? String.valueOf(booleanField) : booleanField))); + expected.put("rows", singletonList(singletonList(booleanField))); assertResponse(expected, runSql(query)); } else { expectSourceDisabledError(query); @@ -354,7 +359,7 @@ public void testIpField() throws IOException { String ipField = "192.168.1.1"; String actualValue = ipField; boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean ignoreMalformed = randomBoolean(); Map indexProps = new HashMap<>(1); @@ -560,7 +565,7 @@ public void testAliasFromSourceField() throws IOException { columnInfo("plain", "a.b.c.text_field_alias", "text", JDBCType.VARCHAR, Integer.MAX_VALUE) ) ); - expected.put("rows", singletonList(Arrays.asList(text, null, null))); + expected.put("rows", singletonList(Arrays.asList(text, text, text))); assertResponse(expected, runSql("SELECT text_field, text_field_alias, a.b.c.text_field_alias FROM test")); } @@ -592,7 +597,7 @@ public void testAliasAggregatableFromSourceField() throws IOException { columnInfo("plain", "a.b.c.integer_field_alias", "integer", JDBCType.INTEGER, Integer.MAX_VALUE) ) ); - expected.put("rows", singletonList(Arrays.asList(number, null, number))); + expected.put("rows", singletonList(Arrays.asList(number, number, number))); assertResponse(expected, runSql("SELECT integer_field, integer_field_alias, a.b.c.integer_field_alias FROM test")); } @@ -609,9 +614,8 @@ public void testAliasAggregatableFromSourceField() throws IOException { */ public void testTextFieldWithKeywordSubfield() throws IOException { String text = randomAlphaOfLength(10) + " " + randomAlphaOfLength(10); - // _source for `keyword` fields doesn't matter, as they should be taken from docvalue_fields boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean ignoreAbove = randomBoolean(); String fieldName = "text_field"; String subFieldName = "text_field.keyword_subfield"; @@ -645,13 +649,7 @@ public void testTextFieldWithKeywordSubfield() throws IOException { assertResponse(expected, runSql(query)); } else { expectSourceDisabledError(query); - - // even if the _source is disabled, selecting only the keyword sub-field should work as expected - Map expected = new HashMap<>(); - expected.put("columns", Arrays.asList(columnInfo("plain", subFieldName, "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE))); - - expected.put("rows", singletonList(singletonList(ignoreAbove ? null : text))); - assertResponse(expected, runSql("SELECT text_field.keyword_subfield FROM test")); + expectSourceDisabledError("SELECT " + subFieldName + " FROM test"); } } @@ -669,7 +667,7 @@ public void testTextFieldWithKeywordSubfield() throws IOException { public void testTextFieldWithIntegerNumberSubfield() throws IOException { Integer number = randomInt(); boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean ignoreMalformed = randomBoolean(); // ignore_malformed is true, thus test a non-number value Object actualValue = number; String fieldName = "text_field"; @@ -787,7 +785,7 @@ public void testTextFieldWithIpSubfield() throws IOException { public void testNumberFieldWithTextOrKeywordSubfield() throws IOException { Integer number = randomInt(); boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean ignoreMalformed = randomBoolean(); // ignore_malformed is true, thus test a non-number value boolean isKeyword = randomBoolean(); // text or keyword subfield Object actualValue = number; @@ -833,21 +831,12 @@ public void testNumberFieldWithTextOrKeywordSubfield() throws IOException { } assertResponse(expected, runSql(query)); } else { + // disabling the _source means that nothing should be retrieved by the "fields" API if (isKeyword) { - // selecting only the keyword subfield when the _source is disabled should work - Map expected = new HashMap<>(); - expected.put("columns", singletonList(columnInfo("plain", subFieldName, "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE))); - if (ignoreMalformed) { - expected.put("rows", singletonList(singletonList("foo"))); - } else { - expected.put("rows", singletonList(singletonList(String.valueOf(number)))); - } - assertResponse(expected, runSql("SELECT integer_field.keyword_subfield FROM test")); + expectSourceDisabledError("SELECT integer_field.keyword_subfield FROM test"); } else { expectSourceDisabledError(query); } - - // if the _source is disabled, selecting only the integer field shouldn't work expectSourceDisabledError("SELECT " + fieldName + " FROM test"); } } @@ -947,7 +936,7 @@ public void testIntegerFieldWithByteSubfield() throws IOException { boolean isByte = randomBoolean(); Integer number = isByte ? randomByte() : randomIntBetween(Byte.MAX_VALUE + 1, Integer.MAX_VALUE); boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean rootIgnoreMalformed = randomBoolean(); // root field ignore_malformed boolean subFieldIgnoreMalformed = randomBoolean(); // sub-field ignore_malformed String fieldName = "integer_field"; @@ -1016,7 +1005,7 @@ public void testByteFieldWithIntegerSubfield() throws IOException { boolean isByte = randomBoolean(); Integer number = isByte ? randomByte() : randomIntBetween(Byte.MAX_VALUE + 1, Integer.MAX_VALUE); boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level + boolean enableSource = randomBoolean(); // enable _source at index level boolean rootIgnoreMalformed = randomBoolean(); // root field ignore_malformed boolean subFieldIgnoreMalformed = randomBoolean(); // sub-field ignore_malformed String fieldName = "byte_field"; @@ -1073,7 +1062,7 @@ private void expectSourceDisabledError(String query) { expectBadRequest(() -> { client().performRequest(buildRequest(query)); return Collections.emptyMap(); - }, containsString("unable to fetch fields from _source field: _source is disabled in the mappings for index [test]")); + }, containsString("Unable to retrieve the requested [fields] since _source is disabled in the mappings for index [test]")); } private void createIndexWithFieldTypeAndAlias(String type, Map> fieldProps, Map indexProps) diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java index 4bf509bd42afa..c357c01b62793 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java @@ -633,11 +633,10 @@ public void testBasicTranslateQuery() throws IOException { Map response = runTranslateSql(query("SELECT * FROM test").toString()); assertEquals(1000, response.get("size")); + // assertFalse((Boolean) response.get("_source")); @SuppressWarnings("unchecked") - Map source = (Map) response.get("_source"); - assertNotNull(source); - assertEquals(emptyList(), source.get("excludes")); - assertEquals(singletonList("test"), source.get("includes")); + List> source = (List>) response.get("fields"); + assertEquals(singletonList(singletonMap("field", "test")), source); } public void testBasicQueryWithFilter() throws IOException { @@ -704,11 +703,10 @@ public void testBasicTranslateQueryWithFilter() throws IOException { Map response = runTranslateSql(query("SELECT * FROM test").filter("{\"match\": {\"test\": \"foo\"}}").toString()); assertEquals(response.get("size"), 1000); + assertFalse((Boolean) response.get("_source")); @SuppressWarnings("unchecked") - Map source = (Map) response.get("_source"); - assertNotNull(source); - assertEquals(emptyList(), source.get("excludes")); - assertEquals(singletonList("test"), source.get("includes")); + List> source = (List>) response.get("fields"); + assertEquals(singletonList(singletonMap("field", "test")), source); @SuppressWarnings("unchecked") Map query = (Map) response.get("query"); @@ -745,7 +743,7 @@ public void testTranslateQueryWithGroupByAndHaving() throws IOException { assertEquals(response.get("size"), 0); assertEquals(false, response.get("_source")); - assertEquals("_none_", response.get("stored_fields")); + assertNull(response.get("stored_fields")); @SuppressWarnings("unchecked") Map aggregations = (Map) response.get("aggregations"); diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/docs/geo.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/docs/geo.csv-spec index 899147fd3e6d9..d10890e227e44 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/docs/geo.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/docs/geo.csv-spec @@ -15,7 +15,7 @@ selectAsWKT SELECT city, ST_AsWKT(location) location FROM "geo" WHERE city = 'Amsterdam'; city:s | location:s -Amsterdam |POINT (4.850311987102032 52.347556999884546) +Amsterdam |POINT (4.850312 52.347557) // end::aswkt ; diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/geo/geosql.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/geo/geosql.csv-spec index c9d7bc85448de..391f0effc6154 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/geo/geosql.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/geo/geosql.csv-spec @@ -32,72 +32,72 @@ shape | GEOMETRY | shape selectAllPointsAsStrings SELECT city, CAST(location AS STRING) location, CAST(location_no_dv AS STRING) location_no_dv, CAST(geoshape AS STRING) geoshape, CAST(shape AS STRING) shape, region FROM "geo" ORDER BY "city"; - city:s | location:s | location_no_dv:s | geoshape:s | shape:s | region:s -Amsterdam |POINT (4.850311987102032 52.347556999884546) |POINT (4.850312 52.347557) |POINT (4.850312 52.347557 2.0) |POINT (4.850312 52.347557 2.0) |Europe -Berlin |POINT (13.390888944268227 52.48670099303126) |POINT (13.390889 52.486701) |POINT (13.390889 52.486701 34.0) |POINT (13.390889 52.486701 34.0) |Europe -Chicago |POINT (-87.63787407428026 41.888782968744636) |POINT (-87.637874 41.888783) |POINT (-87.637874 41.888783 181.0) |POINT (-87.637874 41.888783 181.0) |Americas -Hong Kong |POINT (114.18392493389547 22.28139698971063) |POINT (114.183925 22.281397) |POINT (114.183925 22.281397 552.0) |POINT (114.183925 22.281397 552.0) |Asia -London |POINT (-0.12167204171419144 51.51087098289281)|POINT (-0.121672 51.510871) |POINT (-0.121672 51.510871 11.0) |POINT (-0.121672 51.510871 11.0) |Europe -Mountain View |POINT (-122.08384302444756 37.38648299127817) |POINT (-122.083843 37.386483) |POINT (-122.083843 37.386483 30.0) |POINT (-122.083843 37.386483 30.0) |Americas -Munich |POINT (11.537504978477955 48.14632098656148) |POINT (11.537505 48.146321) |POINT (11.537505 48.146321 519.0) |POINT (11.537505 48.146321 519.0) |Europe -New York |POINT (-73.9900270756334 40.74517097789794) |POINT (-73.990027 40.745171) |POINT (-73.990027 40.745171 10.0) |POINT (-73.990027 40.745171 10.0) |Americas -Paris |POINT (2.3517729341983795 48.84553796611726) |POINT (2.351773 48.845538) |POINT (2.351773 48.845538 35.0) |POINT (2.351773 48.845538 35.0) |Europe -Phoenix |POINT (-111.97350500151515 33.37624196894467) |POINT (-111.973505 33.376242) |POINT (-111.973505 33.376242 331.0)|POINT (-111.973505 33.376242 331.0)|Americas -San Francisco |POINT (-122.39422800019383 37.789540970698) |POINT (-122.394228 37.789541) |POINT (-122.394228 37.789541 16.0) |POINT (-122.394228 37.789541 16.0) |Americas -Seoul |POINT (127.06085099838674 37.50913198571652) |POINT (127.060851 37.509132) |POINT (127.060851 37.509132 38.0) |POINT (127.060851 37.509132 38.0) |Asia -Singapore |POINT (103.8555349688977 1.2958679627627134) |POINT (103.855535 1.295868) |POINT (103.855535 1.295868 15.0) |POINT (103.855535 1.295868 15.0) |Asia -Sydney |POINT (151.20862897485495 -33.863385021686554)|POINT (151.208629 -33.863385) |POINT (151.208629 -33.863385 100.0)|POINT (151.208629 -33.863385 100.0)|Asia -Tokyo |POINT (139.76402222178876 35.66961596254259) |POINT (139.76402225 35.669616)|POINT (139.76402225 35.669616 40.0)|POINT (139.76402225 35.669616 40.0)|Asia + city:s | location:s | location_no_dv:s | geoshape:s | shape:s | region:s +Amsterdam |POINT (4.850312 52.347557) |POINT (4.850312 52.347557) |POINT (4.850312 52.347557 2.0) |POINT (4.850312 52.347557 2.0) |Europe +Berlin |POINT (13.390889 52.486701) |POINT (13.390889 52.486701) |POINT (13.390889 52.486701 34.0) |POINT (13.390889 52.486701 34.0) |Europe +Chicago |POINT (-87.637874 41.888783) |POINT (-87.637874 41.888783) |POINT (-87.637874 41.888783 181.0) |POINT (-87.637874 41.888783 181.0) |Americas +Hong Kong |POINT (114.183925 22.281397) |POINT (114.183925 22.281397) |POINT (114.183925 22.281397 552.0) |POINT (114.183925 22.281397 552.0) |Asia +London |POINT (-0.121672 51.510871) |POINT (-0.121672 51.510871) |POINT (-0.121672 51.510871 11.0) |POINT (-0.121672 51.510871 11.0) |Europe +Mountain View |POINT (-122.083843 37.386483) |POINT (-122.083843 37.386483) |POINT (-122.083843 37.386483 30.0) |POINT (-122.083843 37.386483 30.0) |Americas +Munich |POINT (11.537505 48.146321) |POINT (11.537505 48.146321) |POINT (11.537505 48.146321 519.0) |POINT (11.537505 48.146321 519.0) |Europe +New York |POINT (-73.990027 40.745171) |POINT (-73.990027 40.745171) |POINT (-73.990027 40.745171 10.0) |POINT (-73.990027 40.745171 10.0) |Americas +Paris |POINT (2.351773 48.845538) |POINT (2.351773 48.845538) |POINT (2.351773 48.845538 35.0) |POINT (2.351773 48.845538 35.0) |Europe +Phoenix |POINT (-111.973505 33.376242) |POINT (-111.973505 33.376242) |POINT (-111.973505 33.376242 331.0)|POINT (-111.973505 33.376242 331.0)|Americas +San Francisco |POINT (-122.394228 37.789541) |POINT (-122.394228 37.789541) |POINT (-122.394228 37.789541 16.0) |POINT (-122.394228 37.789541 16.0) |Americas +Seoul |POINT (127.060851 37.509132) |POINT (127.060851 37.509132) |POINT (127.060851 37.509132 38.0) |POINT (127.060851 37.509132 38.0) |Asia +Singapore |POINT (103.855535 1.295868) |POINT (103.855535 1.295868) |POINT (103.855535 1.295868 15.0) |POINT (103.855535 1.295868 15.0) |Asia +Sydney |POINT (151.208629 -33.863385) |POINT (151.208629 -33.863385) |POINT (151.208629 -33.863385 100.0)|POINT (151.208629 -33.863385 100.0)|Asia +Tokyo |POINT (139.76402225 35.669616)|POINT (139.76402225 35.669616)|POINT (139.76402225 35.669616 40.0)|POINT (139.76402225 35.669616 40.0)|Asia ; // TODO: Both shape and location contain the same data for now, we should change it later to make things more interesting selectAllPointsAsWKT SELECT city, ST_ASWKT(location) location_wkt, ST_ASWKT(geoshape) geoshape_wkt, region FROM "geo" ORDER BY "city"; - city:s | location_wkt:s | geoshape_wkt:s | region:s -Amsterdam |POINT (4.850311987102032 52.347556999884546) |POINT (4.850312 52.347557 2.0) |Europe -Berlin |POINT (13.390888944268227 52.48670099303126) |POINT (13.390889 52.486701 34.0) |Europe -Chicago |POINT (-87.63787407428026 41.888782968744636) |POINT (-87.637874 41.888783 181.0) |Americas -Hong Kong |POINT (114.18392493389547 22.28139698971063) |POINT (114.183925 22.281397 552.0) |Asia -London |POINT (-0.12167204171419144 51.51087098289281)|POINT (-0.121672 51.510871 11.0) |Europe -Mountain View |POINT (-122.08384302444756 37.38648299127817) |POINT (-122.083843 37.386483 30.0) |Americas -Munich |POINT (11.537504978477955 48.14632098656148) |POINT (11.537505 48.146321 519.0) |Europe -New York |POINT (-73.9900270756334 40.74517097789794) |POINT (-73.990027 40.745171 10.0) |Americas -Paris |POINT (2.3517729341983795 48.84553796611726) |POINT (2.351773 48.845538 35.0) |Europe -Phoenix |POINT (-111.97350500151515 33.37624196894467) |POINT (-111.973505 33.376242 331.0) |Americas -San Francisco |POINT (-122.39422800019383 37.789540970698) |POINT (-122.394228 37.789541 16.0) |Americas -Seoul |POINT (127.06085099838674 37.50913198571652) |POINT (127.060851 37.509132 38.0) |Asia -Singapore |POINT (103.8555349688977 1.2958679627627134) |POINT (103.855535 1.295868 15.0) |Asia -Sydney |POINT (151.20862897485495 -33.863385021686554)|POINT (151.208629 -33.863385 100.0) |Asia -Tokyo |POINT (139.76402222178876 35.66961596254259) |POINT (139.76402225 35.669616 40.0) |Asia + city:s | location_wkt:s | geoshape_wkt:s | region:s +Amsterdam |POINT (4.850312 52.347557) |POINT (4.850312 52.347557 2.0) |Europe +Berlin |POINT (13.390889 52.486701) |POINT (13.390889 52.486701 34.0) |Europe +Chicago |POINT (-87.637874 41.888783) |POINT (-87.637874 41.888783 181.0) |Americas +Hong Kong |POINT (114.183925 22.281397) |POINT (114.183925 22.281397 552.0) |Asia +London |POINT (-0.121672 51.510871) |POINT (-0.121672 51.510871 11.0) |Europe +Mountain View |POINT (-122.083843 37.386483) |POINT (-122.083843 37.386483 30.0) |Americas +Munich |POINT (11.537505 48.146321) |POINT (11.537505 48.146321 519.0) |Europe +New York |POINT (-73.990027 40.745171) |POINT (-73.990027 40.745171 10.0) |Americas +Paris |POINT (2.351773 48.845538) |POINT (2.351773 48.845538 35.0) |Europe +Phoenix |POINT (-111.973505 33.376242) |POINT (-111.973505 33.376242 331.0)|Americas +San Francisco |POINT (-122.394228 37.789541) |POINT (-122.394228 37.789541 16.0) |Americas +Seoul |POINT (127.060851 37.509132) |POINT (127.060851 37.509132 38.0) |Asia +Singapore |POINT (103.855535 1.295868) |POINT (103.855535 1.295868 15.0) |Asia +Sydney |POINT (151.208629 -33.863385) |POINT (151.208629 -33.863385 100.0)|Asia +Tokyo |POINT (139.76402225 35.669616)|POINT (139.76402225 35.669616 40.0)|Asia ; selectWithAsWKTInWhere SELECT city, ST_ASWKT(location) location_wkt, region FROM "geo" WHERE LOCATE('114', ST_ASWKT(location)) > 0 ORDER BY "city"; - city:s | location_wkt:s | region:s -Hong Kong |POINT (114.18392493389547 22.28139698971063)|Asia + city:s | location_wkt:s | region:s +Hong Kong |POINT (114.183925 22.281397)|Asia ; selectAllPointsOrderByLonFromAsWKT SELECT city, SUBSTRING(ST_ASWKT(location), 8, LOCATE(' ', ST_ASWKT(location), 8) - 8) lon FROM "geo" ORDER BY lon; city:s | lon:s -London |-0.12167204171419144 -Phoenix |-111.97350500151515 -Mountain View |-122.08384302444756 -San Francisco |-122.39422800019383 -New York |-73.9900270756334 -Chicago |-87.63787407428026 -Singapore |103.8555349688977 -Munich |11.537504978477955 -Hong Kong |114.18392493389547 -Seoul |127.06085099838674 -Berlin |13.390888944268227 -Tokyo |139.76402222178876 -Sydney |151.20862897485495 -Paris |2.3517729341983795 -Amsterdam |4.850311987102032 +London |-0.121672 +Phoenix |-111.973505 +Mountain View |-122.083843 +San Francisco |-122.394228 +New York |-73.990027 +Chicago |-87.637874 +Singapore |103.855535 +Munich |11.537505 +Hong Kong |114.183925 +Seoul |127.060851 +Berlin |13.390889 +Tokyo |139.76402225 +Sydney |151.208629 +Paris |2.351773 +Amsterdam |4.850312 ; selectAllPointsGroupByHemisphereFromAsWKT @@ -157,11 +157,11 @@ selectCitiesByDistance SELECT region, city, ST_Distance(location, ST_WktToSQL('POINT (-71 42)')) distance FROM geo WHERE distance < 5000000 ORDER BY region, city; region:s | city:s | distance:d -Americas |Chicago |1373941.5140200066 -Americas |Mountain View |4335936.909375596 -Americas |New York |285839.6579622518 -Americas |Phoenix |3692895.0346903414 -Americas |San Francisco |4343565.010996301 +Americas |Chicago |1373941.5075370357 +Americas |Mountain View |4335936.907008218 +Americas |New York |285839.6512191343 +Americas |Phoenix |3692895.0329883597 +Americas |San Francisco |4343565.009715615 ; selectCitiesByDistanceFloored @@ -267,27 +267,27 @@ SELECT COUNT(*) cnt, FLOOR(ST_Y(location)/45) north, FLOOR(ST_X(location)/90) ea selectFilterByXOfLocation SELECT city, ST_X(geoshape) x, ST_Y(geoshape) y, ST_Z(geoshape) z, ST_X(location) lx, ST_Y(location) ly FROM geo WHERE lx > 0 ORDER BY ly; - city:s | x:d | y:d | z:d | lx:d | ly:d -Sydney |151.208629 |-33.863385 |100.0 |151.20862897485495|-33.863385021686554 -Singapore |103.855535 |1.295868 |15.0 |103.8555349688977 |1.2958679627627134 -Hong Kong |114.183925 |22.281397 |552.0 |114.18392493389547|22.28139698971063 -Tokyo |139.76402225 |35.669616 |40.0 |139.76402222178876|35.66961596254259 -Seoul |127.060851 |37.509132 |38.0 |127.06085099838674|37.50913198571652 -Munich |11.537505 |48.146321 |519.0 |11.537504978477955|48.14632098656148 -Paris |2.351773 |48.845538 |35.0 |2.3517729341983795|48.84553796611726 -Amsterdam |4.850312 |52.347557 |2.0 |4.850311987102032 |52.347556999884546 -Berlin |13.390889 |52.486701 |34.0 |13.390888944268227|52.48670099303126 + city:s | x:d | y:d | z:d | lx:d | ly:d +Sydney |151.208629 |-33.863385 |100.0 |151.208629 |-33.863385 +Singapore |103.855535 |1.295868 |15.0 |103.855535 |1.295868 +Hong Kong |114.183925 |22.281397 |552.0 |114.183925 |22.281397 +Tokyo |139.76402225 |35.669616 |40.0 |139.76402225 |35.669616 +Seoul |127.060851 |37.509132 |38.0 |127.060851 |37.509132 +Munich |11.537505 |48.146321 |519.0 |11.537505 |48.146321 +Paris |2.351773 |48.845538 |35.0 |2.351773 |48.845538 +Amsterdam |4.850312 |52.347557 |2.0 |4.850312 |52.347557 +Berlin |13.390889 |52.486701 |34.0 |13.390889 |52.486701 ; selectFilterByRegionPoint SELECT city, region, ST_X(location) x FROM geo WHERE ST_X(ST_WKTTOSQL(region_point)) < 0 ORDER BY x; city:s | region:s | x:d -San Francisco |Americas |-122.39422800019383 -Mountain View |Americas |-122.08384302444756 -Phoenix |Americas |-111.97350500151515 -Chicago |Americas |-87.63787407428026 -New York |Americas |-73.9900270756334 +San Francisco |Americas |-122.394228 +Mountain View |Americas |-122.083843 +Phoenix |Americas |-111.973505 +Chicago |Americas |-87.637874 +New York |Americas |-73.990027 ; selectLargeLat diff --git a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryResponseTests.java b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryResponseTests.java index 7dd3f4aade637..67e7319a72491 100644 --- a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryResponseTests.java +++ b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryResponseTests.java @@ -52,7 +52,8 @@ public static SqlQueryResponse createRandomInstance(String cursor, Mode mode, bo if (randomBoolean()) { columns = new ArrayList<>(columnCount); for (int i = 0; i < columnCount; i++) { - columns.add(new ColumnInfo(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10), randomInt(25))); + columns.add(new ColumnInfo(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10), + randomBoolean() ? null : randomInt(25))); } } diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/ColumnInfo.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/ColumnInfo.java index 7cf31781d9e8f..ed47edfdad0a4 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/ColumnInfo.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/ColumnInfo.java @@ -106,7 +106,7 @@ public String esType() { /** * Used by JDBC */ - public int displaySize() { + public Integer displaySize() { return displaySize; } diff --git a/x-pack/plugin/sql/src/internalClusterTest/java/org/elasticsearch/xpack/sql/action/SqlTranslateActionIT.java b/x-pack/plugin/sql/src/internalClusterTest/java/org/elasticsearch/xpack/sql/action/SqlTranslateActionIT.java index c6d6ce24b1435..5fee778ef0219 100644 --- a/x-pack/plugin/sql/src/internalClusterTest/java/org/elasticsearch/xpack/sql/action/SqlTranslateActionIT.java +++ b/x-pack/plugin/sql/src/internalClusterTest/java/org/elasticsearch/xpack/sql/action/SqlTranslateActionIT.java @@ -8,10 +8,12 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.search.builder.SearchSourceBuilder; -import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.FieldAndFormat; import org.elasticsearch.search.sort.SortBuilders; +import java.util.ArrayList; +import java.util.List; + import static java.util.Collections.singletonList; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; @@ -31,12 +33,19 @@ public void testSqlTranslateAction() { SqlTranslateResponse response = new SqlTranslateRequestBuilder(client(), SqlTranslateAction.INSTANCE) .query("SELECT " + columns + " FROM test ORDER BY count").get(); SearchSourceBuilder source = response.source(); - FetchSourceContext fetch = source.fetchSource(); - assertTrue(fetch.fetchSource()); - assertArrayEquals(new String[] { "data", "count" }, fetch.includes()); - assertEquals( - singletonList(new FieldAndFormat("date", "epoch_millis")), - source.docValueFields()); + List actualFields = source.fetchFields(); + List expectedFields = new ArrayList<>(3); + if (columnOrder) { + expectedFields.add(new FieldAndFormat("data", null)); + expectedFields.add(new FieldAndFormat("count", null)); + expectedFields.add(new FieldAndFormat("date", "epoch_millis")); + } else { + expectedFields.add(new FieldAndFormat("date", "epoch_millis")); + expectedFields.add(new FieldAndFormat("data", null)); + expectedFields.add(new FieldAndFormat("count", null)); + } + + assertEquals(expectedFields, actualFields); assertEquals(singletonList(SortBuilders.fieldSort("count").missing("_last").unmappedType("long")), source.sorts()); } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java index 8aeea3132e8f1..1c7ca44b6a32e 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.sql.execution; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Client; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -22,9 +23,9 @@ import org.elasticsearch.xpack.sql.planner.Planner; import org.elasticsearch.xpack.sql.planner.PlanningException; import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue; -import org.elasticsearch.xpack.sql.session.SqlConfiguration; import org.elasticsearch.xpack.sql.session.Cursor; import org.elasticsearch.xpack.sql.session.Cursor.Page; +import org.elasticsearch.xpack.sql.session.SqlConfiguration; import org.elasticsearch.xpack.sql.session.SqlSession; import org.elasticsearch.xpack.sql.stats.Metrics; import org.elasticsearch.xpack.sql.stats.QueryMetric; @@ -34,6 +35,7 @@ import static org.elasticsearch.action.ActionListener.wrap; public class PlanExecutor { + public static final Version FIELDS_API_INTRODUCTION_VERSION = Version.V_7_10_0; private final Client client; private final NamedWriteableRegistry writableRegistry; @@ -44,7 +46,7 @@ public class PlanExecutor { private final Verifier verifier; private final Optimizer optimizer; private final Planner planner; - + private final Metrics metrics; public PlanExecutor(Client client, IndexResolver indexResolver, NamedWriteableRegistry writeableRegistry) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java index de203cf409bc9..237d91f4e0e00 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java @@ -83,6 +83,7 @@ import static java.util.Collections.singletonList; import static org.elasticsearch.action.ActionListener.wrap; +import static org.elasticsearch.xpack.sql.execution.PlanExecutor.FIELDS_API_INTRODUCTION_VERSION; // TODO: add retry/back-off public class Querier { @@ -143,12 +144,17 @@ public void query(List output, QueryContainer query, String index, Ac public static SearchRequest prepareRequest(Client client, SearchSourceBuilder source, TimeValue timeout, boolean includeFrozen, String... indices) { - return client.prepareSearch(indices) - // always track total hits accurately - .setTrackTotalHits(true).setAllowPartialSearchResults(false).setSource(source).setTimeout(timeout) - .setIndicesOptions( - includeFrozen ? IndexResolver.FIELD_CAPS_FROZEN_INDICES_OPTIONS : IndexResolver.FIELD_CAPS_INDICES_OPTIONS) - .request(); + source.trackTotalHits(true); + source.timeout(timeout); + + SearchRequest searchRequest = new SearchRequest(FIELDS_API_INTRODUCTION_VERSION); + searchRequest.indices(indices); + searchRequest.source(source); + searchRequest.allowPartialSearchResults(false); + searchRequest.indicesOptions( + includeFrozen ? IndexResolver.FIELD_CAPS_FROZEN_INDICES_OPTIONS : IndexResolver.FIELD_CAPS_INDICES_OPTIONS); + + return searchRequest; } protected static void logSearchResponse(SearchResponse response, Logger logger) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java index 55df28b9f09d8..fe364e9edd109 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java @@ -186,8 +186,5 @@ private static void optimize(QueryContainer query, SearchSourceBuilder builder) private static void disableSource(SearchSourceBuilder builder) { builder.fetchSource(FetchSourceContext.DO_NOT_FETCH_SOURCE); - if (builder.storedFields() == null) { - builder.storedFields(NO_STORED_FIELD); - } } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index 0a621eec44cd3..b3a3895525f26 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -7,7 +7,6 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.geo.GeoPoint; -import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.xpack.ql.execution.search.extractor.AbstractFieldHitExtractor; import org.elasticsearch.xpack.ql.execution.search.extractor.HitExtractor; @@ -101,7 +100,7 @@ protected Object unwrapCustomValue(Object values) { if (dataType == GEO_POINT) { try { - GeoPoint geoPoint = GeoUtils.parseGeoPoint(values, true); + GeoPoint geoPoint = geoPointParserDelegate.parse(values); return new GeoShape(geoPoint.lon(), geoPoint.lat()); } catch (ElasticsearchParseException ex) { throw new SqlIllegalArgumentException("Cannot parse geo_point value [{}] (returned by [{}])", values, fieldName()); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java index e10686dec46c2..2eabcd1eedaed 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java @@ -111,7 +111,6 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli throw new IllegalArgumentException("Invalid use of [columnar] argument: cannot be used in combination with " + "txt, csv or tsv formats"); } - long startNanos = System.nanoTime(); return channel -> client.execute(SqlQueryAction.INSTANCE, sqlRequest, new RestResponseListener(channel) { @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java index 153f405950488..59028ba235cdd 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java @@ -5,9 +5,15 @@ */ package org.elasticsearch.xpack.sql.plugin; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionListenerResponseHandler; +import org.elasticsearch.action.search.SearchPhaseExecutionException; +import org.elasticsearch.action.search.VersionMismatchException; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; @@ -19,6 +25,7 @@ import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.security.SecurityContext; import org.elasticsearch.xpack.ql.type.Schema; +import org.elasticsearch.xpack.ql.util.Holder; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.action.SqlQueryAction; import org.elasticsearch.xpack.sql.action.SqlQueryRequest; @@ -26,12 +33,12 @@ import org.elasticsearch.xpack.sql.execution.PlanExecutor; import org.elasticsearch.xpack.sql.proto.ColumnInfo; import org.elasticsearch.xpack.sql.proto.Mode; -import org.elasticsearch.xpack.sql.session.SqlConfiguration; import org.elasticsearch.xpack.sql.session.Cursor; import org.elasticsearch.xpack.sql.session.Cursor.Page; import org.elasticsearch.xpack.sql.session.Cursors; import org.elasticsearch.xpack.sql.session.RowSet; import org.elasticsearch.xpack.sql.session.SchemaRowSet; +import org.elasticsearch.xpack.sql.session.SqlConfiguration; import org.elasticsearch.xpack.sql.type.SqlDataTypes; import java.time.ZoneId; @@ -44,11 +51,13 @@ import static org.elasticsearch.xpack.sql.plugin.Transports.username; public class TransportSqlQueryAction extends HandledTransportAction { + private final SecurityContext securityContext; private final ClusterService clusterService; private final PlanExecutor planExecutor; private final SqlLicenseChecker sqlLicenseChecker; - + private final TransportService transportService; + private static final Logger log = LogManager.getLogger(TransportSqlQueryAction.class); @Inject public TransportSqlQueryAction(Settings settings, ClusterService clusterService, TransportService transportService, ThreadPool threadPool, ActionFilters actionFilters, PlanExecutor planExecutor, @@ -60,19 +69,21 @@ public TransportSqlQueryAction(Settings settings, ClusterService clusterService, this.clusterService = clusterService; this.planExecutor = planExecutor; this.sqlLicenseChecker = sqlLicenseChecker; + this.transportService = transportService; } @Override protected void doExecute(Task task, SqlQueryRequest request, ActionListener listener) { sqlLicenseChecker.checkIfSqlAllowed(request.mode()); - operation(planExecutor, request, listener, username(securityContext), clusterName(clusterService)); + operation(planExecutor, request, listener, username(securityContext), clusterName(clusterService), transportService, + clusterService); } /** * Actual implementation of the action. Statically available to support embedded mode. */ static void operation(PlanExecutor planExecutor, SqlQueryRequest request, ActionListener listener, - String username, String clusterName) { + String username, String clusterName, TransportService transportService, ClusterService clusterService) { // The configuration is always created however when dealing with the next page, only the timeouts are relevant // the rest having default values (since the query is already created) SqlConfiguration cfg = new SqlConfiguration(request.zoneId(), request.fetchSize(), request.requestTimeout(), request.pageTimeout(), @@ -80,8 +91,52 @@ static void operation(PlanExecutor planExecutor, SqlQueryRequest request, Action request.indexIncludeFrozen()); if (Strings.hasText(request.cursor()) == false) { + Holder retrySecondTime = new Holder(false); planExecutor.sql(cfg, request.query(), request.params(), + wrap(p -> listener.onResponse(createResponseWithSchema(request, p)), e -> { + // the search request will likely run on nodes with different versions of ES + // we will retry on a node with an older version that should generate a backwards compatible _search request + if (e instanceof SearchPhaseExecutionException + && ((SearchPhaseExecutionException) e).getCause() instanceof VersionMismatchException) { + + SearchPhaseExecutionException spee = (SearchPhaseExecutionException) e; + if (log.isTraceEnabled()) { + log.trace("Caught exception type [{}] with cause [{}].", e.getClass().getName(), e.getCause()); + } + DiscoveryNode localNode = clusterService.state().nodes().getLocalNode(); + DiscoveryNode candidateNode = null; + for (DiscoveryNode node : clusterService.state().nodes()) { + // find the first node that's older than the current node + if (node != localNode && node.getVersion().before(localNode.getVersion())) { + candidateNode = node; + break; + } + } + if (candidateNode != null) { + if (log.isTraceEnabled()) { + log.trace("Candidate node to resend the request to: address [{}], id [{}], name [{}], version [{}]", + candidateNode.getAddress(), candidateNode.getId(), candidateNode.getName(), candidateNode.getVersion()); + } + // re-send the request to the older node + transportService.sendRequest(candidateNode, SqlQueryAction.NAME, request, + new ActionListenerResponseHandler<>(listener, SqlQueryResponse::new, ThreadPool.Names.SAME)); + } else { + if (log.isTraceEnabled()) { + log.trace("No candidate node found, likely all were upgraded in the meantime"); + } + retrySecondTime.set(true); + } + } else { + listener.onFailure(e); + } + })); + if (retrySecondTime.get()) { + if (log.isTraceEnabled()) { + log.trace("No candidate node found, likely all were upgraded in the meantime. Re-trying the original request."); + } + planExecutor.sql(cfg, request.query(), request.params(), wrap(p -> listener.onResponse(createResponseWithSchema(request, p)), listener::onFailure)); + } } else { Tuple decoded = Cursors.decodeFromStringWithZone(request.cursor()); planExecutor.nextPage(cfg, decoded.v1(), 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 64e4245b557ce..ffa2ce1acd76e 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 @@ -26,7 +26,6 @@ import org.elasticsearch.xpack.ql.querydsl.query.NestedQuery; import org.elasticsearch.xpack.ql.querydsl.query.Query; import org.elasticsearch.xpack.ql.tree.Source; -import org.elasticsearch.xpack.ql.type.DataTypes; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.execution.search.SourceGenerator; import org.elasticsearch.xpack.sql.expression.function.Score; @@ -331,30 +330,7 @@ private FieldExtraction topHitFieldRef(FieldAttribute fieldAttr) { FieldAttribute actualField = fieldAttr; FieldAttribute rootField = fieldAttr; StringBuilder fullFieldName = new StringBuilder(fieldAttr.field().getName()); - - // Only if the field is not an alias (in which case it will be taken out from docvalue_fields if it's isAggregatable()), - // go up the tree of parents until a non-object (and non-nested) type of field is found and use that specific parent - // as the field to extract data from, from _source. We do it like this because sub-fields are not in the _source, only - // the root field to which those sub-fields belong to, are. Instead of "text_field.keyword_subfield" for _source extraction, - // we use "text_field", because there is no source for "keyword_subfield". - /* - * "text_field": { - * "type": "text", - * "fields": { - * "keyword_subfield": { - * "type": "keyword" - * } - * } - * } - */ - if (fieldAttr.field().isAlias() == false) { - while (actualField.parent() != null - && actualField.parent().field().getDataType() != DataTypes.OBJECT - && actualField.parent().field().getDataType() != DataTypes.NESTED - && SqlDataTypes.isFromDocValuesOnly(actualField.field().getDataType()) == false) { - actualField = actualField.parent(); - } - } + while (rootField.parent() != null) { fullFieldName.insert(0, ".").insert(0, rootField.parent().field().getName()); rootField = rootField.parent(); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/SearchHitFieldRef.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/SearchHitFieldRef.java index 3c28c9129220a..b6dba073056fb 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/SearchHitFieldRef.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/SearchHitFieldRef.java @@ -59,11 +59,7 @@ public void collectFields(QlSourceBuilder sourceBuilder) { if (hitName != null) { return; } - if (docValue) { - sourceBuilder.addDocField(name, SqlDataTypes.format(dataType)); - } else { - sourceBuilder.addSourceField(name); - } + sourceBuilder.addFetchField(name, SqlDataTypes.format(dataType)); } @Override diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SqlSourceBuilderTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SqlSourceBuilderTests.java index 5851178b1d445..8301d78ee0d46 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SqlSourceBuilderTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SqlSourceBuilderTests.java @@ -7,26 +7,26 @@ import org.elasticsearch.script.Script; import org.elasticsearch.search.builder.SearchSourceBuilder; -import org.elasticsearch.search.fetch.subphase.FetchSourceContext; +import org.elasticsearch.search.fetch.subphase.FieldAndFormat; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.ql.execution.search.QlSourceBuilder; -import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; public class SqlSourceBuilderTests extends ESTestCase { + public void testSqlSourceBuilder() { final QlSourceBuilder ssb = new QlSourceBuilder(); final SearchSourceBuilder source = new SearchSourceBuilder(); ssb.trackScores(); - ssb.addSourceField("foo"); - ssb.addSourceField("foo2"); - ssb.addDocField("bar", null); - ssb.addDocField("bar2", null); + ssb.addFetchField("foo", null); + ssb.addFetchField("foo2", "test"); final Script s = new Script("eggplant"); ssb.addScriptField("baz", s); final Script s2 = new Script("potato"); @@ -34,9 +34,16 @@ public void testSqlSourceBuilder() { ssb.build(source); assertTrue(source.trackScores()); - FetchSourceContext fsc = source.fetchSource(); - assertThat(Arrays.asList(fsc.includes()), contains("foo", "foo2")); - assertThat(source.docValueFields().stream().map(ff -> ff.field).collect(Collectors.toList()), contains("bar", "bar2")); + assertNull(source.fetchSource()); + assertNull(source.docValueFields()); + + List fetchFields = source.fetchFields(); + assertThat(fetchFields.size(), equalTo(2)); + assertThat(fetchFields.get(0).field, equalTo("foo")); + assertThat(fetchFields.get(0).format, is(nullValue())); + assertThat(fetchFields.get(1).field, equalTo("foo2")); + assertThat(fetchFields.get(1).format, equalTo("test")); + Map scriptFields = source.scriptFields() .stream() .collect(Collectors.toMap(SearchSourceBuilder.ScriptField::fieldName, SearchSourceBuilder.ScriptField::script)); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java index 712676f4c98ff..587e94585311c 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java @@ -46,7 +46,7 @@ public class FieldHitExtractorTests extends AbstractSqlWireSerializingTestCase fe.extract(hit)); - assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported")); - } - - public void testSingleValueArrayInSource() throws IOException { - String fieldName = randomAlphaOfLength(5); - FieldHitExtractor fe = getFieldHitExtractor(fieldName, false); - SearchHit hit = new SearchHit(1); - XContentBuilder source = JsonXContent.contentBuilder(); - Object value = randomValue(); - source.startObject(); { - source.field(fieldName, Collections.singletonList(value)); - } - source.endObject(); - BytesReference sourceRef = BytesReference.bytes(source); - hit.sourceRef(sourceRef); - assertFieldHitEquals(value, fe.extract(hit)); - } - public void testExtractSourcePath() { FieldHitExtractor fe = getFieldHitExtractor("a.b.c", false); Object value = randomValue(); @@ -442,6 +414,7 @@ public void testFieldsWithSingleValueArrayAsSubfield_TwoNestedLists2() { assertEquals(value, fe.extractFromSource(map)); } + @AwaitsFix(bugUrl = "temporary") public void testObjectsForSourceValue() throws IOException { String fieldName = randomAlphaOfLength(5); FieldHitExtractor fe = getFieldHitExtractor(fieldName, false); @@ -487,11 +460,11 @@ public void testMultipleGeoShapeExtraction() { QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map2)); assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported")); - FieldHitExtractor lenientFe = new FieldHitExtractor(fieldName, - randomBoolean() ? GEO_SHAPE : SHAPE, UTC, false, true); + FieldHitExtractor lenientFe = new FieldHitExtractor(fieldName, randomBoolean() ? GEO_SHAPE : SHAPE, UTC, false, true); assertEquals(new GeoShape(1, 2), lenientFe.extractFromSource(map2)); } + @AwaitsFix(bugUrl = "temporary") public void testGeoPointExtractionFromSource() throws IOException { int layers = randomIntBetween(1, 3); String pathCombined = ""; @@ -529,6 +502,7 @@ public void testGeoPointExtractionFromSource() throws IOException { assertEquals(new GeoShape(lon, lat), fe.extract(hit)); } + @AwaitsFix(bugUrl = "temporary") public void testMultipleGeoPointExtractionFromSource() throws IOException { double lat = randomDoubleBetween(-90, 90, true); double lon = randomDoubleBetween(-180, 180, true); @@ -559,7 +533,12 @@ public void testMultipleGeoPointExtractionFromSource() throws IOException { public void testGeoPointExtractionFromDocValues() { String fieldName = randomAlphaOfLength(5); FieldHitExtractor fe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, true); - DocumentField field = new DocumentField(fieldName, singletonList("2, 1")); + + HashMap point = new HashMap<>(); + point.put("coordinates", Arrays.asList(1, 2)); + point.put("type", "Point"); + DocumentField field = new DocumentField(fieldName, singletonList(point)); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); assertEquals(new GeoShape(1, 2), fe.extract(hit)); hit = new SearchHit(1); @@ -569,13 +548,23 @@ public void testGeoPointExtractionFromDocValues() { public void testGeoPointExtractionFromMultipleDocValues() { String fieldName = randomAlphaOfLength(5); FieldHitExtractor fe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, true); + HashMap point1 = new HashMap<>(); + point1.put("coordinates", Arrays.asList(1, 2)); + point1.put("type", "Point"); + HashMap point2 = new HashMap<>(); + point2.put("coordinates", Arrays.asList(3, 4)); + point2.put("type", "Point"); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, - new DocumentField(fieldName, Arrays.asList("2,1", "3,4"))), null); + new DocumentField(fieldName, Arrays.asList(point1, point2))), null); + QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extract(hit)); assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported")); FieldHitExtractor lenientFe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, true, true); - assertEquals(new GeoShape(1, 2), lenientFe.extract(hit)); + GeoShape expectedShape = new GeoShape(1, 2); + Object actualShape = lenientFe.extract(hit); + assertEquals(expectedShape, actualShape); } private FieldHitExtractor getFieldHitExtractor(String fieldName, boolean useDocValue) { From 6812cdd1f8f98288765fbf0efc62215094fc13e3 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Wed, 20 Jan 2021 16:47:56 +0200 Subject: [PATCH 2/7] Start removing stuff --- .../extractor/AbstractFieldHitExtractor.java | 105 +----------------- .../search/extractor/FieldHitExtractor.java | 5 +- 2 files changed, 10 insertions(+), 100 deletions(-) diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java index 96af119a69599..0a446b9feabaf 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java @@ -9,11 +9,8 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.document.DocumentField; -import org.elasticsearch.common.geo.GeoPoint; -import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.index.mapper.IgnoredFieldMapper; import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType; import org.elasticsearch.search.SearchHit; import org.elasticsearch.xpack.ql.QlIllegalArgumentException; @@ -57,8 +54,6 @@ private static String[] sourcePath(String name, boolean useDocValue, String hitN private final boolean useDocValue; private final boolean arrayLeniency; private final String[] path; - private final InternalHitExtractor hitExtractorDelegate; - protected final GeoPointParser geoPointParserDelegate; protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue) { this(name, null, dataType, zoneId, useDocValue, null, false); @@ -85,22 +80,11 @@ protected AbstractFieldHitExtractor(String name, String fullFieldName, DataType } this.path = sourcePath(fieldName, useDocValue, hitName); - hitExtractorDelegate = new FieldsApiFieldHitExtractor(); - geoPointParserDelegate = new FieldsApiGeoPointParser(); } protected AbstractFieldHitExtractor(StreamInput in) throws IOException { fieldName = in.readString(); - if (in.getVersion().onOrAfter(SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION) - && in.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { - fullFieldName = in.readOptionalString(); - hitExtractorDelegate = new SourceDocValuesFieldHitExtractor(); - geoPointParserDelegate = new SourceDocValuesGeoPointParser(); - } else { - fullFieldName = null; - hitExtractorDelegate = new FieldsApiFieldHitExtractor(); - geoPointParserDelegate = new FieldsApiGeoPointParser(); - } + fullFieldName = null; String typeName = in.readOptionalString(); dataType = typeName != null ? loadTypeFromName(typeName) : null; if (in.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { @@ -141,11 +125,12 @@ public void writeTo(StreamOutput out) throws IOException { @Override public Object extract(SearchHit hit) { - if (hitExtractorDelegate != null) { - return hitExtractorDelegate.extract(hit); - } else { - return new FieldsApiFieldHitExtractor().extract(hit); + Object value = null; + DocumentField field = hit.field(fieldName); + if (field != null) { + value = unwrapFieldsMultiValue(field.getValues()); } + return value; } protected Object unwrapFieldsMultiValue(Object values) { @@ -363,82 +348,4 @@ public boolean equals(Object obj) { public int hashCode() { return Objects.hash(fieldName, useDocValue, hitName, arrayLeniency); } - - /* - * Logic specific to extraction preferably from _source when possible, falling back to extraction from doc_values otherwise. - * This has been introduced in 7.4.0 and not needed anymore starting with 7.10.0 - */ - private class SourceDocValuesFieldHitExtractor implements InternalHitExtractor { - public Object extract(SearchHit hit) { - Object value = null; - if (useDocValue) { - DocumentField field = hit.field(fieldName); - if (field != null) { - value = unwrapMultiValue(field.getValues()); - } - } else { - // if the field was ignored because it was malformed and ignore_malformed was turned on - if (fullFieldName != null - && hit.getFields().containsKey(IgnoredFieldMapper.NAME) - && isFromDocValuesOnly(dataType) == false - && dataType.isNumeric()) { - /* - * ignore_malformed makes sense for extraction from _source for numeric fields only. - * And we check here that the data type is actually a numeric one to rule out - * any non-numeric sub-fields (for which the "parent" field should actually be extracted from _source). - * For example, in the case of a malformed number, a "byte" field with "ignore_malformed: true" - * with a "text" sub-field should return "null" for the "byte" parent field and the actual malformed - * data for the "text" sub-field. Also, the _ignored section of the response contains the full field - * name, thus the need to do the comparison with that and not only the field name. - */ - if (hit.getFields().get(IgnoredFieldMapper.NAME).getValues().contains(fullFieldName)) { - return null; - } - } - Map source = hit.getSourceAsMap(); - if (source != null) { - value = extractFromSource(source); - } - } - return value; - } - } - - /* - * Logic specific to extraction from "fields" API (introduced in 7.10.0) - */ - private class FieldsApiFieldHitExtractor implements InternalHitExtractor { - public Object extract(SearchHit hit) { - Object value = null; - DocumentField field = hit.field(fieldName); - if (field != null) { - value = unwrapFieldsMultiValue(field.getValues()); - } - return value; - } - } - - private interface InternalHitExtractor { - Object extract(SearchHit hit); - } - - private class SourceDocValuesGeoPointParser implements GeoPointParser { - @Override - public GeoPoint parse(Object values) { - return GeoUtils.parseGeoPoint(values, true); - } - } - - private class FieldsApiGeoPointParser implements GeoPointParser { - @Override - public GeoPoint parse(Object values) { - @SuppressWarnings("unchecked") - Map map = (Map) values; - return GeoUtils.parseGeoPoint(map.get("coordinates"), true); - } - } - - public interface GeoPointParser { - GeoPoint parse(Object values); - } } \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index b3a3895525f26..a2733830297cc 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -7,6 +7,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.xpack.ql.execution.search.extractor.AbstractFieldHitExtractor; import org.elasticsearch.xpack.ql.execution.search.extractor.HitExtractor; @@ -100,7 +101,9 @@ protected Object unwrapCustomValue(Object values) { if (dataType == GEO_POINT) { try { - GeoPoint geoPoint = geoPointParserDelegate.parse(values); + @SuppressWarnings("unchecked") + Map map = (Map) values; + GeoPoint geoPoint = GeoUtils.parseGeoPoint(map.get("coordinates"), true); return new GeoShape(geoPoint.lon(), geoPoint.lat()); } catch (ElasticsearchParseException ex) { throw new SqlIllegalArgumentException("Cannot parse geo_point value [{}] (returned by [{}])", values, fieldName()); From 2f50d7c13b5646f00ca20129c06e2b57455825b6 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 26 Jan 2021 10:45:49 +0200 Subject: [PATCH 3/7] IT Tests work for 7.11 --- x-pack/plugin/sql/qa/mixed-node/build.gradle | 6 +- .../xpack/sql/qa/mixed_node/SqlSearchIT.java | 264 ++++++++++++------ .../src/test/resources/all_field_types.json | 59 ++++ .../xpack/sql/qa/FieldExtractorTestCase.java | 37 +-- .../xpack/sql/action/SqlQueryResponse.java | 9 +- .../xpack/sql/execution/PlanExecutor.java | 2 - .../xpack/sql/execution/search/Querier.java | 4 +- 7 files changed, 271 insertions(+), 110 deletions(-) create mode 100644 x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json diff --git a/x-pack/plugin/sql/qa/mixed-node/build.gradle b/x-pack/plugin/sql/qa/mixed-node/build.gradle index fded43d44955c..bbbce0556e15d 100644 --- a/x-pack/plugin/sql/qa/mixed-node/build.gradle +++ b/x-pack/plugin/sql/qa/mixed-node/build.gradle @@ -10,11 +10,14 @@ import org.elasticsearch.gradle.testclusters.StandaloneRestIntegTestTask dependencies { testImplementation project(':x-pack:qa') + testImplementation project(path: xpackModule('ql'), configuration: 'default') + testImplementation project(path: xpackModule('sql'), configuration: 'default') } tasks.named("integTest").configure{ enabled = false} -for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it.onOrAfter('7.8.0') }) { +//for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it.onOrAfter('7.9.0') }) { +for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it == Version.fromString("7.11.0") }) { if (bwcVersion == VersionProperties.getElasticsearchVersion()) { // Not really a mixed cluster continue; @@ -31,6 +34,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it.on setting 'xpack.watcher.enabled', 'false' setting 'xpack.ml.enabled', 'false' setting 'xpack.license.self_generated.type', 'trial' + setting 'logger.org.elasticsearch.xpack.sql.plugin.TransportSqlQueryAction', 'TRACE' } } diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java index 73ac7394aeaa3..3630ce53d23df 100644 --- a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java @@ -18,22 +18,30 @@ import org.elasticsearch.test.NotEqualMessageBuilder; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.yaml.ObjectPath; +import org.elasticsearch.xpack.sql.type.SqlDataTypes; +import org.junit.After; import org.junit.Before; +import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.function.BiConsumer; +import java.util.function.Consumer; import java.util.stream.Collectors; -import static java.util.Collections.singletonList; import static java.util.Collections.unmodifiableMap; public class SqlSearchIT extends ESRestTestCase { - private static String index = "test_sql_mixed_versions"; + private static final Version FIELDS_API_QL_INTRODUCTION = Version.V_7_12_0; + private static final String index = "test_sql_mixed_versions"; private static int numShards; private static int numReplicas = 1; private static int numDocs; @@ -43,10 +51,10 @@ public class SqlSearchIT extends ESRestTestCase { private static List bwcNodes; private static Version bwcVersion; private static Version newVersion; - private static Map expectedResponse; + private static boolean isBeforeFieldsApiInQL; @Before - public void prepareTestData() throws IOException { + public void createIndex() throws IOException { nodes = buildNodeAndVersions(client()); numShards = nodes.size(); numDocs = randomIntBetween(numShards, 16); @@ -59,55 +67,169 @@ public void prepareTestData() throws IOException { bwcNodes.addAll(nodes.getBWCNodes()); bwcVersion = nodes.getBWCNodes().get(0).getVersion(); newVersion = nodes.getNewNodes().get(0).getVersion(); + isBeforeFieldsApiInQL = bwcVersion.before(FIELDS_API_QL_INTRODUCTION); + + String mappings = readResource("/all_field_types.json"); + createIndex( + index, + Settings.builder() + .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build(), + mappings + ); + } - if (indexExists(index) == false) { - expectedResponse = new HashMap<>(); - expectedResponse.put("columns", singletonList(columnInfo("test", "text"))); - List> rows = new ArrayList<>(numDocs); - expectedResponse.put("rows", rows); - - createIndex( - index, - Settings.builder() - .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), numShards) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) - .build() - ); - for (int i = 0; i < numDocs; i++) { - String randomValue = "test_" + randomAlphaOfLength(3); - Request request = new Request("PUT", index + "/_doc/" + i); - request.setJsonEntity("{\"test\": \"" + randomValue + "\",\"id\":" + i + "}"); - assertOK(client().performRequest(request)); - rows.add(singletonList(randomValue)); - } + @After + public void cleanUpIndex() throws IOException { + if (indexExists(index) == true) { + deleteIndex(index); } } - public void testQueryToUpgradedNodes() throws Exception { - try ( - RestClient client = buildClient(restClientSettings(), newNodes.stream().map(Node::getPublishAddress).toArray(HttpHost[]::new)) - ) { - Request request = new Request("POST", "_sql"); - request.setJsonEntity( - "{\"mode\":\"jdbc\",\"version\":\"" - + newVersion.toString() - + "\",\"binary_format\":\"false\",\"query\":\"SELECT test FROM " - + index - + " ORDER BY id\"}" - ); - assertBusy(() -> { assertResponse(expectedResponse, runSql(client, request)); }); + public void testAllTypesWithRequestToOldNodes() throws Exception { + Map expectedResponse = prepareTestData( + columns -> { + columns.add(columnInfo("geo_point_field", "geo_point")); + columns.add(columnInfo("float_field", "float")); + columns.add(columnInfo("half_float_field", "half_float")); + }, + (builder, fieldValues) -> { + Float randomFloat = randomFloat(); + // before "fields" API being added to QL, numbers were re-parsed from _source with a similar approach to + // indexing docvalues and for floating point numbers this may be different from the actual value passed in the _source + // floats were indexed as Doubles and the values returned had a greater precision and more decimals + builder.append(","); + if (isBeforeFieldsApiInQL) { + builder.append("\"geo_point_field\":{\"lat\":\"37.386483\", \"lon\":\"-122.083843\"},"); + fieldValues.put("geo_point_field", "POINT (-122.08384302444756 37.38648299127817)"); + builder.append("\"float_field\":" + randomFloat + ","); + fieldValues.put("float_field", Double.valueOf(randomFloat)); + builder.append("\"half_float_field\":123.456"); + fieldValues.put("half_float_field", 123.45600128173828d); + } else { + builder.append("\"geo_point_field\":{\"lat\":\"37.386483\", \"lon\":\"-122.083843\"},"); + fieldValues.put("geo_point_field", "POINT (-122.083843 37.386483)"); + builder.append("\"float_field\":" + randomFloat + ","); + fieldValues.put("float_field", Double.valueOf(Float.valueOf(randomFloat).toString())); + builder.append("\"half_float_field\":" + fieldValues.computeIfAbsent("half_float_field", v -> 123.456)); + } + } + ); + assertAllTypesWithNodes(expectedResponse, bwcNodes); + } + + public void testAllTypesWithRequestToUpgradedNodes() throws Exception { + Map expectedResponse = prepareTestData( + columns -> { + columns.add(columnInfo("geo_point_field", "geo_point")); + columns.add(columnInfo("float_field", "float")); + columns.add(columnInfo("half_float_field", "half_float")); + }, + (builder, fieldValues) -> { + builder.append(","); + builder.append("\"geo_point_field\":{\"lat\":\"37.386483\", \"lon\":\"-122.083843\"},"); + fieldValues.put("geo_point_field", "POINT (-122.083843 37.386483)"); + Float randomFloat = randomFloat(); + builder.append("\"float_field\":" + randomFloat + ","); + fieldValues.put("float_field", Double.valueOf(Float.valueOf(randomFloat).toString())); + builder.append("\"half_float_field\":" + fieldValues.computeIfAbsent("half_float_field", v -> 123.456)); + } + ); + assertAllTypesWithNodes(expectedResponse, newNodes); + } + + @SuppressWarnings("unchecked") + private Map prepareTestData(Consumer>> additionalColumns, + BiConsumer> additionalValues) throws IOException { + Map expectedResponse = new HashMap<>(); + List> columns = new ArrayList<>(); + columns.add(columnInfo("long_field", "long")); + columns.add(columnInfo("integer_field", "integer")); + columns.add(columnInfo("short_field", "short")); + columns.add(columnInfo("byte_field", "byte")); + columns.add(columnInfo("double_field", "double")); + columns.add(columnInfo("scaled_float_field", "scaled_float")); + columns.add(columnInfo("boolean_field", "boolean")); + columns.add(columnInfo("ip_field", "ip")); + columns.add(columnInfo("text_field", "text")); + columns.add(columnInfo("keyword_field", "keyword")); + columns.add(columnInfo("constant_keyword_field", "keyword")); + columns.add(columnInfo("wildcard_field", "keyword")); + columns.add(columnInfo("geo_point_no_dv_field", "geo_point")); + columns.add(columnInfo("geo_shape_field", "geo_shape")); + columns.add(columnInfo("shape_field", "shape")); + + expectedResponse.put("columns", columns); + additionalColumns.accept(columns); + List> rows = new ArrayList<>(numDocs); + expectedResponse.put("rows", rows); + + Map fieldValues; + String constantKeywordValue = randomAlphaOfLength(5); + for (int i = 0; i < numDocs; i++) { + fieldValues = new LinkedHashMap<>(); + StringBuilder builder = new StringBuilder(); + builder.append("{"); + builder.append("\"id\":" + i + ","); + builder.append("\"long_field\":" + fieldValues.computeIfAbsent("long_field", v -> randomLong()) + ","); + builder.append("\"integer_field\":" + fieldValues.computeIfAbsent("integer_field", v -> randomInt()) + ","); + builder.append("\"short_field\":" + fieldValues.computeIfAbsent("short_field", v -> Integer.valueOf(randomShort())) + ","); + builder.append("\"byte_field\":" + fieldValues.computeIfAbsent("byte_field", v -> Integer.valueOf(randomByte())) + ","); + builder.append("\"double_field\":" + fieldValues.computeIfAbsent("double_field", v -> randomDouble()) + ","); + builder.append("\"scaled_float_field\":" + fieldValues.computeIfAbsent("scaled_float_field", v -> 123.5d) + ","); + builder.append("\"boolean_field\":" + fieldValues.computeIfAbsent("boolean_field", v -> randomBoolean()) + ","); + builder.append("\"ip_field\":\"" + fieldValues.computeIfAbsent("ip_field", v -> "123.123.123.123") + "\","); + builder.append("\"text_field\": \"" + fieldValues.computeIfAbsent("text_field", v -> randomAlphaOfLength(5)) + "\","); + builder.append("\"keyword_field\": \"" + fieldValues.computeIfAbsent("keyword_field", v -> randomAlphaOfLength(5)) + "\","); + builder.append("\"constant_keyword_field\": \"" + fieldValues.computeIfAbsent("constant_keyword_field", + v -> constantKeywordValue) + "\","); + builder.append("\"wildcard_field\": \"" + fieldValues.computeIfAbsent("wildcard_field", v -> randomAlphaOfLength(5)) + "\","); + builder.append("\"geo_point_no_dv_field\":{\"lat\":\"40.123456\", \"lon\":\"100.234567\"},"); + fieldValues.put("geo_point_no_dv_field", "POINT (100.234567 40.123456)"); + builder.append("\"geo_shape_field\":\"POINT (-122.083843 37.386483 30)\","); + fieldValues.put("geo_shape_field", "POINT (-122.083843 37.386483 30.0)"); + builder.append("\"shape_field\":\"POINT (-122.083843 37.386483 30)\""); + fieldValues.put("shape_field", "POINT (-122.083843 37.386483 30.0)"); + additionalValues.accept(builder, fieldValues); + builder.append("}"); + + Request request = new Request("PUT", index + "/_doc/" + i); + request.setJsonEntity(builder.toString()); + assertOK(client().performRequest(request)); + + List row = new ArrayList<>(fieldValues.values()); + rows.add(row); } + return expectedResponse; } - public void testQueryToOldNodes() throws Exception { + private Map columnInfo(String name, String type) { + Map column = new HashMap<>(); + column.put("name", name); + column.put("type", type); + //if (bwcVersion.onOrAfter(Version.V_7_2_0)) { + column.put("display_size", SqlDataTypes.displaySize(SqlDataTypes.fromTypeName(type))); + //} else { + //column.put("display_size", 0); + //} + return unmodifiableMap(column); + } + + private void assertAllTypesWithNodes(Map expectedResponse, List nodesList) throws Exception { try ( - RestClient client = buildClient(restClientSettings(), bwcNodes.stream().map(Node::getPublishAddress).toArray(HttpHost[]::new)) + RestClient client = buildClient(restClientSettings(), nodesList.stream().map(Node::getPublishAddress).toArray(HttpHost[]::new)) ) { Request request = new Request("POST", "_sql"); String versionSupport = bwcVersion.onOrAfter(Version.V_7_7_0) ? ",\"version\":\"" + newVersion.toString() + "\"" : ""; String binaryFormatSupport = bwcVersion.onOrAfter(Version.V_7_6_0) ? ",\"binary_format\":\"false\"" : ""; + + @SuppressWarnings("unchecked") + List> columns = (List>) expectedResponse.get("columns"); + String fieldsList = columns.stream().map(m -> (String) m.get("name")).collect(Collectors.toList()).stream() + .collect(Collectors.joining(", ")); request.setJsonEntity( - "{\"mode\":\"jdbc\"" + versionSupport + binaryFormatSupport + ",\"query\":\"SELECT test FROM " + "{\"mode\":\"jdbc\"" + versionSupport + binaryFormatSupport + ",\"query\":\"SELECT " + fieldsList + " FROM " + index + " ORDER BY id\"}" ); @@ -115,18 +237,6 @@ public void testQueryToOldNodes() throws Exception { } } - private Map columnInfo(String name, String type) { - Map column = new HashMap<>(); - column.put("name", name); - column.put("type", type); - if (bwcVersion.onOrAfter(Version.V_7_2_0)) { - column.put("display_size", 2147483647); - } else { - column.put("display_size", 0); - } - return unmodifiableMap(column); - } - private void assertResponse(Map expected, Map actual) { if (false == expected.equals(actual)) { NotEqualMessageBuilder message = new NotEqualMessageBuilder(); @@ -142,6 +252,23 @@ private Map runSql(RestClient client, Request request) throws IO } } + private static String readResource(String location) throws IOException { + StringBuilder builder = new StringBuilder(); + try (BufferedReader reader = new BufferedReader(new InputStreamReader(SqlSearchIT.class.getResourceAsStream(location), + StandardCharsets.UTF_8))) + { + String line = reader.readLine(); + while (line != null) { + if (line.trim().startsWith("//") == false) { + builder.append(line); + builder.append('\n'); + } + line = reader.readLine(); + } + return builder.toString(); + } + } + static Nodes buildNodeAndVersions(RestClient client) throws IOException { Response response = client.performRequest(new Request("GET", "_nodes")); ObjectPath objectPath = ObjectPath.createFromResponse(response); @@ -151,31 +278,15 @@ static Nodes buildNodeAndVersions(RestClient client) throws IOException { nodes.add( new Node( id, - objectPath.evaluate("nodes." + id + ".name"), Version.fromString(objectPath.evaluate("nodes." + id + ".version")), HttpHost.create(objectPath.evaluate("nodes." + id + ".http.publish_address")) ) ); } - response = client.performRequest(new Request("GET", "_cluster/state")); - nodes.setMasterNodeId(ObjectPath.createFromResponse(response).evaluate("master_node")); return nodes; } - static final class Nodes extends HashMap { - - private String masterNodeId = null; - - public Node getMaster() { - return get(masterNodeId); - } - - public void setMasterNodeId(String id) { - if (get(id) == null) { - throw new IllegalArgumentException("node with id [" + id + "] not found. got:" + toString()); - } - masterNodeId = id; - } + private static final class Nodes extends HashMap { public void add(Node node) { put(node.getId(), node); @@ -201,23 +312,18 @@ public Version getBWCVersion() { @Override public String toString() { return "Nodes{" - + "masterNodeId='" - + masterNodeId - + "'\n" + values().stream().map(Node::toString).collect(Collectors.joining("\n")) + '}'; } } - static final class Node { + private static final class Node { private final String id; - private final String nodeName; private final Version version; private final HttpHost publishAddress; - Node(String id, String nodeName, Version version, HttpHost publishAddress) { + Node(String id, Version version, HttpHost publishAddress) { this.id = id; - this.nodeName = nodeName; this.version = version; this.publishAddress = publishAddress; } @@ -226,10 +332,6 @@ public String getId() { return id; } - public String getNodeName() { - return nodeName; - } - public HttpHost getPublishAddress() { return publishAddress; } @@ -240,7 +342,7 @@ public Version getVersion() { @Override public String toString() { - return "Node{" + "id='" + id + '\'' + ", nodeName='" + nodeName + '\'' + ", version=" + version + '}'; + return "Node{" + "id='" + id + '\'' + ", version=" + version + '}'; } } } diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json b/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json new file mode 100644 index 0000000000000..6e01c8f0baecb --- /dev/null +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json @@ -0,0 +1,59 @@ + "properties": { + "long_field": { + "type": "long" + }, + "integer_field": { + "type": "integer" + }, + "short_field": { + "type": "short" + }, + "byte_field": { + "type": "byte" + }, + "double_field": { + "type": "double" + }, + "float_field": { + "type": "float" + }, + "half_float_field": { + "type": "half_float" + }, + "scaled_float_field": { + "type": "scaled_float", + "scaling_factor": 10 + }, + "boolean_field": { + "type": "boolean" + }, + "ip_field": { + "type": "ip" + }, + "text_field": { + "type": "text" + }, + "keyword_field": { + "type": "keyword" + }, + "constant_keyword_field": { + "type": "constant_keyword" + }, + // added in 7.9.0 + "wildcard_field": { + "type": "wildcard" + }, + "geo_point_field": { + "type": "geo_point" + }, + "geo_point_no_dv_field": { + "type": "geo_point", + "doc_values": "false" + }, + "geo_shape_field": { + "type": "geo_shape" + }, + "shape_field": { + "type": "shape" + } + } \ No newline at end of file diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java index e2e3677282cb5..45e7eb6806f68 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java @@ -396,7 +396,6 @@ public void testIpField() throws IOException { public void testGeoPointField() throws IOException { String query = "SELECT geo_point_field FROM test"; String geoPointField = "41.12,-71.34"; - String geoPointFromDocValues = "POINT (-71.34000004269183 41.1199999647215)"; String actualValue = geoPointField; boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting boolean enableSource = randomBoolean(); // enable _source at index level @@ -417,11 +416,17 @@ public void testGeoPointField() throws IOException { createIndexWithFieldTypeAndProperties("geo_point", fieldProps, explicitSourceSetting ? indexProps : null); index("{\"geo_point_field\":\"" + actualValue + "\"}"); - // the values come from docvalues (vs from _source) so disabling the source doesn't have any impact on the values returned - Map expected = new HashMap<>(); - expected.put("columns", Arrays.asList(columnInfo("plain", "geo_point_field", "geo_point", JDBCType.VARCHAR, Integer.MAX_VALUE))); - expected.put("rows", singletonList(singletonList(ignoreMalformed ? null : geoPointFromDocValues))); - assertResponse(expected, runSql(query)); + if (explicitSourceSetting == false || enableSource) { + Map expected = new HashMap<>(); + expected.put( + "columns", + Arrays.asList(columnInfo("plain", "geo_point_field", "geo_point", JDBCType.VARCHAR, Integer.MAX_VALUE)) + ); + expected.put("rows", singletonList(singletonList(ignoreMalformed ? null : "POINT (-71.34 41.12)"))); + assertResponse(expected, runSql(query)); + } else { + expectSourceDisabledError(query); + } } /* @@ -471,7 +476,6 @@ public void testGeoShapeField() throws IOException { * "ignore_malformed": true/false * } */ - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/66678") public void testShapeField() throws IOException { String query = "SELECT shape_field FROM test"; String shapeField = "POINT (-377.03653 389.897676)"; @@ -498,7 +502,7 @@ public void testShapeField() throws IOException { if (explicitSourceSetting == false || enableSource) { Map expected = new HashMap<>(); expected.put("columns", Arrays.asList(columnInfo("plain", "shape_field", "shape", JDBCType.VARCHAR, Integer.MAX_VALUE))); - expected.put("rows", singletonList(singletonList(ignoreMalformed ? null : "POINT (-377.03653 389.897676)"))); + expected.put("rows", singletonList(singletonList(ignoreMalformed ? null : shapeField))); assertResponse(expected, runSql(query)); } else { expectSourceDisabledError(query); @@ -901,22 +905,9 @@ public void testIpFieldWithTextOrKeywordSubfield() throws IOException { } assertResponse(expected, runSql(query)); } else { - if (isKeyword) { - // selecting only the keyword subfield when the _source is disabled should work - Map expected = new HashMap<>(); - expected.put("columns", singletonList(columnInfo("plain", subFieldName, "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE))); - if (ignoreMalformed) { - expected.put("rows", singletonList(singletonList("foo"))); - } else { - expected.put("rows", singletonList(singletonList(ip))); - } - assertResponse(expected, runSql("SELECT ip_field.keyword_subfield FROM test")); - } else { - expectSourceDisabledError(query); - } - - // if the _source is disabled, selecting only the ip field shouldn't work + expectSourceDisabledError(query); expectSourceDisabledError("SELECT " + fieldName + " FROM test"); + expectSourceDisabledError("SELECT " + subFieldName + " FROM test"); } } diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java index 63395611e4d46..0d637f6a53c5d 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java @@ -8,8 +8,10 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.xpack.sql.proto.ColumnInfo; @@ -138,7 +140,12 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVInt(rows.get(0).size()); for (List row : rows) { for (Object value : row) { - out.writeGenericValue(value); + // GeoShape and Interval + if (value instanceof NamedWriteable) { + out.writeNamedWriteable((NamedWriteable) value); + } else { + out.writeGenericValue(value); + } } } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java index 1c7ca44b6a32e..ac468fd7de652 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.sql.execution; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Client; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -35,7 +34,6 @@ import static org.elasticsearch.action.ActionListener.wrap; public class PlanExecutor { - public static final Version FIELDS_API_INTRODUCTION_VERSION = Version.V_7_10_0; private final Client client; private final NamedWriteableRegistry writableRegistry; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java index 237d91f4e0e00..b11d039bb57a9 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java @@ -8,6 +8,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.PriorityQueue; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; @@ -83,11 +84,10 @@ import static java.util.Collections.singletonList; import static org.elasticsearch.action.ActionListener.wrap; -import static org.elasticsearch.xpack.sql.execution.PlanExecutor.FIELDS_API_INTRODUCTION_VERSION; // TODO: add retry/back-off public class Querier { - + public static final Version FIELDS_API_INTRODUCTION_VERSION = Version.V_7_10_0; private static final Logger log = LogManager.getLogger(Querier.class); private final PlanExecutor planExecutor; From b1475a9ff76cb47ba13ed1fad2b88d4f24a4f868 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 26 Jan 2021 15:18:17 +0200 Subject: [PATCH 4/7] Remove "useDocValue" --- .../eql/execution/search/RuntimeUtils.java | 2 +- .../search/extractor/FieldHitExtractor.java | 4 +- .../extractor/TimestampFieldHitExtractor.java | 2 +- .../CriterionOrdinalExtractionTests.java | 6 +- .../extractor/AbstractFieldHitExtractor.java | 200 +------ .../xpack/sql/execution/search/Querier.java | 4 +- .../search/extractor/FieldHitExtractor.java | 18 +- .../extractor/ComputingExtractorTests.java | 2 +- .../extractor/FieldHitExtractorTests.java | 506 ++---------------- 9 files changed, 76 insertions(+), 668 deletions(-) diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java index def27b5ccd414..84c02713d9d64 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java @@ -116,7 +116,7 @@ public static List createExtractor(List fields, E public static HitExtractor createExtractor(FieldExtraction ref, EqlConfiguration cfg) { if (ref instanceof SearchHitFieldRef) { SearchHitFieldRef f = (SearchHitFieldRef) ref; - return new FieldHitExtractor(f.name(), f.fullFieldName(), f.getDataType(), cfg.zoneId(), f.useDocValue(), f.hitName(), false); + return new FieldHitExtractor(f.name(), f.fullFieldName(), f.getDataType(), cfg.zoneId(), f.hitName(), false); } if (ref instanceof ComputedRef) { diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java index b6cfee5efc899..7a23caca69898 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java @@ -27,9 +27,9 @@ public FieldHitExtractor(StreamInput in) throws IOException { super(in); } - public FieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, boolean useDocValue, String hitName, + public FieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, String hitName, boolean arrayLeniency) { - super(name, fullFieldName, dataType, zoneId, useDocValue, hitName, arrayLeniency); + super(name, fullFieldName, dataType, zoneId, hitName, arrayLeniency); } @Override diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/TimestampFieldHitExtractor.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/TimestampFieldHitExtractor.java index fd5c54fd0454e..4a55d4b5a5475 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/TimestampFieldHitExtractor.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/TimestampFieldHitExtractor.java @@ -9,7 +9,7 @@ public class TimestampFieldHitExtractor extends FieldHitExtractor { public TimestampFieldHitExtractor(FieldHitExtractor target) { - super(target.fieldName(), target.fullFieldName(), target.dataType(), target.zoneId(), target.useDocValues(), target.hitName(), + super(target.fieldName(), target.fullFieldName(), target.dataType(), target.zoneId(), target.hitName(), target.arrayLeniency()); } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java index 89e4dc9b37e8a..7212ff560077a 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java @@ -30,8 +30,8 @@ public class CriterionOrdinalExtractionTests extends ESTestCase { private String tsField = "timestamp"; private String tbField = "tiebreaker"; - private HitExtractor tsExtractor = new FieldHitExtractor(tsField, tsField, DataTypes.LONG, null, true, null, false); - private HitExtractor tbExtractor = new FieldHitExtractor(tbField, tbField, DataTypes.LONG, null, true, null, false); + private HitExtractor tsExtractor = new FieldHitExtractor(tsField, tsField, DataTypes.LONG, null, null, false); + private HitExtractor tbExtractor = new FieldHitExtractor(tbField, tbField, DataTypes.LONG, null, null, false); public void testTimeOnly() throws Exception { long time = randomLong(); @@ -56,7 +56,7 @@ public void testTimeAndTiebreakerNull() throws Exception { } public void testTimeNotComparable() throws Exception { - HitExtractor badExtractor = new FieldHitExtractor(tsField, tsField, DataTypes.BINARY, null, true, null, false); + HitExtractor badExtractor = new FieldHitExtractor(tsField, tsField, DataTypes.BINARY, null, null, false); SearchHit hit = searchHit(randomAlphaOfLength(10), null); Criterion criterion = new Criterion(0, null, emptyList(), badExtractor, null, false); EqlIllegalArgumentException exception = expectThrows(EqlIllegalArgumentException.class, () -> criterion.ordinal(hit)); diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java index 0a446b9feabaf..ec50e1599d262 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java @@ -5,13 +5,9 @@ */ package org.elasticsearch.xpack.ql.execution.search.extractor; -import org.elasticsearch.Version; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.document.DocumentField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType; import org.elasticsearch.search.SearchHit; import org.elasticsearch.xpack.ql.QlIllegalArgumentException; import org.elasticsearch.xpack.ql.type.DataType; @@ -19,57 +15,34 @@ import java.io.IOException; import java.time.ZoneId; -import java.util.ArrayDeque; -import java.util.Deque; import java.util.List; -import java.util.Locale; -import java.util.Map; import java.util.Objects; -import java.util.StringJoiner; - -import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; -import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD; -import static org.elasticsearch.xpack.ql.type.DataTypes.SCALED_FLOAT; /** * Extractor for ES fields. Works for both 'normal' fields but also nested ones (which require hitName to be set). * The latter is used as metadata in assembling the results in the tabular response. */ public abstract class AbstractFieldHitExtractor implements HitExtractor { - private static final Version SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION = Version.V_7_4_0; - private static final Version SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API = Version.V_7_10_0; - - /** - * Source extraction requires only the (relative) field name, without its parent path. - */ - private static String[] sourcePath(String name, boolean useDocValue, String hitName) { - return useDocValue ? Strings.EMPTY_ARRAY : Strings - .tokenizeToStringArray(hitName == null ? name : name.substring(hitName.length() + 1), "."); - } - private final String fieldName, hitName; private final String fullFieldName; // used to look at the _ignored section of the query response for the actual full field name private final DataType dataType; private final ZoneId zoneId; - private final boolean useDocValue; private final boolean arrayLeniency; - private final String[] path; - protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue) { - this(name, null, dataType, zoneId, useDocValue, null, false); + protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId) { + this(name, null, dataType, zoneId, null, false); } - protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue, boolean arrayLeniency) { - this(name, null, dataType, zoneId, useDocValue, null, arrayLeniency); + protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean arrayLeniency) { + this(name, null, dataType, zoneId, null, arrayLeniency); } - protected AbstractFieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, boolean useDocValue, + protected AbstractFieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, String hitName, boolean arrayLeniency) { this.fieldName = name; this.fullFieldName = fullFieldName; this.dataType = dataType; this.zoneId = zoneId; - this.useDocValue = useDocValue; this.arrayLeniency = arrayLeniency; this.hitName = hitName; @@ -78,28 +51,16 @@ protected AbstractFieldHitExtractor(String name, String fullFieldName, DataType throw new QlIllegalArgumentException("Hitname [{}] specified but not part of the name [{}]", hitName, name); } } - - this.path = sourcePath(fieldName, useDocValue, hitName); } protected AbstractFieldHitExtractor(StreamInput in) throws IOException { fieldName = in.readString(); - fullFieldName = null; + fullFieldName = in.readOptionalString(); String typeName = in.readOptionalString(); dataType = typeName != null ? loadTypeFromName(typeName) : null; - if (in.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { - useDocValue = in.readBoolean(); - } else { - useDocValue = false; // for "fields" API usage, extraction from _source or from docvalues doesn't matter - } hitName = in.readOptionalString(); arrayLeniency = in.readBoolean(); zoneId = readZoneId(in); - if (in.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { - path = sourcePath(fieldName, useDocValue, hitName); - } else { - path = null; - } } protected DataType loadTypeFromName(String typeName) { @@ -111,14 +72,8 @@ protected DataType loadTypeFromName(String typeName) { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(fieldName); - if (out.getVersion().onOrAfter(SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION) - && out.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { - out.writeOptionalString(fullFieldName); - } + out.writeOptionalString(fullFieldName); out.writeOptionalString(dataType == null ? null : dataType.typeName()); - if (out.getVersion().before(SWITCHED_FROM_SOURCE_EXTRACTION_TO_FIELDS_API)) { - out.writeBoolean(useDocValue); - } out.writeOptionalString(hitName); out.writeBoolean(arrayLeniency); } @@ -160,144 +115,10 @@ protected Object unwrapFieldsMultiValue(Object values) { return values; } - protected Object unwrapMultiValue(Object values) { - if (values == null) { - return null; - } - if (values instanceof List) { - List list = (List) values; - if (list.isEmpty()) { - return null; - } else { - if (isPrimitive(list) == false) { - if (list.size() == 1 || arrayLeniency) { - return unwrapMultiValue(list.get(0)); - } else { - throw new QlIllegalArgumentException("Arrays (returned by [{}]) are not supported", fieldName); - } - } - } - } - - Object unwrapped = unwrapCustomValue(values); - if (unwrapped != null) { - return unwrapped; - } - - // The Jackson json parser can generate for numerics - Integers, Longs, BigIntegers (if Long is not enough) - // and BigDecimal (if Double is not enough) - if (values instanceof Number || values instanceof String || values instanceof Boolean) { - if (dataType == null) { - return values; - } - if (dataType.isNumeric() && isFromDocValuesOnly(dataType) == false) { - if (dataType == DataTypes.DOUBLE || dataType == DataTypes.FLOAT || dataType == DataTypes.HALF_FLOAT) { - Number result = null; - try { - result = numberType(dataType).parse(values, true); - } catch(IllegalArgumentException iae) { - return null; - } - // docvalue_fields is always returning a Double value even if the underlying floating point data type is not Double - // even if we don't extract from docvalue_fields anymore, the behavior should be consistent - return result.doubleValue(); - } else { - Number result = null; - try { - result = numberType(dataType).parse(values, true); - } catch(IllegalArgumentException iae) { - return null; - } - return result; - } - } else if (DataTypes.isString(dataType) || dataType == DataTypes.IP) { - return values.toString(); - } else { - return values; - } - } - throw new QlIllegalArgumentException("Type {} (returned by [{}]) is not supported", values.getClass().getSimpleName(), fieldName); - } - - protected boolean isFromDocValuesOnly(DataType dataType) { - return dataType == KEYWORD // because of ignore_above. - || dataType == DATETIME - || dataType == SCALED_FLOAT; // because of scaling_factor - } - - private static NumberType numberType(DataType dataType) { - return NumberType.valueOf(dataType.esType().toUpperCase(Locale.ROOT)); - } - protected abstract Object unwrapCustomValue(Object values); protected abstract boolean isPrimitive(List list); - @SuppressWarnings({ "unchecked", "rawtypes" }) - public Object extractFromSource(Map map) { - Object value = null; - - // Used to avoid recursive method calls - // Holds the sub-maps in the document hierarchy that are pending to be inspected along with the current index of the `path`. - Deque>> queue = new ArrayDeque<>(); - queue.add(new Tuple<>(-1, map)); - - while (!queue.isEmpty()) { - Tuple> tuple = queue.removeLast(); - int idx = tuple.v1(); - Map subMap = tuple.v2(); - - // Find all possible entries by examining all combinations under the current level ("idx") of the "path" - // e.g.: If the path == "a.b.c.d" and the idx == 0, we need to check the current subMap against the keys: - // "b", "b.c" and "b.c.d" - StringJoiner sj = new StringJoiner("."); - for (int i = idx + 1; i < path.length; i++) { - sj.add(path[i]); - Object node = subMap.get(sj.toString()); - - if (node instanceof List) { - List listOfValues = (List) node; - // we can only do this optimization until the last element of our pass since geo points are using arrays - // and we don't want to blindly ignore the second element of array if arrayLeniency is enabled - if ((i < path.length - 1) && (listOfValues.size() == 1 || arrayLeniency)) { - // this is a List with a size of 1 e.g.: {"a" : [{"b" : "value"}]} meaning the JSON is a list with one element - // or a list of values with one element e.g.: {"a": {"b" : ["value"]}} - // in case of being lenient about arrays, just extract the first value in the array - node = listOfValues.get(0); - } else { - // a List of elements with more than one value. Break early and let unwrapMultiValue deal with the list - return unwrapMultiValue(node); - } - } - - if (node instanceof Map) { - if (i < path.length - 1) { - // Add the sub-map to the queue along with the current path index - queue.add(new Tuple<>(i, (Map) node)); - } else { - // We exhausted the path and got a map - // If it is an object - it will be handled in the value extractor - value = node; - } - } else if (node != null) { - if (i < path.length - 1) { - // If we reach a concrete value without exhausting the full path, something is wrong with the mapping - // e.g.: map is {"a" : { "b" : "value }} and we are looking for a path: "a.b.c.d" - throw new QlIllegalArgumentException("Cannot extract value [{}] from source", fieldName); - } - if (value != null) { - // A value has already been found so this means that there are more than one - // values in the document for the same path but different hierarchy. - // e.g.: {"a" : {"b" : {"c" : "value"}}}, {"a.b" : {"c" : "value"}}, ... - throw new QlIllegalArgumentException("Multiple values (returned by [{}]) are not supported", fieldName); - } - value = node; - } - } - } - return unwrapMultiValue(value); - } - @Override public String hitName() { return hitName; @@ -319,10 +140,6 @@ public DataType dataType() { return dataType; } - public boolean useDocValues() { - return useDocValue; - } - public boolean arrayLeniency() { return arrayLeniency; } @@ -340,12 +157,11 @@ public boolean equals(Object obj) { AbstractFieldHitExtractor other = (AbstractFieldHitExtractor) obj; return fieldName.equals(other.fieldName) && hitName.equals(other.hitName) - && useDocValue == other.useDocValue && arrayLeniency == other.arrayLeniency; } @Override public int hashCode() { - return Objects.hash(fieldName, useDocValue, hitName, arrayLeniency); + return Objects.hash(fieldName, hitName, arrayLeniency); } } \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java index b11d039bb57a9..131a01ed14d8d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java @@ -492,13 +492,13 @@ protected void handleResponse(SearchResponse response, ActionListener list private HitExtractor createExtractor(FieldExtraction ref) { if (ref instanceof SearchHitFieldRef) { SearchHitFieldRef f = (SearchHitFieldRef) ref; - return new FieldHitExtractor(f.name(), f.fullFieldName(), f.getDataType(), cfg.zoneId(), f.useDocValue(), f.hitName(), + return new FieldHitExtractor(f.name(), f.fullFieldName(), f.getDataType(), cfg.zoneId(), f.hitName(), multiValueFieldLeniency); } if (ref instanceof ScriptFieldRef) { ScriptFieldRef f = (ScriptFieldRef) ref; - return new FieldHitExtractor(f.name(), null, cfg.zoneId(), true, multiValueFieldLeniency); + return new FieldHitExtractor(f.name(), null, cfg.zoneId(), multiValueFieldLeniency); } if (ref instanceof ComputedRef) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index a2733830297cc..4d4923b44a44b 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -40,17 +40,17 @@ public class FieldHitExtractor extends AbstractFieldHitExtractor { */ static final String NAME = "f"; - public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue, boolean arrayLeniency) { - super(name, dataType, zoneId, useDocValue, arrayLeniency); + public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean arrayLeniency) { + super(name, dataType, zoneId, arrayLeniency); } - public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue) { - super(name, dataType, zoneId, useDocValue); + public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId) { + super(name, dataType, zoneId); } - public FieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, boolean useDocValue, String hitName, + public FieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, String hitName, boolean arrayLeniency) { - super(name, fullFieldName, dataType, zoneId, useDocValue, hitName, arrayLeniency); + super(name, fullFieldName, dataType, zoneId, hitName, arrayLeniency); } public FieldHitExtractor(StreamInput in) throws IOException { @@ -89,12 +89,6 @@ private boolean isGeoPointArray(List list) { return list.get(0) instanceof Number; } - - @Override - protected boolean isFromDocValuesOnly(DataType dataType) { - return SqlDataTypes.isFromDocValuesOnly(dataType); - } - @Override protected Object unwrapCustomValue(Object values) { DataType dataType = dataType(); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java index dc75e1cebcc4d..297fb5758dc5b 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java @@ -73,7 +73,7 @@ protected ComputingExtractor mutateInstance(ComputingExtractor instance) throws public void testGet() { String fieldName = randomAlphaOfLength(5); ChainingProcessor extractor = new ChainingProcessor( - new HitExtractorProcessor(new FieldHitExtractor(fieldName, DOUBLE, UTC, true, false)), + new HitExtractorProcessor(new FieldHitExtractor(fieldName, DOUBLE, UTC, false)), new MathProcessor(MathOperation.LOG)); int times = between(1, 1000); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java index 587e94585311c..e40b0fce57d53 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java @@ -5,11 +5,8 @@ */ package org.elasticsearch.xpack.sql.execution.search.extractor; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.document.DocumentField; import org.elasticsearch.common.io.stream.Writeable.Reader; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.search.SearchHit; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.ql.QlIllegalArgumentException; @@ -28,14 +25,12 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.StringJoiner; import java.util.function.Supplier; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; -import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_POINT; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_SHAPE; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.SHAPE; import static org.elasticsearch.xpack.sql.util.DateUtils.UTC; @@ -46,7 +41,7 @@ public class FieldHitExtractorTests extends AbstractSqlWireSerializingTestCase randomFrom(SqlDataTypes.types())), randomValueOtherThan(instance.zoneId(), ESTestCase::randomZone), - randomBoolean(), instance.hitName() + "mutated", - randomBoolean()); + randomBoolean() + ); } public void testGetDottedValueWithDocValues() { @@ -82,7 +77,7 @@ public void testGetDottedValueWithDocValues() { String child = randomAlphaOfLength(5); String fieldName = grandparent + "." + parent + "." + child; - FieldHitExtractor extractor = getFieldHitExtractor(fieldName, true); + FieldHitExtractor extractor = getFieldHitExtractor(fieldName); int times = between(1, 1000); for (int i = 0; i < times; i++) { @@ -99,53 +94,9 @@ public void testGetDottedValueWithDocValues() { } } - @AwaitsFix(bugUrl = "temporary") - public void testGetDottedValueWithSource() throws Exception { - String grandparent = randomAlphaOfLength(5); - String parent = randomAlphaOfLength(5); - String child = randomAlphaOfLength(5); - String fieldName = grandparent + "." + parent + "." + child; - - FieldHitExtractor extractor = getFieldHitExtractor(fieldName, false); - - int times = between(1, 1000); - for (int i = 0; i < times; i++) { - /* We use values that are parsed from json as "equal" to make the - * test simpler. */ - Object value = randomValue(); - SearchHit hit = new SearchHit(1); - XContentBuilder source = JsonXContent.contentBuilder(); - boolean hasGrandparent = randomBoolean(); - boolean hasParent = randomBoolean(); - boolean hasChild = randomBoolean(); - boolean hasSource = hasGrandparent && hasParent && hasChild; - - source.startObject(); - if (hasGrandparent) { - source.startObject(grandparent); - if (hasParent) { - source.startObject(parent); - if (hasChild) { - source.field(child, value); - if (randomBoolean()) { - source.field(fieldName + randomAlphaOfLength(3), value + randomAlphaOfLength(3)); - } - } - source.endObject(); - } - source.endObject(); - } - source.endObject(); - BytesReference sourceRef = BytesReference.bytes(source); - hit.sourceRef(sourceRef); - Object extract = extractor.extract(hit); - assertFieldHitEquals(hasSource ? value : null, extract); - } - } - public void testGetDocValue() { String fieldName = randomAlphaOfLength(5); - FieldHitExtractor extractor = getFieldHitExtractor(fieldName, true); + FieldHitExtractor extractor = getFieldHitExtractor(fieldName); int times = between(1, 1000); for (int i = 0; i < times; i++) { @@ -170,451 +121,98 @@ public void testGetDate() { assertEquals(DateUtils.asDateTime(millis, zoneId), extractor.extract(hit)); } - @AwaitsFix(bugUrl = "temporary") - public void testGetSource() throws IOException { - String fieldName = randomAlphaOfLength(5); - FieldHitExtractor extractor = getFieldHitExtractor(fieldName, false); - - int times = between(1, 1000); - for (int i = 0; i < times; i++) { - /* We use values that are parsed from json as "equal" to make the - * test simpler. */ - Object value = randomValue(); - SearchHit hit = new SearchHit(1); - XContentBuilder source = JsonXContent.contentBuilder(); - source.startObject(); { - source.field(fieldName, value); - if (randomBoolean()) { - source.field(fieldName + "_random_junk", value + "_random_junk"); - } - } - source.endObject(); - BytesReference sourceRef = BytesReference.bytes(source); - hit.sourceRef(sourceRef); - assertFieldHitEquals(value, extractor.extract(hit)); - } - } - public void testToString() { - assertEquals("hit.field@hit@Europe/Berlin", - new FieldHitExtractor("hit.field", null, null, ZoneId.of("Europe/Berlin"), true, "hit", false).toString()); + assertEquals( + "hit.field@hit@Europe/Berlin", + new FieldHitExtractor("hit.field", null, null, ZoneId.of("Europe/Berlin"), "hit", false).toString() + ); } public void testMultiValuedDocValue() { String fieldName = randomAlphaOfLength(5); - FieldHitExtractor fe = getFieldHitExtractor(fieldName, true); + FieldHitExtractor fe = getFieldHitExtractor(fieldName); DocumentField field = new DocumentField(fieldName, asList("a", "b")); SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extract(hit)); assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported")); } - public void testExtractSourcePath() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c", false); + public void testExtractSourcePath() throws IOException { + FieldHitExtractor fe = getFieldHitExtractor("a.b.c"); Object value = randomValue(); - Map map = singletonMap("a", singletonMap("b", singletonMap("c", value))); - assertThat(fe.extractFromSource(map), is(value)); - } - - public void testExtractSourceIncorrectPath() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c.d", false); - Object value = randomNonNullValue(); - Map map = singletonMap("a", singletonMap("b", singletonMap("c", value))); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map)); - assertThat(ex.getMessage(), is("Cannot extract value [a.b.c.d] from source")); + DocumentField field = new DocumentField("a.b.c", singletonList(value)); + SearchHit hit = new SearchHit(1, null, null, singletonMap("a.b.c", field), null); + assertThat(fe.extract(hit), is(value)); } - + public void testMultiValuedSource() { - FieldHitExtractor fe = getFieldHitExtractor("a", false); + FieldHitExtractor fe = getFieldHitExtractor("a"); Object value = randomValue(); - Map map = singletonMap("a", asList(value, value)); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map)); + DocumentField field = new DocumentField("a", asList(value, value)); + SearchHit hit = new SearchHit(1, null, null, singletonMap("a", field), null); + QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extract(hit)); assertThat(ex.getMessage(), is("Arrays (returned by [a]) are not supported")); } - + public void testMultiValuedSourceAllowed() { - FieldHitExtractor fe = new FieldHitExtractor("a", null, UTC, false, true); + FieldHitExtractor fe = new FieldHitExtractor("a", null, UTC, true); Object valueA = randomValue(); Object valueB = randomValue(); - Map map = singletonMap("a", asList(valueA, valueB)); - assertEquals(valueA, fe.extractFromSource(map)); - } - - public void testFieldWithDots() { - FieldHitExtractor fe = getFieldHitExtractor("a.b", false); - Object value = randomValue(); - Map map = singletonMap("a.b", value); - assertEquals(value, fe.extractFromSource(map)); - } - - public void testNestedFieldWithDots() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c", false); - Object value = randomValue(); - Map map = singletonMap("a", singletonMap("b.c", value)); - assertEquals(value, fe.extractFromSource(map)); - } - - public void testNestedFieldWithDotsWithNestedField() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c.d", false); - Object value = randomValue(); - Map map = singletonMap("a", singletonMap("b.c", singletonMap("d", value))); - assertEquals(value, fe.extractFromSource(map)); - } - - public void testNestedFieldWithDotsWithNestedFieldWithDots() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c.d.e", false); - Object value = randomValue(); - Map map = singletonMap("a", singletonMap("b.c", singletonMap("d.e", value))); - assertEquals(value, fe.extractFromSource(map)); - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - public void testNestedFieldsWithDotsAndRandomHierarchy() { - String[] path = new String[100]; - StringJoiner sj = new StringJoiner("."); - for (int i = 0; i < 100; i++) { - path[i] = randomAlphaOfLength(randomIntBetween(1, 10)); - sj.add(path[i]); - } - boolean arrayLeniency = randomBoolean(); - FieldHitExtractor fe = new FieldHitExtractor(sj.toString(), null, UTC, false, arrayLeniency); - - List paths = new ArrayList<>(path.length); - int start = 0; - while (start < path.length) { - int end = randomIntBetween(start + 1, path.length); - sj = new StringJoiner("."); - for (int j = start; j < end; j++) { - sj.add(path[j]); - } - paths.add(sj.toString()); - start = end; - } - - /* - * Randomize how many values the field to look for will have (1 - 3). It's not really relevant how many values there are in the list - * but that the list has one element or more than one. - * If it has one value, then randomize the way it's indexed: as a single-value array or not e.g.: "a":"value" or "a":["value"]. - * If it has more than one value, it will always be an array e.g.: "a":["v1","v2","v3"]. - */ - int valuesCount = randomIntBetween(1, 3); - Object value = randomValue(); - if (valuesCount == 1) { - value = randomBoolean() ? singletonList(value) : value; - } else { - value = new ArrayList(valuesCount); - for(int i = 0; i < valuesCount; i++) { - ((List) value).add(randomValue()); - } - } - - // the path to the randomly generated fields path - StringBuilder expected = new StringBuilder(paths.get(paths.size() - 1)); - // the actual value we will be looking for in the test at the end - Map map = singletonMap(paths.get(paths.size() - 1), value); - // build the rest of the path and the expected path to check against in the error message - for (int i = paths.size() - 2; i >= 0; i--) { - map = singletonMap(paths.get(i), randomBoolean() ? singletonList(map) : map); - expected.insert(0, paths.get(i) + "."); - } - - if (valuesCount == 1 || arrayLeniency) { - // if the number of generated values is 1, just check we return the correct value - assertEquals(value instanceof List ? ((List) value).get(0) : value, fe.extractFromSource(map)); - } else { - // if we have an array with more than one value in it, check that we throw the correct exception and exception message - final Map map2 = Collections.unmodifiableMap(map); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map2)); - assertThat(ex.getMessage(), is("Arrays (returned by [" + expected + "]) are not supported")); - } - } - - public void testExtractSourceIncorrectPathWithFieldWithDots() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c.d.e", false); - Object value = randomNonNullValue(); - Map map = singletonMap("a", singletonMap("b.c", singletonMap("d", value))); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map)); - assertThat(ex.getMessage(), is("Cannot extract value [a.b.c.d.e] from source")); - } - - public void testFieldWithDotsAndCommonPrefix() { - FieldHitExtractor fe1 = getFieldHitExtractor("a.d", false); - FieldHitExtractor fe2 = getFieldHitExtractor("a.b.c", false); - Object value = randomNonNullValue(); - Map map = new HashMap<>(); - map.put("a", singletonMap("d", value)); - map.put("a.b", singletonMap("c", value)); - assertEquals(value, fe1.extractFromSource(map)); - assertEquals(value, fe2.extractFromSource(map)); - } - - public void testFieldWithDotsAndCommonPrefixes() { - FieldHitExtractor fe1 = getFieldHitExtractor("a1.b.c.d1.e.f.g1", false); - FieldHitExtractor fe2 = getFieldHitExtractor("a2.b.c.d2.e.f.g2", false); - Object value = randomNonNullValue(); - Map map = new HashMap<>(); - map.put("a1", singletonMap("b.c", singletonMap("d1", singletonMap("e.f", singletonMap("g1", value))))); - map.put("a2", singletonMap("b.c", singletonMap("d2", singletonMap("e.f", singletonMap("g2", value))))); - assertEquals(value, fe1.extractFromSource(map)); - assertEquals(value, fe2.extractFromSource(map)); - } - - public void testFieldWithDotsAndSamePathButDifferentHierarchy() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c.d.e.f.g", false); - Object value = randomNonNullValue(); - Map map = new HashMap<>(); - map.put("a.b", singletonMap("c", singletonMap("d.e", singletonMap("f.g", value)))); - map.put("a", singletonMap("b.c", singletonMap("d.e", singletonMap("f", singletonMap("g", value))))); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map)); - assertThat(ex.getMessage(), is("Multiple values (returned by [a.b.c.d.e.f.g]) are not supported")); - } - - public void testFieldsWithSingleValueArrayAsSubfield() { - FieldHitExtractor fe = getFieldHitExtractor("a.b", false); - Object value = randomNonNullValue(); - Map map = new HashMap<>(); - // "a" : [{"b" : "value"}] - map.put("a", singletonList(singletonMap("b", value))); - assertEquals(value, fe.extractFromSource(map)); - } - - public void testFieldsWithMultiValueArrayAsSubfield() { - FieldHitExtractor fe = getFieldHitExtractor("a.b", false); - Map map = new HashMap<>(); - // "a" : [{"b" : "value1"}, {"b" : "value2"}] - map.put("a", asList(singletonMap("b", randomNonNullValue()), singletonMap("b", randomNonNullValue()))); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map)); - assertThat(ex.getMessage(), is("Arrays (returned by [a.b]) are not supported")); - } - - public void testFieldsWithSingleValueArrayAsSubfield_TwoNestedLists() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c", false); - Object value = randomNonNullValue(); - Map map = new HashMap<>(); - // "a" : [{"b" : [{"c" : "value"}]}] - map.put("a", singletonList(singletonMap("b", singletonList(singletonMap("c", value))))); - assertEquals(value, fe.extractFromSource(map)); - } - - public void testFieldsWithMultiValueArrayAsSubfield_ThreeNestedLists() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c", false); - Map map = new HashMap<>(); - // "a" : [{"b" : [{"c" : ["value1", "value2"]}]}] - map.put("a", singletonList(singletonMap("b", singletonList(singletonMap("c", asList("value1", "value2")))))); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map)); - assertThat(ex.getMessage(), is("Arrays (returned by [a.b.c]) are not supported")); - } - - public void testFieldsWithSingleValueArrayAsSubfield_TwoNestedLists2() { - FieldHitExtractor fe = getFieldHitExtractor("a.b.c", false); - Object value = randomNonNullValue(); - Map map = new HashMap<>(); - // "a" : [{"b" : {"c" : ["value"]}]}] - map.put("a", singletonList(singletonMap("b", singletonMap("c", singletonList(value))))); - assertEquals(value, fe.extractFromSource(map)); - } - - @AwaitsFix(bugUrl = "temporary") - public void testObjectsForSourceValue() throws IOException { - String fieldName = randomAlphaOfLength(5); - FieldHitExtractor fe = getFieldHitExtractor(fieldName, false); - SearchHit hit = new SearchHit(1); - XContentBuilder source = JsonXContent.contentBuilder(); - source.startObject(); { - source.startObject(fieldName); { - source.field("b", "c"); - } - source.endObject(); - } - source.endObject(); - BytesReference sourceRef = BytesReference.bytes(source); - hit.sourceRef(sourceRef); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extract(hit)); - assertThat(ex.getMessage(), is("Objects (returned by [" + fieldName + "]) are not supported")); + DocumentField field = new DocumentField("a", asList(valueA, valueB)); + SearchHit hit = new SearchHit(1, null, null, singletonMap("a", field), null); + assertEquals(valueA, fe.extract(hit)); } public void testGeoShapeExtraction() { String fieldName = randomAlphaOfLength(5); FieldHitExtractor fe = new FieldHitExtractor(fieldName, randomBoolean() ? GEO_SHAPE : SHAPE, UTC, false); - Map map = new HashMap<>(); - map.put(fieldName, "POINT (1 2)"); - assertEquals(new GeoShape(1, 2), fe.extractFromSource(map)); - - map = new HashMap<>(); - assertNull(fe.extractFromSource(map)); - } + Map map = new HashMap<>(2); + map.put("coordinates", asList(1d, 2d)); + map.put("type", "Point"); + DocumentField field = new DocumentField(fieldName, singletonList(map)); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); + assertEquals(new GeoShape(1, 2), fe.extract(hit)); + } + public void testMultipleGeoShapeExtraction() { String fieldName = randomAlphaOfLength(5); FieldHitExtractor fe = new FieldHitExtractor(fieldName, randomBoolean() ? GEO_SHAPE : SHAPE, UTC, false); - Map map = new HashMap<>(); - map.put(fieldName, "POINT (1 2)"); - assertEquals(new GeoShape(1, 2), fe.extractFromSource(map)); - - map = new HashMap<>(); - assertNull(fe.extractFromSource(map)); - - Map map2 = new HashMap<>(); - map2.put(fieldName, Arrays.asList("POINT (1 2)", "POINT (3 4)")); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extractFromSource(map2)); - assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported")); - - FieldHitExtractor lenientFe = new FieldHitExtractor(fieldName, randomBoolean() ? GEO_SHAPE : SHAPE, UTC, false, true); - assertEquals(new GeoShape(1, 2), lenientFe.extractFromSource(map2)); - } - - @AwaitsFix(bugUrl = "temporary") - public void testGeoPointExtractionFromSource() throws IOException { - int layers = randomIntBetween(1, 3); - String pathCombined = ""; - double lat = randomDoubleBetween(-90, 90, true); - double lon = randomDoubleBetween(-180, 180, true); - SearchHit hit = new SearchHit(1); - XContentBuilder source = JsonXContent.contentBuilder(); - boolean[] arrayWrap = new boolean[layers - 1]; - source.startObject(); { - for (int i = 0; i < layers - 1; i++) { - arrayWrap[i] = randomBoolean(); - String name = randomAlphaOfLength(10); - source.field(name); - if (arrayWrap[i]) { - source.startArray(); - } - source.startObject(); - pathCombined = pathCombined + name + "."; - } - String name = randomAlphaOfLength(10); - pathCombined = pathCombined + name; - source.field(name, randomPoint(lat, lon)); - for (int i = layers - 2; i >= 0; i--) { - source.endObject(); - if (arrayWrap[i]) { - source.endArray(); - } - } - } - source.endObject(); - BytesReference sourceRef = BytesReference.bytes(source); - hit.sourceRef(sourceRef); - - FieldHitExtractor fe = new FieldHitExtractor(pathCombined, GEO_POINT, UTC, false); - assertEquals(new GeoShape(lon, lat), fe.extract(hit)); - } - - @AwaitsFix(bugUrl = "temporary") - public void testMultipleGeoPointExtractionFromSource() throws IOException { - double lat = randomDoubleBetween(-90, 90, true); - double lon = randomDoubleBetween(-180, 180, true); - SearchHit hit = new SearchHit(1); - String fieldName = randomAlphaOfLength(5); - int arraySize = randomIntBetween(2, 4); - XContentBuilder source = JsonXContent.contentBuilder(); - source.startObject(); { - source.startArray(fieldName); - source.value(randomPoint(lat, lon)); - for (int i = 1; i < arraySize; i++) { - source.value(randomPoint(lat, lon)); - } - source.endArray(); - } - source.endObject(); - BytesReference sourceRef = BytesReference.bytes(source); - hit.sourceRef(sourceRef); - - FieldHitExtractor fe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, false); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extract(hit)); - assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported")); - - FieldHitExtractor lenientFe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, false, true); - assertEquals(new GeoShape(lon, lat), lenientFe.extract(hit)); - } - - public void testGeoPointExtractionFromDocValues() { - String fieldName = randomAlphaOfLength(5); - FieldHitExtractor fe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, true); - - HashMap point = new HashMap<>(); - point.put("coordinates", Arrays.asList(1, 2)); - point.put("type", "Point"); - DocumentField field = new DocumentField(fieldName, singletonList(point)); - + + Map map1 = new HashMap<>(2); + map1.put("coordinates", asList(1d, 2d)); + map1.put("type", "Point"); + Map map2 = new HashMap<>(2); + map2.put("coordinates", asList(3d, 4d)); + map2.put("type", "Point"); + DocumentField field = new DocumentField(fieldName, asList(map1, map2)); SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); - assertEquals(new GeoShape(1, 2), fe.extract(hit)); - hit = new SearchHit(1); - assertNull(fe.extract(hit)); - } - - public void testGeoPointExtractionFromMultipleDocValues() { - String fieldName = randomAlphaOfLength(5); - FieldHitExtractor fe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, true); - HashMap point1 = new HashMap<>(); - point1.put("coordinates", Arrays.asList(1, 2)); - point1.put("type", "Point"); - HashMap point2 = new HashMap<>(); - point2.put("coordinates", Arrays.asList(3, 4)); - point2.put("type", "Point"); - - SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, - new DocumentField(fieldName, Arrays.asList(point1, point2))), null); QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extract(hit)); assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported")); - - FieldHitExtractor lenientFe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, true, true); - GeoShape expectedShape = new GeoShape(1, 2); - Object actualShape = lenientFe.extract(hit); - assertEquals(expectedShape, actualShape); + + FieldHitExtractor lenientFe = new FieldHitExtractor(fieldName, randomBoolean() ? GEO_SHAPE : SHAPE, UTC, true); + assertEquals(new GeoShape(3, 4), lenientFe.extract(new SearchHit(1, null, null, singletonMap(fieldName, + new DocumentField(fieldName, singletonList(map2))), null))); } - private FieldHitExtractor getFieldHitExtractor(String fieldName, boolean useDocValue) { - return new FieldHitExtractor(fieldName, null, UTC, useDocValue); + private FieldHitExtractor getFieldHitExtractor(String fieldName) { + return new FieldHitExtractor(fieldName, null, UTC); } private Object randomValue() { - Supplier value = randomFrom(Arrays.asList( + Supplier value = randomFrom( + Arrays.asList( () -> randomAlphaOfLength(10), ESTestCase::randomLong, ESTestCase::randomDouble, ESTestCase::randomInt, () -> BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE), () -> new BigDecimal("20012312345621343256123456254.20012312345621343256123456254"), - () -> null)); - return value.get(); - } - - private Object randomNonNullValue() { - Supplier value = randomFrom(Arrays.asList( - () -> randomAlphaOfLength(10), - ESTestCase::randomLong, - ESTestCase::randomDouble, - ESTestCase::randomInt, - () -> BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE), - () -> new BigDecimal("20012312345621343256123456254.20012312345621343256123456254"))); - return value.get(); - } - - private void assertFieldHitEquals(Object expected, Object actual) { - if (expected instanceof BigDecimal) { - // parsing will, by default, build a Double even if the initial value is BigDecimal - // Elasticsearch does this the same when returning the results - assertEquals(((BigDecimal) expected).doubleValue(), actual); - } else { - assertEquals(expected, actual); - } - } - - private Object randomPoint(double lat, double lon) { - Supplier value = randomFrom(Arrays.asList( - () -> lat + "," + lon, - () -> Arrays.asList(lon, lat), - () -> { - Map map1 = new HashMap<>(); - map1.put("lat", lat); - map1.put("lon", lon); - return map1; - } - )); + () -> null + ) + ); return value.get(); } } From a1a611949fea3b1528141f22076827a1225d0d97 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 26 Jan 2021 16:01:14 +0200 Subject: [PATCH 5/7] Remove fullFieldName from *QL and useDocValues from EQL --- .../sql/endpoints/translate.asciidoc | 20 +++++++------- .../eql/execution/search/RuntimeUtils.java | 2 +- .../search/extractor/FieldHitExtractor.java | 4 +-- .../extractor/TimestampFieldHitExtractor.java | 2 +- .../container/FieldExtractorRegistry.java | 12 +-------- .../querydsl/container/SearchHitFieldRef.java | 26 +++---------------- .../CriterionOrdinalExtractionTests.java | 6 ++--- .../extractor/AbstractFieldHitExtractor.java | 15 +++-------- .../xpack/sql/execution/search/Querier.java | 3 +-- .../search/extractor/FieldHitExtractor.java | 4 +-- .../extractor/FieldHitExtractorTests.java | 5 ++-- .../rest-api-spec/test/sql/translate.yml | 7 ++--- 12 files changed, 32 insertions(+), 74 deletions(-) diff --git a/docs/reference/sql/endpoints/translate.asciidoc b/docs/reference/sql/endpoints/translate.asciidoc index fdccbf00956b4..086efe1e1de9c 100644 --- a/docs/reference/sql/endpoints/translate.asciidoc +++ b/docs/reference/sql/endpoints/translate.asciidoc @@ -22,20 +22,22 @@ Which returns: -------------------------------------------------- { "size": 10, - "docvalue_fields": [ + "_source": false, + "fields": [ + { + "field": "author" + }, + { + "field": "name" + }, + { + "field": "page_count" + }, { "field": "release_date", "format": "epoch_millis" } ], - "_source": { - "includes": [ - "author", - "name", - "page_count" - ], - "excludes": [] - }, "sort": [ { "page_count": { diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java index 84c02713d9d64..1ccc06d1b7672 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java @@ -116,7 +116,7 @@ public static List createExtractor(List fields, E public static HitExtractor createExtractor(FieldExtraction ref, EqlConfiguration cfg) { if (ref instanceof SearchHitFieldRef) { SearchHitFieldRef f = (SearchHitFieldRef) ref; - return new FieldHitExtractor(f.name(), f.fullFieldName(), f.getDataType(), cfg.zoneId(), f.hitName(), false); + return new FieldHitExtractor(f.name(), f.getDataType(), cfg.zoneId(), f.hitName(), false); } if (ref instanceof ComputedRef) { diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java index 7a23caca69898..b832312d52fec 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java @@ -27,9 +27,9 @@ public FieldHitExtractor(StreamInput in) throws IOException { super(in); } - public FieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, String hitName, + public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId, String hitName, boolean arrayLeniency) { - super(name, fullFieldName, dataType, zoneId, hitName, arrayLeniency); + super(name, dataType, zoneId, hitName, arrayLeniency); } @Override diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/TimestampFieldHitExtractor.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/TimestampFieldHitExtractor.java index 4a55d4b5a5475..9369c5ed7dcb9 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/TimestampFieldHitExtractor.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/TimestampFieldHitExtractor.java @@ -9,7 +9,7 @@ public class TimestampFieldHitExtractor extends FieldHitExtractor { public TimestampFieldHitExtractor(FieldHitExtractor target) { - super(target.fieldName(), target.fullFieldName(), target.dataType(), target.zoneId(), target.hitName(), + super(target.fieldName(), target.dataType(), target.zoneId(), target.hitName(), target.arrayLeniency()); } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/FieldExtractorRegistry.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/FieldExtractorRegistry.java index 05a20b5f20e5e..a2e4cf893f49b 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/FieldExtractorRegistry.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/FieldExtractorRegistry.java @@ -44,16 +44,6 @@ private FieldExtraction createFieldExtractionFor(Expression expression) { } private FieldExtraction topHitFieldExtractor(FieldAttribute fieldAttr) { - FieldAttribute actualField = fieldAttr; - FieldAttribute rootField = fieldAttr; - StringBuilder fullFieldName = new StringBuilder(fieldAttr.field().getName()); - - while (rootField.parent() != null) { - fullFieldName.insert(0, ".").insert(0, rootField.parent().field().getName()); - rootField = rootField.parent(); - } - - return new SearchHitFieldRef(actualField.name(), fullFieldName.toString(), fieldAttr.field().getDataType(), - fieldAttr.field().isAggregatable(), fieldAttr.field().isAlias()); + return new SearchHitFieldRef(fieldAttr.name(), fieldAttr.field().getDataType(), fieldAttr.field().isAlias()); } } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/SearchHitFieldRef.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/SearchHitFieldRef.java index 67a3f012e03fb..dd296193d9d77 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/SearchHitFieldRef.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/querydsl/container/SearchHitFieldRef.java @@ -11,7 +11,6 @@ import org.elasticsearch.xpack.ql.type.DataType; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; -import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD; // NB: this class is taken from SQL - it hasn't been ported over to QL // since at this stage is unclear whether the whole FieldExtraction infrastructure @@ -19,23 +18,16 @@ public class SearchHitFieldRef implements FieldExtraction { private final String name; - private final String fullFieldName; // path included. If field full path is a.b.c, full field name is "a.b.c" and name is "c" private final DataType dataType; - private final boolean docValue; private final String hitName; - public SearchHitFieldRef(String name, String fullFieldName, DataType dataType, boolean useDocValueInsteadOfSource, boolean isAlias) { - this(name, fullFieldName, dataType, useDocValueInsteadOfSource, isAlias, null); + public SearchHitFieldRef(String name, DataType dataType, boolean isAlias) { + this(name, dataType, isAlias, null); } - public SearchHitFieldRef(String name, String fullFieldName, DataType dataType, boolean useDocValueInsteadOfSource, boolean isAlias, - String hitName) { + public SearchHitFieldRef(String name, DataType dataType, boolean isAlias, String hitName) { this.name = name; - this.fullFieldName = fullFieldName; this.dataType = dataType; - // these field types can only be extracted from docvalue_fields (ie, values already computed by Elasticsearch) - // because, for us to be able to extract them from _source, we would need the mapping of those fields (which we don't have) - this.docValue = isAlias ? useDocValueInsteadOfSource : (hasDocValues(dataType) ? useDocValueInsteadOfSource : false); this.hitName = hitName; } @@ -47,18 +39,10 @@ public String name() { return name; } - public String fullFieldName() { - return fullFieldName; - } - public DataType getDataType() { return dataType; } - public boolean useDocValue() { - return docValue; - } - @Override public void collectFields(QlSourceBuilder sourceBuilder) { // nested fields are handled by inner hits @@ -78,10 +62,6 @@ public String toString() { return name; } - private static boolean hasDocValues(DataType dataType) { - return dataType == KEYWORD || dataType == DATETIME; - } - private static String format(DataType dataType) { return dataType == DATETIME ? "epoch_millis" : null; } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java index 7212ff560077a..a51ca54978d0a 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java @@ -30,8 +30,8 @@ public class CriterionOrdinalExtractionTests extends ESTestCase { private String tsField = "timestamp"; private String tbField = "tiebreaker"; - private HitExtractor tsExtractor = new FieldHitExtractor(tsField, tsField, DataTypes.LONG, null, null, false); - private HitExtractor tbExtractor = new FieldHitExtractor(tbField, tbField, DataTypes.LONG, null, null, false); + private HitExtractor tsExtractor = new FieldHitExtractor(tsField, DataTypes.LONG, null, null, false); + private HitExtractor tbExtractor = new FieldHitExtractor(tbField, DataTypes.LONG, null, null, false); public void testTimeOnly() throws Exception { long time = randomLong(); @@ -56,7 +56,7 @@ public void testTimeAndTiebreakerNull() throws Exception { } public void testTimeNotComparable() throws Exception { - HitExtractor badExtractor = new FieldHitExtractor(tsField, tsField, DataTypes.BINARY, null, null, false); + HitExtractor badExtractor = new FieldHitExtractor(tsField, DataTypes.BINARY, null, null, false); SearchHit hit = searchHit(randomAlphaOfLength(10), null); Criterion criterion = new Criterion(0, null, emptyList(), badExtractor, null, false); EqlIllegalArgumentException exception = expectThrows(EqlIllegalArgumentException.class, () -> criterion.ordinal(hit)); diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java index ec50e1599d262..2443b45b060eb 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java @@ -24,23 +24,20 @@ public abstract class AbstractFieldHitExtractor implements HitExtractor { private final String fieldName, hitName; - private final String fullFieldName; // used to look at the _ignored section of the query response for the actual full field name private final DataType dataType; private final ZoneId zoneId; private final boolean arrayLeniency; protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId) { - this(name, null, dataType, zoneId, null, false); + this(name, dataType, zoneId, null, false); } protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean arrayLeniency) { - this(name, null, dataType, zoneId, null, arrayLeniency); + this(name, dataType, zoneId, null, arrayLeniency); } - protected AbstractFieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, - String hitName, boolean arrayLeniency) { + protected AbstractFieldHitExtractor(String name, DataType dataType, ZoneId zoneId, String hitName, boolean arrayLeniency) { this.fieldName = name; - this.fullFieldName = fullFieldName; this.dataType = dataType; this.zoneId = zoneId; this.arrayLeniency = arrayLeniency; @@ -55,7 +52,6 @@ protected AbstractFieldHitExtractor(String name, String fullFieldName, DataType protected AbstractFieldHitExtractor(StreamInput in) throws IOException { fieldName = in.readString(); - fullFieldName = in.readOptionalString(); String typeName = in.readOptionalString(); dataType = typeName != null ? loadTypeFromName(typeName) : null; hitName = in.readOptionalString(); @@ -72,7 +68,6 @@ protected DataType loadTypeFromName(String typeName) { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(fieldName); - out.writeOptionalString(fullFieldName); out.writeOptionalString(dataType == null ? null : dataType.typeName()); out.writeOptionalString(hitName); out.writeBoolean(arrayLeniency); @@ -128,10 +123,6 @@ public String fieldName() { return fieldName; } - public String fullFieldName() { - return fullFieldName; - } - public ZoneId zoneId() { return zoneId; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java index 131a01ed14d8d..dfa348190b650 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java @@ -492,8 +492,7 @@ protected void handleResponse(SearchResponse response, ActionListener list private HitExtractor createExtractor(FieldExtraction ref) { if (ref instanceof SearchHitFieldRef) { SearchHitFieldRef f = (SearchHitFieldRef) ref; - return new FieldHitExtractor(f.name(), f.fullFieldName(), f.getDataType(), cfg.zoneId(), f.hitName(), - multiValueFieldLeniency); + return new FieldHitExtractor(f.name(), f.getDataType(), cfg.zoneId(), f.hitName(), multiValueFieldLeniency); } if (ref instanceof ScriptFieldRef) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index 4d4923b44a44b..22941c5a7d1c8 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -48,9 +48,9 @@ public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId) { super(name, dataType, zoneId); } - public FieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, String hitName, + public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId, String hitName, boolean arrayLeniency) { - super(name, fullFieldName, dataType, zoneId, hitName, arrayLeniency); + super(name, dataType, zoneId, hitName, arrayLeniency); } public FieldHitExtractor(StreamInput in) throws IOException { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java index e40b0fce57d53..c8c4a2cff4252 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java @@ -41,7 +41,7 @@ public class FieldHitExtractorTests extends AbstractSqlWireSerializingTestCase randomFrom(SqlDataTypes.types())), randomValueOtherThan(instance.zoneId(), ESTestCase::randomZone), instance.hitName() + "mutated", @@ -124,7 +123,7 @@ public void testGetDate() { public void testToString() { assertEquals( "hit.field@hit@Europe/Berlin", - new FieldHitExtractor("hit.field", null, null, ZoneId.of("Europe/Berlin"), "hit", false).toString() + new FieldHitExtractor("hit.field", null, ZoneId.of("Europe/Berlin"), "hit", false).toString() ); } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/sql/translate.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/sql/translate.yml index ee5af390678ed..eceaa65b8e999 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/sql/translate.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/sql/translate.yml @@ -22,11 +22,8 @@ - match: $body: size: 1000 - _source: - includes: - - int - - str - excludes: [] + _source: false + fields: [ {"field" : "str" }, {"field" : "int" } ] sort: - int: order: asc From 18bf44f349335abd9ff3a453a48e77665795f890 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Mon, 1 Feb 2021 18:47:56 +0200 Subject: [PATCH 6/7] More eql updates --- x-pack/plugin/eql/qa/mixed-node/build.gradle | 65 +++ .../xpack/eql/qa/mixed_node/EqlSearchIT.java | 385 ++++++++++++++++++ .../src/test/resources/eql_mapping.json | 35 ++ .../eql/execution/search/RuntimeUtils.java | 14 +- .../eql/plugin/TransportEqlSearchAction.java | 70 +++- .../ql/execution/search/QlSourceBuilder.java | 2 + .../xpack/sql/qa/mixed_node/SqlSearchIT.java | 6 +- .../xpack/sql/execution/search/Querier.java | 3 +- .../sql/plugin/TransportSqlQueryAction.java | 4 +- 9 files changed, 564 insertions(+), 20 deletions(-) create mode 100644 x-pack/plugin/eql/qa/mixed-node/build.gradle create mode 100644 x-pack/plugin/eql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/eql/qa/mixed_node/EqlSearchIT.java create mode 100644 x-pack/plugin/eql/qa/mixed-node/src/test/resources/eql_mapping.json diff --git a/x-pack/plugin/eql/qa/mixed-node/build.gradle b/x-pack/plugin/eql/qa/mixed-node/build.gradle new file mode 100644 index 0000000000000..a49a1e967aeb3 --- /dev/null +++ b/x-pack/plugin/eql/qa/mixed-node/build.gradle @@ -0,0 +1,65 @@ +apply plugin: 'elasticsearch.testclusters' +apply plugin: 'elasticsearch.standalone-rest-test' +apply from : "$rootDir/gradle/bwc-test.gradle" +apply plugin: 'elasticsearch.rest-test' + +import org.elasticsearch.gradle.Version +import org.elasticsearch.gradle.VersionProperties +import org.elasticsearch.gradle.info.BuildParams +import org.elasticsearch.gradle.testclusters.StandaloneRestIntegTestTask + +dependencies { + testImplementation project(':x-pack:qa') + testImplementation project(path: xpackModule('ql'), configuration: 'default') + testImplementation project(path: xpackModule('eql'), configuration: 'default') +} + +tasks.named("integTest").configure{ enabled = false} + +//for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it.onOrAfter('7.9.0') }) { +for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it == Version.fromString("7.9.0") }) { + if (bwcVersion == VersionProperties.getElasticsearchVersion()) { + // Not really a mixed cluster + continue; + } + + String baseName = "v${bwcVersion}" + + testClusters { + "${baseName}" { + versions = [bwcVersion.toString(), project.version] + numberOfNodes = 3 + testDistribution = 'DEFAULT' + setting 'xpack.security.enabled', 'false' + setting 'xpack.watcher.enabled', 'false' + setting 'xpack.ml.enabled', 'false' + setting 'xpack.eql.enabled', 'true' + setting 'xpack.license.self_generated.type', 'trial' + setting 'logger.org.elasticsearch.xpack.eql.plugin.TransportEqlSearchAction', 'TRACE' + } + } + + tasks.register("${baseName}#mixedClusterTest", StandaloneRestIntegTestTask) { + useCluster testClusters."${baseName}" + mustRunAfter("precommit") + doFirst { + // Getting the endpoints causes a wait for the cluster + println "Endpoints are: ${-> testClusters."${baseName}".allHttpSocketURI.join(",")}" + println "Upgrading one node to create a mixed cluster" + testClusters."${baseName}".nextNodeToNextVersion() + + println "Upgrade complete, endpoints are: ${-> testClusters."${baseName}".allHttpSocketURI.join(",")}" + nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}".allHttpSocketURI.join(",")}") + nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}") + } + onlyIf { project.bwc_tests_enabled } + } + + tasks.register(bwcTaskName(bwcVersion)) { + dependsOn "${baseName}#mixedClusterTest" + } + + tasks.named("check").configure { + dependsOn "${baseName}#mixedClusterTest" + } +} diff --git a/x-pack/plugin/eql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/eql/qa/mixed_node/EqlSearchIT.java b/x-pack/plugin/eql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/eql/qa/mixed_node/EqlSearchIT.java new file mode 100644 index 0000000000000..b1a6649c1a95b --- /dev/null +++ b/x-pack/plugin/eql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/eql/qa/mixed_node/EqlSearchIT.java @@ -0,0 +1,385 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.eql.qa.mixed_node; + +import org.apache.http.HttpHost; +import org.elasticsearch.Version; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.test.NotEqualMessageBuilder; +import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.test.rest.yaml.ObjectPath; +import org.junit.After; +import org.junit.Before; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static java.util.Collections.unmodifiableList; + +/** + * Class testing the behavior of events and sequence queries in a mixed cluster scenario (during rolling upgrade). + * The test is against a three-node cluster where one node is upgraded, the other two are on the old version. + */ +public class EqlSearchIT extends ESRestTestCase { + + private static final Version FIELDS_API_QL_INTRODUCTION = Version.V_7_12_0; + private static final String index = "test_eql_mixed_versions"; + private static int numShards; + private static int numReplicas = 1; + private static int numDocs; + private static Nodes nodes; + private static List allNodes; + private static List newNodes; + private static List bwcNodes; + private static Version bwcVersion; + private static Version newVersion; + private static boolean isBeforeFieldsApiInQL; + + @Before + public void createIndex() throws IOException { + nodes = buildNodeAndVersions(client()); + numShards = nodes.size(); + numDocs = randomIntBetween(numShards, 16); + allNodes = new ArrayList<>(); + allNodes.addAll(nodes.getBWCNodes()); + allNodes.addAll(nodes.getNewNodes()); + newNodes = new ArrayList<>(); + newNodes.addAll(nodes.getNewNodes()); + bwcNodes = new ArrayList<>(); + bwcNodes.addAll(nodes.getBWCNodes()); + bwcVersion = nodes.getBWCNodes().get(0).getVersion(); + newVersion = nodes.getNewNodes().get(0).getVersion(); + isBeforeFieldsApiInQL = bwcVersion.before(FIELDS_API_QL_INTRODUCTION); + + String mappings = readResource("/eql_mapping.json"); + createIndex( + index, + Settings.builder() + .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build(), + mappings + ); + } + + @After + public void cleanUpIndex() throws IOException { + if (indexExists(index) == true) { + deleteIndex(index); + } + } + + public void testEventsWithRequestToOldNodes() throws Exception { + assertEventsQueryOnNodes(bwcNodes); + } + + public void testEventsWithRequestToUpgradedNodes() throws Exception { + assertEventsQueryOnNodes(newNodes); + } + + public void testSequencesWithRequestToOldNodes() throws Exception { + assertSequncesQueryOnNodes(bwcNodes); + } + + public void testSequencesWithRequestToUpgradedNodes() throws Exception { + assertSequncesQueryOnNodes(newNodes); + } + + private void assertEventsQueryOnNodes(List nodesList) throws Exception { + final String event = randomEvent(); + Map expectedResponse = prepareTestData(event); + try ( + RestClient client = buildClient(restClientSettings(), nodesList.stream().map(Node::getPublishAddress).toArray(HttpHost[]::new)) + ) { + String filterPath = "filter_path=hits.events._source.@timestamp,hits.events._source.event_type,hits.events._source.sequence"; + + Request request = new Request("POST", index + "/_eql/search?" + filterPath); + request.setJsonEntity("{\"query\":\"" + event + " where true\"}"); + assertBusy(() -> { assertResponse(expectedResponse, runEql(client, request)); }); + } + } + + private void assertSequncesQueryOnNodes(List nodesList) throws Exception { + Map expectedResponse = prepareSequencesTestData(); + try ( + RestClient client = buildClient(restClientSettings(), nodesList.stream().map(Node::getPublishAddress).toArray(HttpHost[]::new)) + ) { + String filterPath = "filter_path=hits.sequences.join_keys,hits.sequences.events._id,hits.sequences.events._source"; + String query = "sequence by `sequence` with maxspan=100ms [success where true] by correlation_success1, correlation_success2 " + + "[failure where true] by correlation_failure1, correlation_failure2"; + String filter = "{\"range\":{\"@timestamp\":{\"gte\":\"1970-05-01\"}}}"; + + Request request = new Request("POST", index + "/_eql/search?" + filterPath); + request.setJsonEntity("{\"query\":\"" + query + "\",\"filter\":" + filter + "}"); + assertBusy(() -> { assertResponse(expectedResponse, runEql(client, request)); }); + } + } + + private String randomEvent() { + return randomFrom("success", "failure"); + } + + @SuppressWarnings("unchecked") + private Map prepareTestData(String event) throws IOException { + List> sourceEvents = new ArrayList>(); + Map expectedResponse = singletonMap("hits", singletonMap("events", sourceEvents)); + + for (int i = 0; i < numDocs; i++) { + StringBuilder builder = new StringBuilder(); + final String randomEvent = randomEvent(); + builder.append("{"); + builder.append("\"@timestamp\":" + i + ","); + builder.append("\"event_type\":\"" + randomEvent + "\","); + builder.append("\"sequence\":" + i); + builder.append("}"); + if (randomEvent.equals(event)) { + Map eventSource = new HashMap<>(); + eventSource.put("@timestamp", i); + eventSource.put("event_type", randomEvent); + eventSource.put("sequence", i); + sourceEvents.add(singletonMap("_source", eventSource)); + } + + Request request = new Request("PUT", index + "/_doc/" + i); + request.setJsonEntity(builder.toString()); + assertOK(client().performRequest(request)); + } + if (sourceEvents.isEmpty()) { + return Collections.EMPTY_MAP; + } + return expectedResponse; + } + + /* + * Output to compare with looks like this: + * { + * "hits": { + * "sequences": [ + * { + * "join_keys": [ + * 44, + * "C", + * "D" + * ], + * "events": [ + * { + * "_id": "14", + * "_source": { + * ... + * } + * } + * ] + * } + * } + * } + * + */ + private Map prepareSequencesTestData() throws IOException { + Map event14 = new HashMap<>(); + Map event14Source = new HashMap<>(); + event14.put("_id", "14"); + event14.put("_source", event14Source); + event14Source.put("@timestamp", "12345678914"); + event14Source.put("event_type", "success"); + event14Source.put("sequence", 44); + event14Source.put("correlation_success1", "C"); + event14Source.put("correlation_success2", "D"); + + Map event15 = new HashMap<>(); + Map event15Source = new HashMap<>(); + event15.put("_id", "15"); + event15.put("_source", event15Source); + event15Source.put("@timestamp", "12345678999"); + event15Source.put("event_type", "failure"); + event15Source.put("sequence", 44); + event15Source.put("correlation_failure1", "C"); + event15Source.put("correlation_failure2", "D"); + + Map sequence = new HashMap<>(); + List> events = unmodifiableList(asList(event14, event15)); + List> sequences = singletonList(sequence); + Map expectedResponse = singletonMap("hits", singletonMap("sequences", sequences)); + + sequence.put("join_keys", asList(44, "C", "D")); + sequence.put("events", events); + + final List bulkEntries = getSequencesBulkEntries(); + StringBuilder builder = new StringBuilder(); + for (int i = 1; i < 16; i++) { + builder.append("{\"index\": {\"_id\":" + i + "}}\n"); + builder.append(bulkEntries.get(i - 1)); + } + + Request request = new Request("POST", index + "/_bulk?refresh"); + request.setJsonEntity(builder.toString()); + assertOK(client().performRequest(request)); + + return expectedResponse; + } + + private void assertResponse(Map expected, Map actual) { + if (false == expected.equals(actual)) { + NotEqualMessageBuilder message = new NotEqualMessageBuilder(); + message.compareMaps(actual, expected); + fail("Response does not match:\n" + message.toString()); + } + } + + private Map runEql(RestClient client, Request request) throws IOException { + Response response = client.performRequest(request); + try (InputStream content = response.getEntity().getContent()) { + return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false); + } + } + + private static String readResource(String location) throws IOException { + StringBuilder builder = new StringBuilder(); + try (BufferedReader reader = new BufferedReader(new InputStreamReader(EqlSearchIT.class.getResourceAsStream(location), + StandardCharsets.UTF_8))) + { + String line = reader.readLine(); + while (line != null) { + if (line.trim().startsWith("//") == false) { + builder.append(line); + builder.append('\n'); + } + line = reader.readLine(); + } + return builder.toString(); + } + } + + private List getSequencesBulkEntries() { + List bulkEntries = new ArrayList<>(15); + bulkEntries.add("{\"@timestamp\":\"1234567891\",\"event_type\":\"success\",\"sequence\":1,\"correlation_success1\":\"A\"," + + "\"correlation_success2\":\"B\"}\n"); + bulkEntries.add("{\"@timestamp\":\"1234567892\",\"event_type\":\"failure\",\"sequence\":2,\"correlation_failure1\":\"A\"," + + "\"correlation_failure2\":\"B\"}\n"); + bulkEntries.add("{\"@timestamp\":\"1234567893\",\"event_type\":\"success\",\"sequence\":3,\"correlation_success1\":\"A\"," + + "\"correlation_success2\":\"A\"}\n"); + bulkEntries.add("{\"@timestamp\":\"1234567894\",\"event_type\":\"success\",\"sequence\":4,\"correlation_success1\":\"C\"," + + "\"correlation_success2\":\"C\"}\n"); + bulkEntries.add("{\"@timestamp\":\"1234567895\",\"event_type\":\"failure\",\"sequence\":5,\"correlation_failure1\":\"B\"," + + "\"correlation_failure2\":\"C\"}\n"); + bulkEntries.add("{\"@timestamp\":\"1234567896\",\"event_type\":\"success\",\"sequence\":1,\"correlation_success1\":\"A\"," + + "\"correlation_success2\":\"A\"}\n"); + bulkEntries.add("{\"@timestamp\":\"1234567897\",\"event_type\":\"failure\",\"sequence\":1,\"correlation_failure1\":\"A\"," + + "\"correlation_failure2\":\"A\"}\n"); + bulkEntries.add("{\"@timestamp\":\"1234567898\",\"event_type\":\"success\",\"sequence\":3,\"correlation_success1\":\"A\"," + + "\"correlation_success2\":\"A\"}\n"); + bulkEntries.add("{\"@timestamp\":\"1234567899\",\"event_type\":\"success\",\"sequence\":4,\"correlation_success1\":\"C\"," + + "\"correlation_success2\":\"B\"}\n"); + bulkEntries.add("{\"@timestamp\":\"12345678910\",\"event_type\":\"failure\",\"sequence\":4,\"correlation_failure1\":\"B\"," + + "\"correlation_failure2\":\"B\"}\n"); + bulkEntries.add("{\"@timestamp\":\"12345678911\",\"event_type\":\"success\",\"sequence\":1,\"correlation_success1\":\"A\"," + + "\"correlation_success2\":\"A\"}\n"); + bulkEntries.add("{\"@timestamp\":\"12345678912\",\"event_type\":\"failure\",\"sequence\":1,\"correlation_failure1\":\"A\"," + + "\"correlation_failure2\":\"B\"}\n"); + bulkEntries.add("{\"@timestamp\":\"12345678913\",\"event_type\":\"success\",\"sequence\":3,\"correlation_success1\":\"A\"," + + "\"correlation_success2\":\"A\"}\n"); + bulkEntries.add("{\"@timestamp\":\"12345678914\",\"event_type\":\"success\",\"sequence\":44,\"correlation_success1\":\"C\"," + + "\"correlation_success2\":\"D\"}\n"); + bulkEntries.add("{\"@timestamp\":\"12345678999\",\"event_type\":\"failure\",\"sequence\":44,\"correlation_failure1\":\"C\"," + + "\"correlation_failure2\":\"D\"}\n"); + return unmodifiableList(bulkEntries); + } + + static Nodes buildNodeAndVersions(RestClient client) throws IOException { + Response response = client.performRequest(new Request("GET", "_nodes")); + ObjectPath objectPath = ObjectPath.createFromResponse(response); + Map nodesAsMap = objectPath.evaluate("nodes"); + Nodes nodes = new Nodes(); + for (String id : nodesAsMap.keySet()) { + nodes.add( + new Node( + id, + Version.fromString(objectPath.evaluate("nodes." + id + ".version")), + HttpHost.create(objectPath.evaluate("nodes." + id + ".http.publish_address")) + ) + ); + } + return nodes; + } + + private static final class Nodes extends HashMap { + + public void add(Node node) { + put(node.getId(), node); + } + + public List getNewNodes() { + Version bwcVersion = getBWCVersion(); + return values().stream().filter(n -> n.getVersion().after(bwcVersion)).collect(Collectors.toList()); + } + + public List getBWCNodes() { + Version bwcVersion = getBWCVersion(); + return values().stream().filter(n -> n.getVersion().equals(bwcVersion)).collect(Collectors.toList()); + } + + public Version getBWCVersion() { + if (isEmpty()) { + throw new IllegalStateException("no nodes available"); + } + return Version.fromId(values().stream().map(node -> node.getVersion().id).min(Integer::compareTo).get()); + } + + @Override + public String toString() { + return "Nodes{" + + values().stream().map(Node::toString).collect(Collectors.joining("\n")) + + '}'; + } + } + + private static final class Node { + private final String id; + private final Version version; + private final HttpHost publishAddress; + + Node(String id, Version version, HttpHost publishAddress) { + this.id = id; + this.version = version; + this.publishAddress = publishAddress; + } + + public String getId() { + return id; + } + + public HttpHost getPublishAddress() { + return publishAddress; + } + + public Version getVersion() { + return version; + } + + @Override + public String toString() { + return "Node{" + "id='" + id + '\'' + ", version=" + version + '}'; + } + } +} diff --git a/x-pack/plugin/eql/qa/mixed-node/src/test/resources/eql_mapping.json b/x-pack/plugin/eql/qa/mixed-node/src/test/resources/eql_mapping.json new file mode 100644 index 0000000000000..16551df22c25f --- /dev/null +++ b/x-pack/plugin/eql/qa/mixed-node/src/test/resources/eql_mapping.json @@ -0,0 +1,35 @@ + "properties": { + "@timestamp": { + "type": "date" + }, + "event_type": { + "type": "keyword" + }, + "sequence": { + "type": "long" + }, + "correlation_success1": { + "type": "wildcard" + }, + "correlation_failure1": { + "type": "wildcard" + }, + "correlation_success2": { + "type": "keyword" + }, + "correlation_failure2": { + "type": "keyword" + }, + "event": { + "properties": { + "category": { + "type": "alias", + "path": "event_type" + }, + "sequence": { + "type": "alias", + "path": "sequence" + } + } + } + } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java index 1ccc06d1b7672..c8155af0d5ee2 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/RuntimeUtils.java @@ -41,6 +41,7 @@ import java.util.Set; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; +import static org.elasticsearch.xpack.ql.execution.search.QlSourceBuilder.FIELDS_API_INTRODUCTION_VERSION; public final class RuntimeUtils { @@ -148,12 +149,13 @@ public static SearchRequest prepareRequest(Client client, SearchSourceBuilder source, boolean includeFrozen, String... indices) { - return client.prepareSearch(indices) - .setSource(source) - .setAllowPartialSearchResults(false) - .setIndicesOptions( - includeFrozen ? IndexResolver.FIELD_CAPS_FROZEN_INDICES_OPTIONS : IndexResolver.FIELD_CAPS_INDICES_OPTIONS) - .request(); + SearchRequest searchRequest = new SearchRequest(FIELDS_API_INTRODUCTION_VERSION); + searchRequest.indices(indices); + searchRequest.source(source); + searchRequest.allowPartialSearchResults(false); + searchRequest.indicesOptions( + includeFrozen ? IndexResolver.FIELD_CAPS_FROZEN_INDICES_OPTIONS : IndexResolver.FIELD_CAPS_INDICES_OPTIONS); + return searchRequest; } public static List searchHits(SearchResponse response) { diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/TransportEqlSearchAction.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/TransportEqlSearchAction.java index 8a78e8e16e738..35d9761fbb6ed 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/TransportEqlSearchAction.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/TransportEqlSearchAction.java @@ -5,10 +5,16 @@ */ package org.elasticsearch.xpack.eql.plugin; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionListenerResponseHandler; +import org.elasticsearch.action.search.SearchPhaseExecutionException; +import org.elasticsearch.action.search.VersionMismatchException; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -35,10 +41,12 @@ import org.elasticsearch.xpack.eql.session.EqlConfiguration; import org.elasticsearch.xpack.eql.session.Results; import org.elasticsearch.xpack.ql.expression.Order; +import org.elasticsearch.xpack.ql.util.Holder; import java.io.IOException; import java.time.ZoneId; import java.util.Map; +import java.util.Objects; import static org.elasticsearch.action.ActionListener.wrap; import static org.elasticsearch.xpack.core.ClientHelper.ASYNC_SEARCH_ORIGIN; @@ -46,10 +54,12 @@ public class TransportEqlSearchAction extends HandledTransportAction implements AsyncTaskManagementService.AsyncOperation { + private static final Logger log = LogManager.getLogger(TransportEqlSearchAction.class); private final SecurityContext securityContext; private final ClusterService clusterService; private final PlanExecutor planExecutor; private final ThreadPool threadPool; + private final TransportService transportService; private final AsyncTaskManagementService asyncTaskManagementService; @Inject @@ -63,6 +73,7 @@ public TransportEqlSearchAction(Settings settings, ClusterService clusterService this.clusterService = clusterService; this.planExecutor = planExecutor; this.threadPool = threadPool; + this.transportService = transportService; this.asyncTaskManagementService = new AsyncTaskManagementService<>(XPackPlugin.ASYNC_RESULTS_INDEX, client, ASYNC_SEARCH_ORIGIN, registry, taskManager, EqlSearchAction.INSTANCE.name(), this, EqlSearchTask.class, clusterService, threadPool); @@ -77,8 +88,7 @@ public EqlSearchTask createTask(EqlSearchRequest request, long id, String type, @Override public void execute(EqlSearchRequest request, EqlSearchTask task, ActionListener listener) { - operation(planExecutor, task, request, username(securityContext), clusterName(clusterService), - clusterService.localNode().getId(), listener); + operation(planExecutor, task, request, username(securityContext), transportService, clusterService, listener); } @Override @@ -98,13 +108,15 @@ protected void doExecute(Task task, EqlSearchRequest request, ActionListener listener) { + TransportService transportService, ClusterService clusterService, + ActionListener listener) { + String nodeId = clusterService.localNode().getId(); + String clusterName = clusterName(clusterService); // TODO: these should be sent by the client ZoneId zoneId = DateUtils.of("Z"); QueryBuilder filter = request.filter(); @@ -121,8 +133,52 @@ public static void operation(PlanExecutor planExecutor, EqlSearchTask task, EqlS EqlConfiguration cfg = new EqlConfiguration(request.indices(), zoneId, username, clusterName, filter, timeout, request.indicesOptions(), request.fetchSize(), clientId, new TaskId(nodeId, task.getId()), task); - planExecutor.eql(cfg, request.query(), params, wrap(r -> listener.onResponse(createResponse(r, task.getExecutionId())), - listener::onFailure)); + Holder retrySecondTime = new Holder(false); + planExecutor.eql(cfg, request.query(), params, wrap(r -> listener.onResponse(createResponse(r, task.getExecutionId())), e -> { + // the search request will likely run on nodes with different versions of ES + // we will retry on a node with an older version that should generate a backwards compatible _search request + // this scenario is happening for queries generated during sequences + if (e instanceof SearchPhaseExecutionException + && ((SearchPhaseExecutionException) e).getCause() instanceof VersionMismatchException) { + + SearchPhaseExecutionException spee = (SearchPhaseExecutionException) e; + if (log.isTraceEnabled()) { + log.trace("Caught exception type [{}] with cause [{}].", e.getClass().getName(), e.getCause()); + } + DiscoveryNode localNode = clusterService.state().nodes().getLocalNode(); + DiscoveryNode candidateNode = null; + for (DiscoveryNode node : clusterService.state().nodes()) { + // find the first node that's older than the current node + if (Objects.equals(node, localNode) == false && node.getVersion().before(localNode.getVersion())) { + candidateNode = node; + break; + } + } + if (candidateNode != null) { + if (log.isTraceEnabled()) { + log.trace("Candidate node to resend the request to: address [{}], id [{}], name [{}], version [{}]", + candidateNode.getAddress(), candidateNode.getId(), candidateNode.getName(), candidateNode.getVersion()); + } + // re-send the request to the older node + transportService.sendRequest(candidateNode, EqlSearchAction.NAME, request, + new ActionListenerResponseHandler<>(listener, EqlSearchResponse::new, ThreadPool.Names.SAME)); + } else { + if (log.isTraceEnabled()) { + log.trace("No candidate node found, likely all were upgraded in the meantime"); + } + retrySecondTime.set(true); + } + } else { + listener.onFailure(e); + } + })); + if (retrySecondTime.get()) { + if (log.isTraceEnabled()) { + log.trace("No candidate node found, likely all were upgraded in the meantime. Re-trying the original request."); + } + planExecutor.eql(cfg, request.query(), params, wrap(r -> listener.onResponse(createResponse(r, task.getExecutionId())), + listener::onFailure)); + } } static EqlSearchResponse createResponse(Results results, AsyncExecutionId id) { diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java index dcb4f517e755d..f8d3fd67b1201 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.ql.execution.search; +import org.elasticsearch.Version; import org.elasticsearch.script.Script; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.fetch.subphase.FieldAndFormat; @@ -20,6 +21,7 @@ * the resulting ES document as a field. */ public class QlSourceBuilder { + public static final Version FIELDS_API_INTRODUCTION_VERSION = Version.V_7_10_0; // The LinkedHashMaps preserve the order of the fields in the response private final Set fetchFields = new LinkedHashSet<>(); private final Map scriptFields = new LinkedHashMap<>(); diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java index 3630ce53d23df..876a4d0d24ac2 100644 --- a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java @@ -269,7 +269,7 @@ private static String readResource(String location) throws IOException { } } - static Nodes buildNodeAndVersions(RestClient client) throws IOException { + public static Nodes buildNodeAndVersions(RestClient client) throws IOException { Response response = client.performRequest(new Request("GET", "_nodes")); ObjectPath objectPath = ObjectPath.createFromResponse(response); Map nodesAsMap = objectPath.evaluate("nodes"); @@ -286,7 +286,7 @@ static Nodes buildNodeAndVersions(RestClient client) throws IOException { return nodes; } - private static final class Nodes extends HashMap { + public static final class Nodes extends HashMap { public void add(Node node) { put(node.getId(), node); @@ -317,7 +317,7 @@ public String toString() { } } - private static final class Node { + public static final class Node { private final String id; private final Version version; private final HttpHost publishAddress; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java index dfa348190b650..964a7f89e413f 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java @@ -8,7 +8,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.PriorityQueue; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; @@ -84,10 +83,10 @@ import static java.util.Collections.singletonList; import static org.elasticsearch.action.ActionListener.wrap; +import static org.elasticsearch.xpack.ql.execution.search.QlSourceBuilder.FIELDS_API_INTRODUCTION_VERSION; // TODO: add retry/back-off public class Querier { - public static final Version FIELDS_API_INTRODUCTION_VERSION = Version.V_7_10_0; private static final Logger log = LogManager.getLogger(Querier.class); private final PlanExecutor planExecutor; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java index 59028ba235cdd..9bcfecf22e007 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java @@ -51,13 +51,13 @@ import static org.elasticsearch.xpack.sql.plugin.Transports.username; public class TransportSqlQueryAction extends HandledTransportAction { - + private static final Logger log = LogManager.getLogger(TransportSqlQueryAction.class); private final SecurityContext securityContext; private final ClusterService clusterService; private final PlanExecutor planExecutor; private final SqlLicenseChecker sqlLicenseChecker; private final TransportService transportService; - private static final Logger log = LogManager.getLogger(TransportSqlQueryAction.class); + @Inject public TransportSqlQueryAction(Settings settings, ClusterService clusterService, TransportService transportService, ThreadPool threadPool, ActionFilters actionFilters, PlanExecutor planExecutor, From 7a4c90d04a5c8d3a37211cc0c01d21c1547a73d0 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Wed, 3 Feb 2021 16:01:21 +0200 Subject: [PATCH 7/7] add intervals to the query --- x-pack/plugin/sql/qa/mixed-node/build.gradle | 2 +- .../xpack/sql/qa/mixed_node/SqlSearchIT.java | 23 +++++++++++-------- .../xpack/sql/action/SqlQueryResponse.java | 9 +------- .../sql/plugin/TransportSqlQueryAction.java | 16 ++++++++++++- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/x-pack/plugin/sql/qa/mixed-node/build.gradle b/x-pack/plugin/sql/qa/mixed-node/build.gradle index bbbce0556e15d..86ef03776a368 100644 --- a/x-pack/plugin/sql/qa/mixed-node/build.gradle +++ b/x-pack/plugin/sql/qa/mixed-node/build.gradle @@ -17,7 +17,7 @@ dependencies { tasks.named("integTest").configure{ enabled = false} //for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it.onOrAfter('7.9.0') }) { -for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it == Version.fromString("7.11.0") }) { +for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it == Version.fromString("7.10.3") }) { if (bwcVersion == VersionProperties.getElasticsearchVersion()) { // Not really a mixed cluster continue; diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java index 876a4d0d24ac2..cef15f09c564c 100644 --- a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java @@ -144,6 +144,8 @@ private Map prepareTestData(Consumer>> BiConsumer> additionalValues) throws IOException { Map expectedResponse = new HashMap<>(); List> columns = new ArrayList<>(); + columns.add(columnInfo("interval_year", "interval_year")); + columns.add(columnInfo("interval_minute", "interval_minute")); columns.add(columnInfo("long_field", "long")); columns.add(columnInfo("integer_field", "integer")); columns.add(columnInfo("short_field", "short")); @@ -169,6 +171,9 @@ private Map prepareTestData(Consumer>> String constantKeywordValue = randomAlphaOfLength(5); for (int i = 0; i < numDocs; i++) { fieldValues = new LinkedHashMap<>(); + fieldValues.put("interval_year", "P150Y"); + fieldValues.put("interval_minute", "PT2H43M"); + StringBuilder builder = new StringBuilder(); builder.append("{"); builder.append("\"id\":" + i + ","); @@ -208,11 +213,7 @@ private Map columnInfo(String name, String type) { Map column = new HashMap<>(); column.put("name", name); column.put("type", type); - //if (bwcVersion.onOrAfter(Version.V_7_2_0)) { column.put("display_size", SqlDataTypes.displaySize(SqlDataTypes.fromTypeName(type))); - //} else { - //column.put("display_size", 0); - //} return unmodifiableMap(column); } @@ -226,12 +227,16 @@ private void assertAllTypesWithNodes(Map expectedResponse, List< @SuppressWarnings("unchecked") List> columns = (List>) expectedResponse.get("columns"); - String fieldsList = columns.stream().map(m -> (String) m.get("name")).collect(Collectors.toList()).stream() - .collect(Collectors.joining(", ")); + String intervalYearMonth = "INTERVAL '150' YEAR AS interval_year, "; + String intervalDayTime = "INTERVAL '163' MINUTE AS interval_minute, "; + + // get all fields names from the expected response built earlier, skipping the intervals as they execute locally + // and not taken from the index itself + String fieldsList = columns.stream().map(m -> (String) m.get("name")).filter(str -> str.startsWith("interval") == false) + .collect(Collectors.toList()).stream().collect(Collectors.joining(", ")); + String query = "SELECT " + intervalYearMonth + intervalDayTime + fieldsList + " FROM " + index + " ORDER BY id"; request.setJsonEntity( - "{\"mode\":\"jdbc\"" + versionSupport + binaryFormatSupport + ",\"query\":\"SELECT " + fieldsList + " FROM " - + index - + " ORDER BY id\"}" + "{\"mode\":\"jdbc\"" + versionSupport + binaryFormatSupport + ",\"query\":\"" + query + "\"}" ); assertBusy(() -> { assertResponse(expectedResponse, runSql(client, request)); }); } diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java index 0d637f6a53c5d..63395611e4d46 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java @@ -8,10 +8,8 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.xpack.sql.proto.ColumnInfo; @@ -140,12 +138,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVInt(rows.get(0).size()); for (List row : rows) { for (Object value : row) { - // GeoShape and Interval - if (value instanceof NamedWriteable) { - out.writeNamedWriteable((NamedWriteable) value); - } else { - out.writeGenericValue(value); - } + out.writeGenericValue(value); } } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java index 9bcfecf22e007..77fe38c210597 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java @@ -31,6 +31,8 @@ import org.elasticsearch.xpack.sql.action.SqlQueryRequest; import org.elasticsearch.xpack.sql.action.SqlQueryResponse; import org.elasticsearch.xpack.sql.execution.PlanExecutor; +import org.elasticsearch.xpack.sql.expression.literal.geo.GeoShape; +import org.elasticsearch.xpack.sql.expression.literal.interval.Interval; import org.elasticsearch.xpack.sql.proto.ColumnInfo; import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.session.Cursor; @@ -49,6 +51,7 @@ import static org.elasticsearch.action.ActionListener.wrap; import static org.elasticsearch.xpack.sql.plugin.Transports.clusterName; import static org.elasticsearch.xpack.sql.plugin.Transports.username; +import static org.elasticsearch.xpack.sql.proto.Mode.CLI; public class TransportSqlQueryAction extends HandledTransportAction { private static final Logger log = LogManager.getLogger(TransportSqlQueryAction.class); @@ -168,7 +171,7 @@ private static SqlQueryResponse createResponse(SqlQueryRequest request, ZoneId z List> rows = new ArrayList<>(); page.rowSet().forEachRow(rowView -> { List row = new ArrayList<>(rowView.columnCount()); - rowView.forEachColumn(row::add); + rowView.forEachColumn(r -> row.add(value(r, request.mode()))); rows.add(unmodifiableList(row)); }); @@ -179,4 +182,15 @@ private static SqlQueryResponse createResponse(SqlQueryRequest request, ZoneId z header, rows); } + + @SuppressWarnings("rawtypes") + private static Object value(Object r, Mode mode) { + // Intervals and GeoShape instances + if (r instanceof GeoShape) { + return r.toString(); + } else if (r instanceof Interval && mode != CLI) { + return ((Interval) r).value(); + } + return r; + } }