diff --git a/docs/changelog/100656.yaml b/docs/changelog/100656.yaml new file mode 100644 index 0000000000000..1ee9a2ad0e47a --- /dev/null +++ b/docs/changelog/100656.yaml @@ -0,0 +1,6 @@ +pr: 100656 +summary: "ESQL: fix non-null value being returned for unsupported data types in `ValueSources`" +area: ES|QL +type: bug +issues: + - 100048 diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/ValueSources.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/ValueSources.java index e5ce5436990b7..29a539b1e068e 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/ValueSources.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/ValueSources.java @@ -69,6 +69,18 @@ public static List sources( sources.add(new ValueSourceInfo(new NullValueSourceType(), new NullValueSource(), elementType, ctx.getIndexReader())); continue; // the field does not exist in this context } + if (asUnsupportedSource) { + sources.add( + new ValueSourceInfo( + new UnsupportedValueSourceType(fieldType.typeName()), + new UnsupportedValueSource(null), + elementType, + ctx.getIndexReader() + ) + ); + HeaderWarning.addWarning("Field [{}] cannot be retrieved, it is unsupported or not indexed; returning null", fieldName); + continue; + } if (fieldType.hasDocValues() == false) { // MatchOnlyTextFieldMapper class lives in the mapper-extras module. We use string equality @@ -99,19 +111,7 @@ public static List sources( var fieldContext = new FieldContext(fieldName, fieldData, fieldType); var vsType = fieldData.getValuesSourceType(); var vs = vsType.getField(fieldContext, null); - - if (asUnsupportedSource) { - sources.add( - new ValueSourceInfo( - new UnsupportedValueSourceType(fieldType.typeName()), - new UnsupportedValueSource(vs), - elementType, - ctx.getIndexReader() - ) - ); - } else { - sources.add(new ValueSourceInfo(vsType, vs, elementType, ctx.getIndexReader())); - } + sources.add(new ValueSourceInfo(vsType, vs, elementType, ctx.getIndexReader())); } return sources; diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/DefaultSortableTopNEncoder.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/DefaultSortableTopNEncoder.java index 8634d87e2932f..6ccde6b76ce13 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/DefaultSortableTopNEncoder.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/DefaultSortableTopNEncoder.java @@ -23,7 +23,7 @@ public BytesRef decodeBytesRef(BytesRef bytes, BytesRef scratch) { @Override public String toString() { - return "DefaultUnsortable"; + return "DefaultSortable"; } @Override diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/TopNEncoder.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/TopNEncoder.java index 2d8f2666ff2f2..f1fb7cb7736c5 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/TopNEncoder.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/TopNEncoder.java @@ -41,6 +41,11 @@ public interface TopNEncoder { */ VersionTopNEncoder VERSION = new VersionTopNEncoder(); + /** + * Placeholder encoder for unsupported data types. + */ + UnsupportedTypesTopNEncoder UNSUPPORTED = new UnsupportedTypesTopNEncoder(); + void encodeLong(long value, BreakingBytesRefBuilder bytesRefBuilder); long decodeLong(BytesRef bytes); diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/UnsupportedTypesTopNEncoder.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/UnsupportedTypesTopNEncoder.java new file mode 100644 index 0000000000000..d80d70970409e --- /dev/null +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/UnsupportedTypesTopNEncoder.java @@ -0,0 +1,45 @@ +/* + * 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.compute.operator.topn; + +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.compute.operator.BreakingBytesRefBuilder; + +/** + * TopNEncoder for data types that are unsupported. This is just a placeholder class, reaching the encode/decode methods here is a bug. + * + * While this class is needed to build the TopNOperator value and key extractors infrastructure, encoding/decoding is needed + * when actually sorting on a field (which shouldn't be possible for unsupported data types) using key extractors, or when encoding/decoding + * unsupported data types fields values (which should always be "null" by convention) using value extractors. + */ +class UnsupportedTypesTopNEncoder extends SortableTopNEncoder { + @Override + public int encodeBytesRef(BytesRef value, BreakingBytesRefBuilder bytesRefBuilder) { + throw new UnsupportedOperationException("Encountered a bug; trying to encode an unsupported data type value for TopN"); + } + + @Override + public BytesRef decodeBytesRef(BytesRef bytes, BytesRef scratch) { + throw new UnsupportedOperationException("Encountered a bug; trying to decode an unsupported data type value for TopN"); + } + + @Override + public String toString() { + return "UnsupportedTypesTopNEncoder"; + } + + @Override + public TopNEncoder toSortable() { + return this; + } + + @Override + public TopNEncoder toUnsortable() { + return this; + } +} diff --git a/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/40_tsdb.yml b/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/40_tsdb.yml index 14ae1ff98d8ad..895a1718b2cbc 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/40_tsdb.yml +++ b/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/40_tsdb.yml @@ -106,6 +106,8 @@ setup: --- load everything: - do: + allowed_warnings_regex: + - "Field \\[.*\\] cannot be retrieved, it is unsupported or not indexed; returning null" esql.query: body: query: 'from test' @@ -156,6 +158,8 @@ filter on counter: --- from doc with aggregate_metric_double: - do: + allowed_warnings_regex: + - "Field \\[.*\\] cannot be retrieved, it is unsupported or not indexed; returning null" esql.query: body: query: 'from test2' @@ -183,6 +187,8 @@ stats on aggregate_metric_double: --- from index pattern unsupported counter: - do: + allowed_warnings_regex: + - "Field \\[.*\\] cannot be retrieved, it is unsupported or not indexed; returning null" esql.query: body: query: 'FROM test*' diff --git a/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/40_unsupported_types.yml b/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/40_unsupported_types.yml index 44af9559598ab..ad0c7b516fde1 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/40_unsupported_types.yml +++ b/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/40_unsupported_types.yml @@ -263,3 +263,96 @@ unsupported: - match: { columns.0.name: shape } - match: { columns.0.type: unsupported } - length: { values: 0 } + +--- +unsupported with sort: + - do: + allowed_warnings_regex: + - "Field \\[.*\\] cannot be retrieved, it is unsupported or not indexed; returning null" + esql.query: + body: + query: 'from test | sort some_doc.bar' + + - match: { columns.0.name: aggregate_metric_double } + - match: { columns.0.type: unsupported } + - match: { columns.1.name: binary } + - match: { columns.1.type: unsupported } + - match: { columns.2.name: completion } + - match: { columns.2.type: unsupported } + - match: { columns.3.name: date_nanos } + - match: { columns.3.type: unsupported } + - match: { columns.4.name: date_range } + - match: { columns.4.type: unsupported } + - match: { columns.5.name: dense_vector } + - match: { columns.5.type: unsupported } + - match: { columns.6.name: double_range } + - match: { columns.6.type: unsupported } + - match: { columns.7.name: float_range } + - match: { columns.7.type: unsupported } + - match: { columns.8.name: geo_point } + - match: { columns.8.type: unsupported } + - match: { columns.9.name: geo_point_alias } + - match: { columns.9.type: unsupported } + - match: { columns.10.name: histogram } + - match: { columns.10.type: unsupported } + - match: { columns.11.name: integer_range } + - match: { columns.11.type: unsupported } + - match: { columns.12.name: ip_range } + - match: { columns.12.type: unsupported } + - match: { columns.13.name: long_range } + - match: { columns.13.type: unsupported } + - match: { columns.14.name: match_only_text } + - match: { columns.14.type: text } + - match: { columns.15.name: name } + - match: { columns.15.type: keyword } + - match: { columns.16.name: rank_feature } + - match: { columns.16.type: unsupported } + - match: { columns.17.name: rank_features } + - match: { columns.17.type: unsupported } + - match: { columns.18.name: search_as_you_type } + - match: { columns.18.type: unsupported } + - match: { columns.19.name: search_as_you_type._2gram } + - match: { columns.19.type: unsupported } + - match: { columns.20.name: search_as_you_type._3gram } + - match: { columns.20.type: unsupported } + - match: { columns.21.name: search_as_you_type._index_prefix } + - match: { columns.21.type: unsupported } + - match: { columns.22.name: shape } + - match: { columns.22.type: unsupported } + - match: { columns.23.name: some_doc.bar } + - match: { columns.23.type: long } + - match: { columns.24.name: some_doc.foo } + - match: { columns.24.type: keyword } + - match: { columns.25.name: text } + - match: { columns.25.type: text } + - match: { columns.26.name: token_count } + - match: { columns.26.type: integer } + + - length: { values: 1 } + - match: { values.0.0: null } + - match: { values.0.1: null } + - match: { values.0.2: null } + - match: { values.0.3: null } + - match: { values.0.4: null } + - match: { values.0.5: null } + - match: { values.0.6: null } + - match: { values.0.7: null } + - match: { values.0.8: null } + - match: { values.0.9: null } + - match: { values.0.10: null } + - match: { values.0.11: null } + - match: { values.0.12: null } + - match: { values.0.13: null } + - match: { values.0.14: "foo bar baz" } + - match: { values.0.15: Alice } + - match: { values.0.16: null } + - match: { values.0.17: null } + - match: { values.0.18: null } + - match: { values.0.19: null } + - match: { values.0.20: null } + - match: { values.0.21: null } + - match: { values.0.22: null } + - match: { values.0.23: 12 } + - match: { values.0.24: xy } + - match: { values.0.25: "foo bar" } + - match: { values.0.26: 3 } diff --git a/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/50_index_patterns.yml b/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/50_index_patterns.yml index 280a32aa10cd3..ff327b2592c88 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/50_index_patterns.yml +++ b/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/50_index_patterns.yml @@ -267,6 +267,8 @@ disjoint_mappings: --- same_name_different_type: + - skip: + features: allowed_warnings_regex - do: indices.create: index: test1 @@ -307,6 +309,8 @@ same_name_different_type: - { "message": 2 } - do: + allowed_warnings_regex: + - "Field \\[.*\\] cannot be retrieved, it is unsupported or not indexed; returning null" esql.query: body: query: 'from test1,test2 ' diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java index 1c26de4a599f5..b86072e1b6da0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java @@ -380,6 +380,8 @@ private PhysicalOperation planTopN(TopNExec topNExec, LocalExecutionPlannerConte case "version" -> TopNEncoder.VERSION; case "boolean", "null", "byte", "short", "integer", "long", "double", "float", "half_float", "datetime", "date_period", "time_duration", "object", "nested", "scaled_float", "unsigned_long", "_doc" -> TopNEncoder.DEFAULT_SORTABLE; + // unsupported fields are encoded as BytesRef, we'll use the same encoder; all values should be null at this point + case "unsupported" -> TopNEncoder.UNSUPPORTED; default -> throw new EsqlIllegalArgumentException("No TopN sorting encoder for type " + inverse.get(channel).type()); }; }