From 174fa45f198d994163b8591ee688e96f38927c02 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Fri, 5 Feb 2021 00:12:44 +0200 Subject: [PATCH 1/3] Integrate "fields" API into QL (#68467) --- .../sql/endpoints/translate.asciidoc | 20 +- .../eql/execution/search/RuntimeUtils.java | 12 +- .../eql/execution/search/SourceGenerator.java | 11 +- .../search/extractor/FieldHitExtractor.java | 4 +- .../extractor/TimestampFieldHitExtractor.java | 2 +- .../container/FieldExtractorRegistry.java | 36 +- .../querydsl/container/SearchHitFieldRef.java | 32 +- .../CriterionOrdinalExtractionTests.java | 6 +- .../ql/execution/search/QlSourceBuilder.java | 26 +- .../extractor/AbstractFieldHitExtractor.java | 217 +------- .../xpack/ql/querydsl/query/NestedQuery.java | 19 +- .../sql/qa/single_node/CliExplainIT.java | 29 +- .../xpack/sql/qa/FieldExtractorTestCase.java | 148 +++-- .../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 | 2 +- .../xpack/sql/execution/search/Querier.java | 16 +- .../sql/execution/search/SourceGenerator.java | 21 - .../search/extractor/FieldHitExtractor.java | 22 +- .../querydsl/container/QueryContainer.java | 24 - .../querydsl/container/SearchHitFieldRef.java | 6 +- .../search/SqlSourceBuilderTests.java | 27 +- .../extractor/ComputingExtractorTests.java | 2 +- .../extractor/FieldHitExtractorTests.java | 518 ++---------------- .../rest-api-spec/test/sql/translate.yml | 7 +- 29 files changed, 321 insertions(+), 1070 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 b7531b2bb8aea..e29f9da2c063e 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 @@ -117,7 +117,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.getDataType(), cfg.zoneId(), f.hitName(), false); } if (ref instanceof ComputedRef) { @@ -150,11 +150,11 @@ public static SearchRequest prepareRequest(Client client, 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(); + .setSource(source) + .setAllowPartialSearchResults(false) + .setIndicesOptions( + includeFrozen ? IndexResolver.FIELD_CAPS_FROZEN_INDICES_OPTIONS : IndexResolver.FIELD_CAPS_INDICES_OPTIONS) + .request(); } public static List searchHits(SearchResponse response) { 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 adff22ec90bf2..74d59279d26d5 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 @@ -6,10 +6,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; @@ -60,13 +58,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/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/extractor/FieldHitExtractor.java index 36cfc4a70201a..5ed427b383b33 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 @@ -29,9 +29,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, DataType dataType, ZoneId zoneId, String hitName, boolean arrayLeniency) { - super(name, fullFieldName, dataType, zoneId, useDocValue, 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 127efa521daeb..42530c202800c 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 @@ -10,7 +10,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.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 2c4049d1511a5..7726d0d7e3a0d 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 @@ -13,7 +13,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; @@ -46,39 +45,6 @@ private FieldExtraction createFieldExtractionFor(Expression expression) { } private FieldExtraction topHitFieldExtractor(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 - && actualField.field().getDataType().hasDocValues() == false) { - actualField = actualField.parent(); - } - } - 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 c95d6b2707e4a..9778eecd6a38b 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 @@ -13,7 +13,6 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS; -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 @@ -21,23 +20,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; } @@ -49,29 +41,17 @@ 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 if (hitName != null) { return; } - if (docValue) { - sourceBuilder.addDocField(name, format(dataType)); - } else { - sourceBuilder.addSourceField(name); - } + sourceBuilder.addFetchField(name, format(dataType)); } @Override @@ -84,10 +64,6 @@ public String toString() { return name; } - private static boolean hasDocValues(DataType dataType) { - return dataType == KEYWORD || dataType == DATETIME || dataType == DATETIME_NANOS; - } - private static String format(DataType dataType) { if (dataType == DATETIME_NANOS) { return "strict_date_optional_time_nanos"; 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 d0e20d2a6c7d4..bb92fff60559d 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, 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, true, 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/QlSourceBuilder.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java index 0e1b83f658a00..2d4bc896e6966 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 @@ -6,7 +6,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; @@ -23,8 +22,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; @@ -40,17 +38,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)); } /** @@ -66,14 +57,7 @@ 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(); - } } 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 dcc11e8ec629e..0e13c0fa137ee 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 @@ -6,33 +6,18 @@ */ package org.elasticsearch.xpack.ql.execution.search.extractor; -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 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.IgnoredFieldMapper; -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; import org.elasticsearch.xpack.ql.type.DataTypes; -import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; -import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS; -import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD; -import static org.elasticsearch.xpack.ql.type.DataTypes.SCALED_FLOAT; +import java.io.IOException; +import java.time.ZoneId; +import java.util.List; +import java.util.Objects; /** * Extractor for ES fields. Works for both 'normal' fields but also nested ones (which require hitName to be set). @@ -40,39 +25,23 @@ */ public abstract class AbstractFieldHitExtractor implements HitExtractor { - private static final Version SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION = Version.V_7_4_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, 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, dataType, zoneId, null, arrayLeniency); } - protected AbstractFieldHitExtractor(String name, String fullFieldName, DataType dataType, ZoneId zoneId, boolean useDocValue, - 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.useDocValue = useDocValue; this.arrayLeniency = arrayLeniency; this.hitName = hitName; @@ -81,23 +50,14 @@ 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(); - if (in.getVersion().onOrAfter(SWITCHED_FROM_DOCVALUES_TO_SOURCE_EXTRACTION)) { - fullFieldName = in.readOptionalString(); - } else { - fullFieldName = null; - } String typeName = in.readOptionalString(); dataType = typeName != null ? loadTypeFromName(typeName) : null; - useDocValue = in.readBoolean(); hitName = in.readOptionalString(); arrayLeniency = in.readBoolean(); - path = sourcePath(fieldName, useDocValue, hitName); zoneId = readZoneId(in); } @@ -110,11 +70,7 @@ 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.writeOptionalString(fullFieldName); - } out.writeOptionalString(dataType == null ? null : dataType.typeName()); - out.writeBoolean(useDocValue); out.writeOptionalString(hitName); out.writeBoolean(arrayLeniency); } @@ -122,36 +78,14 @@ public void writeTo(StreamOutput out) throws IOException { @Override 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) { - /* - * 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; - } - } - Map source = hit.getSourceAsMap(); - if (source != null) { - value = extractFromSource(source); - } + DocumentField field = hit.field(fieldName); + if (field != null) { + value = unwrapFieldsMultiValue(field.getValues()); } return value; } - protected Object unwrapMultiValue(Object values) { + protected Object unwrapFieldsMultiValue(Object values) { if (values == null) { return null; } @@ -162,7 +96,7 @@ protected Object unwrapMultiValue(Object values) { } else { if (isPrimitive(list) == false) { if (list.size() == 1 || arrayLeniency) { - return unwrapMultiValue(list.get(0)); + return unwrapFieldsMultiValue(list.get(0)); } else { throw new QlIllegalArgumentException("Arrays (returned by [{}]) are not supported", fieldName); } @@ -175,121 +109,13 @@ protected Object unwrapMultiValue(Object values) { 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 == DATETIME_NANOS - || dataType == SCALED_FLOAT; // because of scaling_factor - } - - private static NumberType numberType(DataType dataType) { - return NumberType.valueOf(dataType.esType().toUpperCase(Locale.ROOT)); + return values; } 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; @@ -299,10 +125,6 @@ public String fieldName() { return fieldName; } - public String fullFieldName() { - return fullFieldName; - } - public ZoneId zoneId() { return zoneId; } @@ -311,10 +133,6 @@ public DataType dataType() { return dataType; } - public boolean useDocValues() { - return useDocValue; - } - public boolean arrayLeniency() { return arrayLeniency; } @@ -332,12 +150,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/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 62b2851cb637f..89ae68c0943de 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 @@ -16,7 +16,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; @@ -116,26 +115,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/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 60245d1144dd5..82b49bd9f4250 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 @@ -40,12 +40,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\" :")); @@ -97,13 +97,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\" :")); @@ -143,7 +145,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 cd12021aa3593..2a0391290bc1d 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 @@ -49,7 +49,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); @@ -74,10 +74,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); @@ -94,10 +94,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); + } } /* @@ -107,10 +111,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); @@ -126,13 +130,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); + } } /* @@ -142,10 +150,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); @@ -162,10 +170,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); + } } /* @@ -222,14 +234,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")); } @@ -282,7 +287,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); @@ -320,7 +325,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); @@ -337,7 +342,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); @@ -355,7 +360,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); @@ -392,7 +397,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 @@ -413,11 +417,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); + } } /* @@ -467,7 +477,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)"; @@ -494,7 +503,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); @@ -561,7 +570,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")); } @@ -593,7 +602,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")); } @@ -610,9 +619,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"; @@ -646,13 +654,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"); } } @@ -670,7 +672,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"; @@ -788,7 +790,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; @@ -834,21 +836,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"); } } @@ -913,22 +906,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"); } } @@ -948,7 +928,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"; @@ -1017,7 +997,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"; @@ -1074,7 +1054,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 5edb871c6d994..2a85daefdd9e5 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 @@ -628,11 +628,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 { @@ -699,11 +698,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"); @@ -740,7 +738,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 de29df32f1a29..d3e14f5a00a52 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 @@ -54,7 +54,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 1efc1820a4170..ecb22a4f0274b 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 @@ -107,7 +107,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 a32d4deb55e88..a4c30bb35aebe 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 @@ -9,10 +9,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; @@ -32,12 +34,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 4b07b007ce68a..b5a7ccb1194f4 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 @@ -23,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; 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 e1116f19e7901..608f0b896b36a 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 @@ -87,7 +87,6 @@ // TODO: add retry/back-off public class Querier { - private static final Logger log = LogManager.getLogger(Querier.class); private final PlanExecutor planExecutor; @@ -145,11 +144,11 @@ 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(); + // 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(); } protected static void logSearchResponse(SearchResponse response, Logger logger) { @@ -488,13 +487,12 @@ 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(), - multiValueFieldLeniency); + return new FieldHitExtractor(f.name(), 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/SourceGenerator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java index 93dfba550ea52..1e9a72c67fa70 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 @@ -10,7 +10,6 @@ import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.bucket.composite.CompositeAggregationBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; -import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.sort.FieldSortBuilder; import org.elasticsearch.search.sort.NestedSortBuilder; @@ -25,9 +24,6 @@ import org.elasticsearch.xpack.sql.querydsl.container.QueryContainer; import org.elasticsearch.xpack.sql.querydsl.container.ScoreSort; -import java.util.List; - -import static java.util.Collections.singletonList; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; import static org.elasticsearch.search.sort.SortBuilders.fieldSort; import static org.elasticsearch.search.sort.SortBuilders.scoreSort; @@ -37,8 +33,6 @@ public abstract class SourceGenerator { private SourceGenerator() {} - private static final List NO_STORED_FIELD = singletonList(StoredFieldsContext._NONE_); - public static SearchSourceBuilder sourceBuilder(QueryContainer container, QueryBuilder filter, Integer size) { QueryBuilder finalQuery = null; // add the source @@ -64,7 +58,6 @@ public static SearchSourceBuilder sourceBuilder(QueryContainer container, QueryB // NB: the sortBuilder takes care of eliminating duplicates container.fields().forEach(f -> f.v1().collectFields(sortBuilder)); sortBuilder.build(source); - optimize(sortBuilder, source); // add the aggs (if present) AggregationBuilder aggBuilder = container.aggs().asAggBuilder(); @@ -166,29 +159,15 @@ private static void sorting(QueryContainer container, SearchSourceBuilder source } } - private static void optimize(QlSourceBuilder sqlSource, SearchSourceBuilder builder) { - if (sqlSource.noSource()) { - disableSource(builder); - } - } - private static void optimize(QueryContainer query, SearchSourceBuilder builder) { // if only aggs are needed, don't retrieve any docs and remove scoring if (query.isAggsOnly()) { builder.size(0); builder.trackScores(false); - // disable source fetching (only doc values are used) - disableSource(builder); } if (query.shouldTrackHits()) { builder.trackTotalHits(true); } - } - - 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 8eb5fc6b7b1a0..8f2e089c01c65 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 @@ -42,17 +42,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, DataType dataType, ZoneId zoneId, String hitName, boolean arrayLeniency) { - super(name, fullFieldName, dataType, zoneId, useDocValue, hitName, arrayLeniency); + super(name, dataType, zoneId, hitName, arrayLeniency); } public FieldHitExtractor(StreamInput in) throws IOException { @@ -91,19 +91,15 @@ 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(); if (dataType == GEO_POINT) { try { - GeoPoint geoPoint = GeoUtils.parseGeoPoint(values, true); + @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()); 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 865b1aea7b447..13b8a59882f77 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 @@ -27,7 +27,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; @@ -333,29 +332,6 @@ private FieldExtraction topHitFieldRef(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 - && 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 26c0605c87361..675c6ed0207f3 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 @@ -60,11 +60,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 ca9de884103ee..1f5922c4d1f35 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 @@ -8,26 +8,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"); @@ -35,9 +35,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/ComputingExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java index fc02d27221166..1b35d50be0f99 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 @@ -74,7 +74,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 b0dc9af8542c0..72e2dc04b3ec9 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 @@ -6,11 +6,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; @@ -31,7 +28,6 @@ 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; @@ -40,7 +36,6 @@ import static org.elasticsearch.common.time.DateUtils.toMilliSeconds; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS; -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; @@ -51,7 +46,7 @@ public class FieldHitExtractorTests extends AbstractSqlWireSerializingTestCase randomFrom(SqlDataTypes.types())), randomValueOtherThan(instance.zoneId(), ESTestCase::randomZone), - randomBoolean(), instance.hitName() + "mutated", - randomBoolean()); + randomBoolean() + ); } public void testGetDottedValueWithDocValues() { @@ -87,7 +81,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++) { @@ -104,52 +98,9 @@ public void testGetDottedValueWithDocValues() { } } - 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++) { @@ -187,463 +138,98 @@ public void testGetDateNanos() { assertEquals(zdt, extractor.extract(hit)); } - 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, 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, 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 testMultiValuedSourceValue() throws IOException { - String fieldName = randomAlphaOfLength(5); - FieldHitExtractor fe = getFieldHitExtractor(fieldName, false); - SearchHit hit = new SearchHit(1); - XContentBuilder source = JsonXContent.contentBuilder(); - source.startObject(); { - source.field(fieldName, asList("a", "b")); - } - source.endObject(); - BytesReference sourceRef = BytesReference.bytes(source); - hit.sourceRef(sourceRef); - QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> 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); + 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)); - } - - 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)); - } - - 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)); - } - - 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); - DocumentField field = new DocumentField(fieldName, singletonList("2, 1")); + + 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, 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); - SearchHit hit = new SearchHit(1, null, singletonMap(fieldName, - new DocumentField(fieldName, Arrays.asList("2,1", "3,4"))), 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)); + + 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(); } } 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 3e61e2ed0e9eb..6439b6f1be92b 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 @@ -17,11 +17,8 @@ - match: $body: size: 1000 - _source: - includes: - - int - - str - excludes: [] + _source: false + fields: [ {"field" : "int" }, {"field" : "str" } ] sort: - int: order: asc From 72a337118c1175df22deddf37969b604a737b7f2 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 9 Feb 2021 23:55:04 +0200 Subject: [PATCH 2/3] Adapt nested fields extraction from "fields" API output to the new un-flattened structure (#68745) --- .../search/extractor/AbstractFieldHitExtractor.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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 c14c73986d6bb..9a174ac7221e0 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 @@ -17,6 +17,7 @@ import java.io.IOException; import java.time.ZoneId; import java.util.List; +import java.util.Map; import java.util.Objects; /** @@ -78,7 +79,13 @@ public void writeTo(StreamOutput out) throws IOException { @Override public Object extract(SearchHit hit) { Object value = null; - DocumentField field = hit.field(fieldName); + DocumentField field = null; + if (hitName != null) { + // a nested field value is grouped under the nested parent name (ie dep.dep_name lives under "dep":[{dep_name:value}]) + field = hit.field(hitName); + } else { + field = hit.field(fieldName); + } if (field != null) { value = unwrapFieldsMultiValue(field.getValues()); } @@ -89,6 +96,10 @@ protected Object unwrapFieldsMultiValue(Object values) { if (values == null) { return null; } + if (values instanceof Map && hitName != null) { + // extract the sub-field from a nested field (dep.dep_name -> dep_name) + return unwrapFieldsMultiValue(((Map) values).get(fieldName.substring(hitName.length() + 1))); + } if (values instanceof List) { List list = (List) values; if (list.isEmpty()) { From 801de6a5d1dbf5ac7bb3bffc0967d7f986ba9eb6 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Wed, 10 Feb 2021 01:32:02 +0200 Subject: [PATCH 3/3] QL: retry SQL and EQL requests in a mixed-node (rolling upgrade) cluster (#68602) --- x-pack/plugin/eql/qa/mixed-node/build.gradle | 66 +++++ .../xpack/eql/qa/mixed_node/EqlSearchIT.java | 239 +++++++++++++++ .../src/test/resources/eql_data.json | 30 ++ .../src/test/resources/eql_mapping.json | 35 +++ .../execution/search/BasicQueryClient.java | 4 +- .../eql/execution/search/RuntimeUtils.java | 18 +- .../eql/plugin/TransportEqlSearchAction.java | 26 +- .../xpack/eql/analysis/CancellationTests.java | 30 +- .../ql/execution/search/QlSourceBuilder.java | 2 + .../xpack/ql/plugin/TransportActionUtils.java | 71 +++++ .../org/elasticsearch/xpack/ql/TestNode.java | 41 +++ .../org/elasticsearch/xpack/ql/TestNodes.java | 43 +++ .../org/elasticsearch/xpack/ql/TestUtils.java | 35 +++ x-pack/plugin/sql/qa/mixed-node/build.gradle | 66 +++++ .../xpack/sql/qa/mixed_node/SqlSearchIT.java | 273 ++++++++++++++++++ .../src/test/resources/all_field_types.json | 59 ++++ .../xpack/sql/execution/search/Querier.java | 18 +- .../sql/plugin/TransportSqlQueryAction.java | 20 +- 18 files changed, 1043 insertions(+), 33 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_data.json create mode 100644 x-pack/plugin/eql/qa/mixed-node/src/test/resources/eql_mapping.json create mode 100644 x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/plugin/TransportActionUtils.java create mode 100644 x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestNode.java create mode 100644 x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestNodes.java 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 create mode 100644 x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.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..5e041a2aa5f2d --- /dev/null +++ b/x-pack/plugin/eql/qa/mixed-node/build.gradle @@ -0,0 +1,66 @@ +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(xpackModule('ql:test'))) + testImplementation project(path: xpackModule('eql'), configuration: 'default') +} + +tasks.named("integTest").configure{ enabled = false} + +for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it.onOrAfter('7.10.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' + // for debugging purposes + // 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" + } + + // run these bwc tests as part of the "check" task + 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..8e3c45f816ffc --- /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,239 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.eql.qa.mixed_node; + +import org.apache.http.HttpHost; +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.xpack.ql.TestNode; +import org.elasticsearch.xpack.ql.TestNodes; +import org.junit.After; +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 static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static java.util.Collections.unmodifiableList; +import static org.elasticsearch.xpack.ql.TestUtils.buildNodeAndVersions; +import static org.elasticsearch.xpack.ql.TestUtils.readResource; + +/** + * 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 String index = "test_eql_mixed_versions"; + private static int numShards; + private static int numReplicas = 1; + private static int numDocs; + private static TestNodes nodes; + private static List newNodes; + private static List bwcNodes; + + @Before + public void createIndex() throws IOException { + nodes = buildNodeAndVersions(client()); + numShards = nodes.size(); + numDocs = randomIntBetween(numShards, 15); + newNodes = new ArrayList<>(nodes.getNewNodes()); + bwcNodes = new ArrayList<>(nodes.getBWCNodes()); + + String mappings = readResource(EqlSearchIT.class.getResourceAsStream("/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)) { + 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 = prepareEventsTestData(event); + try ( + RestClient client = buildClient(restClientSettings(), + nodesList.stream().map(TestNode::getPublishAddress).toArray(HttpHost[]::new)) + ) { + // filter only the relevant bits of the response + 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(TestNode::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"); + } + + private Map prepareEventsTestData(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 emptyMap(); + } + 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 String bulkEntries = readResource(EqlSearchIT.class.getResourceAsStream("/eql_data.json")); + Request request = new Request("POST", index + "/_bulk?refresh"); + request.setJsonEntity(bulkEntries); + 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); + } + } +} diff --git a/x-pack/plugin/eql/qa/mixed-node/src/test/resources/eql_data.json b/x-pack/plugin/eql/qa/mixed-node/src/test/resources/eql_data.json new file mode 100644 index 0000000000000..3fc6e28834ea8 --- /dev/null +++ b/x-pack/plugin/eql/qa/mixed-node/src/test/resources/eql_data.json @@ -0,0 +1,30 @@ +{"index":{"_id":1}} +{"@timestamp":"1234567891","event_type":"success","sequence":1,"correlation_success1":"A","correlation_success2":"B"} +{"index":{"_id":2}} +{"@timestamp":"1234567892","event_type":"failure","sequence":2,"correlation_failure1":"A","correlation_failure2":"B"} +{"index":{"_id":3}} +{"@timestamp":"1234567893","event_type":"success","sequence":3,"correlation_success1":"A","correlation_success2":"A"} +{"index":{"_id":4}} +{"@timestamp":"1234567894","event_type":"success","sequence":4,"correlation_success1":"C","correlation_success2":"C"} +{"index":{"_id":5}} +{"@timestamp":"1234567895","event_type":"failure","sequence":5,"correlation_failure1":"B","correlation_failure2":"C"} +{"index":{"_id":6}} +{"@timestamp":"1234567896","event_type":"success","sequence":1,"correlation_success1":"A","correlation_success2":"A"} +{"index":{"_id":7}} +{"@timestamp":"1234567897","event_type":"failure","sequence":1,"correlation_failure1":"A","correlation_failure2":"A"} +{"index":{"_id":8}} +{"@timestamp":"1234567898","event_type":"success","sequence":3,"correlation_success1":"A","correlation_success2":"A"} +{"index":{"_id":9}} +{"@timestamp":"1234567899","event_type":"success","sequence":4,"correlation_success1":"C","correlation_success2":"B"} +{"index":{"_id":10}} +{"@timestamp":"12345678910","event_type":"failure","sequence":4,"correlation_failure1":"B","correlation_failure2":"B"} +{"index":{"_id":11}} +{"@timestamp":"12345678911","event_type":"success","sequence":1,"correlation_success1":"A","correlation_success2":"A"} +{"index":{"_id":12}} +{"@timestamp":"12345678912","event_type":"failure","sequence":1,"correlation_failure1":"A","correlation_failure2":"B"} +{"index":{"_id":13}} +{"@timestamp":"12345678913","event_type":"success","sequence":3,"correlation_success1":"A","correlation_success2":"A"} +{"index":{"_id":14}} +{"@timestamp":"12345678914","event_type":"success","sequence":44,"correlation_success1":"C","correlation_success2":"D"} +{"index":{"_id":15}} +{"@timestamp":"12345678999","event_type":"failure","sequence":44,"correlation_failure1":"C","correlation_failure2":"D"} 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..f56dea6722183 --- /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/BasicQueryClient.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/BasicQueryClient.java index a357491496085..38a2b292cb9c9 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/BasicQueryClient.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/BasicQueryClient.java @@ -57,7 +57,7 @@ public void query(QueryRequest request, ActionListener listener) // set query timeout searchSource.timeout(cfg.requestTimeout()); - SearchRequest search = prepareRequest(client, searchSource, false, indices); + SearchRequest search = prepareRequest(searchSource, false, indices); search(search, searchLogListener(listener, log)); } @@ -138,7 +138,7 @@ public void fetchHits(Iterable> refs, ActionListener 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 8e7a5133d1117..e454b7c908c4e 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 @@ -6,7 +6,10 @@ */ 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.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.client.Client; @@ -43,14 +46,17 @@ import static org.elasticsearch.action.ActionListener.wrap; import static org.elasticsearch.xpack.core.ClientHelper.ASYNC_SEARCH_ORIGIN; +import static org.elasticsearch.xpack.ql.plugin.TransportActionUtils.executeRequestWithRetryAttempt; 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 @@ -64,6 +70,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); @@ -78,8 +85,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 @@ -99,13 +105,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(); @@ -122,8 +130,12 @@ 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)); + executeRequestWithRetryAttempt(clusterService, listener::onFailure, + onFailure -> planExecutor.eql(cfg, request.query(), params, + wrap(r -> listener.onResponse(createResponse(r, task.getExecutionId())), onFailure)), + node -> transportService.sendRequest(node, EqlSearchAction.NAME, request, + new ActionListenerResponseHandler<>(listener, EqlSearchResponse::new, ThreadPool.Names.SAME)), + log); } static EqlSearchResponse createResponse(Results results, AsyncExecutionId id) { diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/CancellationTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/CancellationTests.java index e3704ba26540a..d69f522f8bf47 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/CancellationTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/CancellationTests.java @@ -14,10 +14,14 @@ import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.tasks.TaskCancelledException; import org.elasticsearch.tasks.TaskId; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.eql.action.EqlSearchRequest; import org.elasticsearch.xpack.eql.action.EqlSearchResponse; import org.elasticsearch.xpack.eql.action.EqlSearchTask; @@ -51,12 +55,13 @@ public void testCancellationBeforeFieldCaps() throws InterruptedException { Client client = mock(Client.class); EqlSearchTask task = mock(EqlSearchTask.class); when(task.isCancelled()).thenReturn(true); + ClusterService mockClusterService = mockClusterService(); IndexResolver indexResolver = new IndexResolver(client, randomAlphaOfLength(10), DefaultDataTypeRegistry.INSTANCE); PlanExecutor planExecutor = new PlanExecutor(client, indexResolver, new NamedWriteableRegistry(Collections.emptyList())); CountDownLatch countDownLatch = new CountDownLatch(1); - TransportEqlSearchAction.operation(planExecutor, task, new EqlSearchRequest().query("foo where blah"), "", "", "node_id", - new ActionListener<>() { + TransportEqlSearchAction.operation(planExecutor, task, new EqlSearchRequest().query("foo where blah"), "", + mock(TransportService.class), mockClusterService, new ActionListener<>() { @Override public void onResponse(EqlSearchResponse eqlSearchResponse) { fail("Shouldn't be here"); @@ -96,10 +101,10 @@ public void testCancellationBeforeSearch() throws InterruptedException { AtomicBoolean cancelled = new AtomicBoolean(false); EqlSearchTask task = mock(EqlSearchTask.class); - String nodeId = randomAlphaOfLength(10); long taskId = randomNonNegativeLong(); when(task.isCancelled()).then(invocationOnMock -> cancelled.get()); when(task.getId()).thenReturn(taskId); + ClusterService mockClusterService = mockClusterService(); String[] indices = new String[]{"endgame"}; @@ -119,7 +124,7 @@ public void testCancellationBeforeSearch() throws InterruptedException { PlanExecutor planExecutor = new PlanExecutor(client, indexResolver, new NamedWriteableRegistry(Collections.emptyList())); CountDownLatch countDownLatch = new CountDownLatch(1); TransportEqlSearchAction.operation(planExecutor, task, new EqlSearchRequest().indices("endgame") - .query("process where foo==3"), "", "", nodeId, new ActionListener<>() { + .query("process where foo==3"), "", mock(TransportService.class), mockClusterService, new ActionListener<>() { @Override public void onResponse(EqlSearchResponse eqlSearchResponse) { fail("Shouldn't be here"); @@ -149,6 +154,7 @@ public void testCancellationDuringSearch() throws InterruptedException { long taskId = randomNonNegativeLong(); when(task.isCancelled()).thenReturn(false); when(task.getId()).thenReturn(taskId); + ClusterService mockClusterService = mockClusterService(nodeId); String[] indices = new String[]{"endgame"}; @@ -183,7 +189,7 @@ public void testCancellationDuringSearch() throws InterruptedException { PlanExecutor planExecutor = new PlanExecutor(client, indexResolver, new NamedWriteableRegistry(Collections.emptyList())); CountDownLatch countDownLatch = new CountDownLatch(1); TransportEqlSearchAction.operation(planExecutor, task, new EqlSearchRequest().indices("endgame") - .query("process where foo==3"), "", "", nodeId, new ActionListener<>() { + .query("process where foo==3"), "", mock(TransportService.class), mockClusterService, new ActionListener<>() { @Override public void onResponse(EqlSearchResponse eqlSearchResponse) { fail("Shouldn't be here"); @@ -207,4 +213,18 @@ public void onFailure(Exception e) { verifyNoMoreInteractions(client, task); } + private ClusterService mockClusterService() { + return mockClusterService(null); + } + + private ClusterService mockClusterService(String nodeId) { + final ClusterService mockClusterService = mock(ClusterService.class); + final DiscoveryNode mockNode = mock(DiscoveryNode.class); + final ClusterName mockClusterName = mock(ClusterName.class); + when(mockNode.getId()).thenReturn(nodeId == null ? randomAlphaOfLength(10) : nodeId); + when(mockClusterService.localNode()).thenReturn(mockNode); + when(mockClusterName.value()).thenReturn(randomAlphaOfLength(10)); + when(mockClusterService.getClusterName()).thenReturn(mockClusterName); + return mockClusterService; + } } 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 2d4bc896e6966..4f0c1f623b772 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 @@ -6,6 +6,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; @@ -21,6 +22,7 @@ * the resulting ES document as a field. */ public class QlSourceBuilder { + public static final Version SWITCH_TO_FIELDS_API_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/ql/src/main/java/org/elasticsearch/xpack/ql/plugin/TransportActionUtils.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/plugin/TransportActionUtils.java new file mode 100644 index 0000000000000..f839ba54c55ef --- /dev/null +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/plugin/TransportActionUtils.java @@ -0,0 +1,71 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +package org.elasticsearch.xpack.ql.plugin; + +import org.apache.logging.log4j.Logger; +import org.elasticsearch.action.search.SearchPhaseExecutionException; +import org.elasticsearch.action.search.VersionMismatchException; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.xpack.ql.util.Holder; + +import java.util.function.Consumer; + +public final class TransportActionUtils { + + /** + * Execute a *QL request and re-try it in case the first request failed with a {@code VersionMismatchException} + * + * @param clusterService The cluster service instance + * @param onFailure On-failure handler in case the request doesn't fail with a {@code VersionMismatchException} + * @param queryRunner *QL query execution code, typically a Plan Executor running the query + * @param retryRequest Re-trial logic + * @param log Log4j logger + */ + public static void executeRequestWithRetryAttempt(ClusterService clusterService, Consumer onFailure, + Consumer> queryRunner, Consumer retryRequest, Logger log) { + + Holder retrySecondTime = new Holder(false); + queryRunner.accept(e -> { + // the search request likely ran 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) { + if (log.isDebugEnabled()) { + log.debug("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.isDebugEnabled()) { + log.debug("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 + retryRequest.accept(candidateNode); + } else { + retrySecondTime.set(true); + } + } else { + onFailure.accept(e); + } + }); + if (retrySecondTime.get()) { + if (log.isDebugEnabled()) { + log.debug("No candidate node found, likely all were upgraded in the meantime. Re-trying the original request."); + } + queryRunner.accept(onFailure); + } + } +} diff --git a/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestNode.java b/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestNode.java new file mode 100644 index 0000000000000..746ff91dc902e --- /dev/null +++ b/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestNode.java @@ -0,0 +1,41 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ql; + +import org.apache.http.HttpHost; +import org.elasticsearch.Version; + +public final class TestNode { + + private final String id; + private final Version version; + private final HttpHost publishAddress; + + public TestNode(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/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestNodes.java b/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestNodes.java new file mode 100644 index 0000000000000..9681e66183530 --- /dev/null +++ b/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestNodes.java @@ -0,0 +1,43 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ql; + +import org.elasticsearch.Version; + +import java.util.HashMap; +import java.util.List; +import java.util.stream.Collectors; + +public final class TestNodes extends HashMap { + + public void add(TestNode 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(TestNode::toString).collect(Collectors.joining("\n")) + '}'; + } +} diff --git a/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestUtils.java b/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestUtils.java index 84eb576d2dd31..bc933c8b1fe9d 100644 --- a/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestUtils.java +++ b/x-pack/plugin/ql/test/src/main/java/org/elasticsearch/xpack/ql/TestUtils.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.ql; +import org.apache.http.HttpHost; +import org.elasticsearch.Version; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; @@ -16,6 +18,7 @@ import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.test.rest.yaml.ObjectPath; import org.elasticsearch.xpack.ql.expression.Expression; import org.elasticsearch.xpack.ql.expression.FieldAttribute; import org.elasticsearch.xpack.ql.expression.Literal; @@ -265,4 +268,36 @@ public static Tuple pathAndName(String string) { } return new Tuple<>(folder, file); } + + public static TestNodes buildNodeAndVersions(RestClient client) throws IOException { + Response response = client.performRequest(new Request("GET", "_nodes")); + ObjectPath objectPath = ObjectPath.createFromResponse(response); + Map nodesAsMap = objectPath.evaluate("nodes"); + TestNodes nodes = new TestNodes(); + for (String id : nodesAsMap.keySet()) { + nodes.add( + new TestNode( + id, + Version.fromString(objectPath.evaluate("nodes." + id + ".version")), + HttpHost.create(objectPath.evaluate("nodes." + id + ".http.publish_address")) + ) + ); + } + return nodes; + } + + public static String readResource(InputStream input) throws IOException { + StringBuilder builder = new StringBuilder(); + try (BufferedReader reader = new BufferedReader(new InputStreamReader(input, 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(); + } + } } 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..8a39ecc795848 --- /dev/null +++ b/x-pack/plugin/sql/qa/mixed-node/build.gradle @@ -0,0 +1,66 @@ +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(xpackModule('ql:test'))) + testImplementation project(path: xpackModule('sql'), configuration: 'default') +} + +tasks.named("integTest").configure{ enabled = false} + +// A bug (https://github.com/elastic/elasticsearch/issues/68439) limits us to perform tests with versions from 7.10.3 onwards +for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible.findAll { it.onOrAfter('7.10.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' + // for debugging purposes + // setting 'logger.org.elasticsearch.xpack.sql.plugin.TransportSqlQueryAction', '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" + } + + // run these bwc tests as part of the "check" task + 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..7c3c304e1c025 --- /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,273 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +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.xpack.ql.TestNode; +import org.elasticsearch.xpack.ql.TestNodes; +import org.elasticsearch.xpack.sql.type.SqlDataTypes; +import org.junit.After; +import org.junit.Before; + +import java.io.IOException; +import java.io.InputStream; +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.unmodifiableMap; +import static org.elasticsearch.xpack.ql.TestUtils.buildNodeAndVersions; +import static org.elasticsearch.xpack.ql.TestUtils.readResource; +import static org.elasticsearch.xpack.ql.execution.search.QlSourceBuilder.SWITCH_TO_FIELDS_API_VERSION; + +public class SqlSearchIT extends ESRestTestCase { + + /* + * The version where we made a significant change to how we query ES and how we interpret the results we get from ES, is 7.12 + * (the switch from extracting from _source and docvalues to using the "fields" API). The behavior of the tests is slightly + * changed on some versions and it all depends on when this above mentioned change was made. + */ + 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; + private static TestNodes nodes; + private static List newNodes; + private static List bwcNodes; + private static Version bwcVersion; + private static Version newVersion; + private static boolean isBwcNodeBeforeFieldsApiInQL; + private static boolean isBwcNodeBeforeFieldsApiInES; + + @Before + public void createIndex() throws IOException { + nodes = buildNodeAndVersions(client()); + numShards = nodes.size(); + numDocs = randomIntBetween(numShards, 15); + newNodes = new ArrayList<>(nodes.getNewNodes()); + bwcNodes = new ArrayList<>(nodes.getBWCNodes()); + bwcVersion = nodes.getBWCNodes().get(0).getVersion(); + newVersion = nodes.getNewNodes().get(0).getVersion(); + // TODO: remove the 8.0.0 version check after the code reaches 7.x as well + isBwcNodeBeforeFieldsApiInQL = newVersion == Version.V_8_0_0 || bwcVersion.before(FIELDS_API_QL_INTRODUCTION); + isBwcNodeBeforeFieldsApiInES = bwcVersion.before(SWITCH_TO_FIELDS_API_VERSION); + + String mappings = readResource(SqlSearchIT.class.getResourceAsStream("/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 + ); + } + + @After + public void cleanUpIndex() throws IOException { + if (indexExists(index)) { + deleteIndex(index); + } + } + + 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 (isBwcNodeBeforeFieldsApiInQL) { + 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) -> { + Float randomFloat = randomFloat(); + builder.append(","); + if (isBwcNodeBeforeFieldsApiInQL && isBwcNodeBeforeFieldsApiInES) { + 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, newNodes); + } + + @SuppressWarnings("unchecked") + private Map prepareTestData(Consumer>> additionalColumns, + 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")); + 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<>(); + fieldValues.put("interval_year", "P150Y"); + fieldValues.put("interval_minute", "PT2H43M"); + + 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; + } + + private Map columnInfo(String name, String type) { + Map column = new HashMap<>(); + column.put("name", name); + column.put("type", type); + column.put("display_size", SqlDataTypes.displaySize(SqlDataTypes.fromTypeName(type))); + return unmodifiableMap(column); + } + + private void assertAllTypesWithNodes(Map expectedResponse, List nodesList) throws Exception { + try ( + RestClient client = buildClient(restClientSettings(), + nodesList.stream().map(TestNode::getPublishAddress).toArray(HttpHost[]::new)) + ) { + Request request = new Request("POST", "_sql"); + String version = ",\"version\":\"" + newVersion.toString() + "\""; + String binaryFormat = ",\"binary_format\":\"false\""; + + @SuppressWarnings("unchecked") + List> columns = (List>) expectedResponse.get("columns"); + 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\"" + version + binaryFormat + ",\"query\":\"" + query + "\"}" + ); + assertBusy(() -> { assertResponse(expectedResponse, runSql(client, request)); }); + } + } + + 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); + } + } +} 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..491be6f0f96b6 --- /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/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 a839ae92fa109..d40909cad9ce3 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 @@ -84,6 +84,7 @@ import static java.util.Collections.singletonList; import static org.elasticsearch.action.ActionListener.wrap; +import static org.elasticsearch.xpack.ql.execution.search.QlSourceBuilder.SWITCH_TO_FIELDS_API_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(SWITCH_TO_FIELDS_API_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/plugin/TransportSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java index 4ff9ef54d8452..e2f067ed95f3f 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 @@ -6,7 +6,10 @@ */ 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.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.cluster.service.ClusterService; @@ -43,15 +46,18 @@ import static java.util.Collections.unmodifiableList; import static org.elasticsearch.action.ActionListener.wrap; +import static org.elasticsearch.xpack.ql.plugin.TransportActionUtils.executeRequestWithRetryAttempt; 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); private final SecurityContext securityContext; private final ClusterService clusterService; private final PlanExecutor planExecutor; private final SqlLicenseChecker sqlLicenseChecker; + private final TransportService transportService; @Inject public TransportSqlQueryAction(Settings settings, ClusterService clusterService, TransportService transportService, @@ -64,19 +70,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(), @@ -84,8 +92,12 @@ static void operation(PlanExecutor planExecutor, SqlQueryRequest request, Action request.fieldMultiValueLeniency(), request.indexIncludeFrozen()); if (Strings.hasText(request.cursor()) == false) { - planExecutor.sql(cfg, request.query(), request.params(), - wrap(p -> listener.onResponse(createResponseWithSchema(request, p)), listener::onFailure)); + executeRequestWithRetryAttempt(clusterService, listener::onFailure, + onFailure -> planExecutor.sql(cfg, request.query(), request.params(), + wrap(p -> listener.onResponse(createResponseWithSchema(request, p)), onFailure)), + node -> transportService.sendRequest(node, SqlQueryAction.NAME, request, + new ActionListenerResponseHandler<>(listener, SqlQueryResponse::new, ThreadPool.Names.SAME)), + log); } else { Tuple decoded = Cursors.decodeFromStringWithZone(request.cursor()); planExecutor.nextPage(cfg, decoded.v1(),