From a1a6358af606da8ea6931a1a49396b46c12ec5a5 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 11 Jun 2025 14:39:21 +0200 Subject: [PATCH 1/3] ESQL: Fix FieldAttribute name usage in InferNonNullAggConstraint (#128910) * Fix InferNonNullAggConstraint with union types * Begin fixing LucenePushdownPredicates with union types * Introduce a dedicated wrapper record FieldName to be used where field names are really required. The fixes consist of using FieldAttribute.fieldName() instead of .name() or .field().name(). .name() can be some temporary string unrelated to the actual name of the Lucene index field, whereas .field().name() doesn't know about parent fields; .fieldName() gives the full field name (from the root of the document). The biggest offender of such misuse is SearchStats; make this always require a FieldName, not a String - and make FieldAttribute#fieldName handily return an instance of FieldName so users of SearchStats don't accidentally use the return value of FieldAttribute#name. (cherry picked from commit 0850bd713b4b5c85e8d7449eae56967c151d4ef2) # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchContextStats.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java --- docs/changelog/128910.yaml | 5 ++ .../esql/core/expression/FieldAttribute.java | 41 +++++++---- .../xpack/esql/core/type/EsField.java | 2 +- .../xpack/esql/EsqlTestUtils.java | 41 +++++------ .../src/main/resources/union_types.csv-spec | 13 ++++ .../xpack/esql/analysis/Analyzer.java | 2 +- .../function/UnsupportedAttribute.java | 11 +-- .../local/InferNonNullAggConstraint.java | 4 +- .../local/LucenePushdownPredicates.java | 16 +++-- .../physical/local/PushStatsToSource.java | 2 +- .../planner/EsPhysicalOperationProviders.java | 4 +- .../xpack/esql/stats/SearchContextStats.java | 68 +++++++++++-------- .../xpack/esql/stats/SearchStats.java | 44 ++++++------ .../LocalLogicalPlanOptimizerTests.java | 63 +++++++++++++++-- .../LocalPhysicalPlanOptimizerTests.java | 18 ++--- .../optimizer/LogicalPlanOptimizerTests.java | 2 +- .../optimizer/PhysicalPlanOptimizerTests.java | 25 +++---- .../physical/local/PushTopNToSourceTests.java | 2 +- .../TestPhysicalOperationProviders.java | 2 +- .../xpack/esql/stats/DisabledSearchStats.java | 21 +++--- 20 files changed, 249 insertions(+), 137 deletions(-) create mode 100644 docs/changelog/128910.yaml diff --git a/docs/changelog/128910.yaml b/docs/changelog/128910.yaml new file mode 100644 index 0000000000000..e81a1b5996484 --- /dev/null +++ b/docs/changelog/128910.yaml @@ -0,0 +1,5 @@ +pr: 128910 +summary: Fix `FieldAttribute` name usage in `InferNonNullAggConstraint` +area: ES|QL +type: bug +issues: [] diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java index 576df0f393484..b6e10d1b63fc3 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java @@ -27,14 +27,24 @@ /** * Attribute for an ES field. - * To differentiate between the different type of fields this class offers: - * - name - the fully qualified name (foo.bar.tar) - * - path - the path pointing to the field name (foo.bar) - * - parent - the immediate parent of the field; useful for figuring out the type of field (nested vs object) - * - nestedParent - if nested, what's the parent (which might not be the immediate one) + * This class offers: + * - name - the name of the attribute, but not necessarily of the field. + * - The raw EsField representing the field; for parent.child.grandchild this is just grandchild. + * - parentName - the full path to the immediate parent of the field, e.g. parent.child (without .grandchild) + * + * To adequately represent e.g. union types, the name of the attribute can be altered because we may have multiple synthetic field + * attributes that really belong to the same underlying field. For instance, if a multi-typed field is used both as {@code field::string} + * and {@code field::ip}, we'll generate 2 field attributes called {@code $$field$converted_to$string} and {@code $$field$converted_to$ip} + * but still referring to the same underlying field. */ public class FieldAttribute extends TypedAttribute { + /** + * A field name, as found in the mapping. Includes the whole path from the root of the document. + * Implemented as a wrapper around {@link String} to distinguish from the attribute name (which sometimes differs!) at compile time. + */ + public record FieldName(String string) {}; + static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry( Attribute.class, "FieldAttribute", @@ -43,6 +53,7 @@ public class FieldAttribute extends TypedAttribute { private final String parentName; private final EsField field; + protected FieldName lazyFieldName; public FieldAttribute(Source source, String name, EsField field) { this(source, null, name, field); @@ -184,15 +195,19 @@ public String parentName() { /** * The full name of the field in the index, including all parent fields. E.g. {@code parent.subfield.this_field}. */ - public String fieldName() { - // Before 8.15, the field name was the same as the attribute's name. - // On later versions, the attribute can be renamed when creating synthetic attributes. - // Because until 8.15, we couldn't set `synthetic` to true due to a bug, in that version such FieldAttributes are marked by their - // name starting with `$$`. - if ((synthetic() || name().startsWith(SYNTHETIC_ATTRIBUTE_NAME_PREFIX)) == false) { - return name(); + public FieldName fieldName() { + if (lazyFieldName == null) { + // Before 8.15, the field name was the same as the attribute's name. + // On later versions, the attribute can be renamed when creating synthetic attributes. + // Because until 8.15, we couldn't set `synthetic` to true due to a bug, in that version such FieldAttributes are marked by + // their + // name starting with `$$`. + if ((synthetic() || name().startsWith(SYNTHETIC_ATTRIBUTE_NAME_PREFIX)) == false) { + lazyFieldName = new FieldName(name()); + } + lazyFieldName = new FieldName(Strings.hasText(parentName) ? parentName + "." + field.getName() : field.getName()); } - return Strings.hasText(parentName) ? parentName + "." + field.getName() : field.getName(); + return lazyFieldName; } public EsField.Exact getExactInfo() { diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java index 73e2d5ec626ac..3c94e9b20054e 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java @@ -116,7 +116,7 @@ public String getWriteableName() { } /** - * Returns the field path + * Returns the simple name, but not the full field path. The latter requires knowing the path of the parent field. */ public String getName() { return name; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java index 7823ed78f4723..4085476dc021c 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java @@ -50,6 +50,7 @@ import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute.FieldName; import org.elasticsearch.xpack.esql.core.expression.FoldContext; import org.elasticsearch.xpack.esql.core.expression.Literal; import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute; @@ -239,22 +240,22 @@ public static EsRelation relation() { public static class TestSearchStats implements SearchStats { @Override - public boolean exists(String field) { + public boolean exists(FieldName field) { return true; } @Override - public boolean isIndexed(String field) { + public boolean isIndexed(FieldName field) { return exists(field); } @Override - public boolean hasDocValues(String field) { + public boolean hasDocValues(FieldName field) { return exists(field); } @Override - public boolean hasExactSubfield(String field) { + public boolean hasExactSubfield(FieldName field) { return exists(field); } @@ -264,32 +265,32 @@ public long count() { } @Override - public long count(String field) { + public long count(FieldName field) { return exists(field) ? -1 : 0; } @Override - public long count(String field, BytesRef value) { + public long count(FieldName field, BytesRef value) { return exists(field) ? -1 : 0; } @Override - public byte[] min(String field, DataType dataType) { + public byte[] min(FieldName field, DataType dataType) { return null; } @Override - public byte[] max(String field, DataType dataType) { + public byte[] max(FieldName field, DataType dataType) { return null; } @Override - public boolean isSingleValue(String field) { + public boolean isSingleValue(FieldName field) { return false; } @Override - public boolean canUseEqualityOnSyntheticSourceDelegate(String name, String value) { + public boolean canUseEqualityOnSyntheticSourceDelegate(FieldName name, String value) { return false; } } @@ -340,23 +341,23 @@ private boolean isConfigationSet(Config config, String field) { } @Override - public boolean exists(String field) { - return isConfigationSet(Config.EXISTS, field); + public boolean exists(FieldName field) { + return isConfigationSet(Config.EXISTS, field.string()); } @Override - public boolean isIndexed(String field) { - return isConfigationSet(Config.INDEXED, field); + public boolean isIndexed(FieldName field) { + return isConfigationSet(Config.INDEXED, field.string()); } @Override - public boolean hasDocValues(String field) { - return isConfigationSet(Config.DOC_VALUES, field); + public boolean hasDocValues(FieldName field) { + return isConfigationSet(Config.DOC_VALUES, field.string()); } @Override - public boolean hasExactSubfield(String field) { - return isConfigationSet(Config.EXACT_SUBFIELD, field); + public boolean hasExactSubfield(FieldName field) { + return isConfigationSet(Config.EXACT_SUBFIELD, field.string()); } @Override @@ -474,8 +475,8 @@ private static SearchStats fieldMatchingExistOrMissing(boolean exists, String... private final Set fields = Set.of(names); @Override - public boolean exists(String field) { - return fields.contains(field) == exists; + public boolean exists(FieldName field) { + return fields.contains(field.string()) == exists; } }; } diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec index ce933bd6eba7f..7c89864989b08 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec @@ -1325,6 +1325,19 @@ count:long | message:keyword 3 | Connected to 10.1.0.3 ; +multiIndexStatsOfMultiTypedField +required_capability: union_types +required_capability: casting_operator +required_capability: union_types_numeric_widening + +FROM apps, apps_short +| STATS s = sum(id::integer) +; + +s:long +210 +; + multiIndexMultiColumnTypesRename required_capability: union_types required_capability: index_metadata_field diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index d0a479f476b6c..45759682e7884 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -1451,7 +1451,7 @@ private Expression resolveConvertFunction(AbstractConvertFunction convert, List< indexToConversionExpressions.put(indexName, newConvertFunction); } MultiTypeEsField multiTypeEsField = new MultiTypeEsField( - fa.fieldName(), + fa.fieldName().string(), convert.dataType(), false, indexToConversionExpressions diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/UnsupportedAttribute.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/UnsupportedAttribute.java index 945117032a5fd..af2c805f30e2c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/UnsupportedAttribute.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/UnsupportedAttribute.java @@ -123,10 +123,13 @@ public UnsupportedEsField field() { } @Override - public String fieldName() { - // The super fieldName uses parents to compute the path; this class ignores parents, so we need to rely on the name instead. - // Using field().getName() would be wrong: for subfields like parent.subfield that would return only the last part, subfield. - return name(); + public FieldName fieldName() { + if (lazyFieldName == null) { + // The super fieldName uses parents to compute the path; this class ignores parents, so we need to rely on the name instead. + // Using field().getName() would be wrong: for subfields like parent.subfield that would return only the last part, subfield. + lazyFieldName = new FieldName(name()); + } + return lazyFieldName; } @Override diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/InferNonNullAggConstraint.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/InferNonNullAggConstraint.java index bb818eb987021..1d6aa2d9ba1d1 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/InferNonNullAggConstraint.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/InferNonNullAggConstraint.java @@ -25,7 +25,7 @@ /** * The vast majority of aggs ignore null entries - this rule adds a pushable filter, as it is cheap - * to execute, to filter this entries out to begin with. + * to execute, to filter these entries out to begin with. * STATS x = min(a), y = sum(b) * becomes * | WHERE a IS NOT NULL OR b IS NOT NULL @@ -55,7 +55,7 @@ protected LogicalPlan rule(Aggregate aggregate, LocalLogicalOptimizerContext con Expression field = af.field(); // ignore literals (e.g. COUNT(1)) // make sure the field exists at the source and is indexed (not runtime) - if (field.foldable() == false && field instanceof FieldAttribute fa && stats.isIndexed(fa.name())) { + if (field.foldable() == false && field instanceof FieldAttribute fa && stats.isIndexed(fa.fieldName())) { nonNullAggFields.add(field); } else { // otherwise bail out since unless disjunction needs to cover _all_ fields, things get filtered out diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushdownPredicates.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushdownPredicates.java index 7843f8a6cfe04..a476086980534 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushdownPredicates.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushdownPredicates.java @@ -126,14 +126,16 @@ public boolean canUseEqualityOnSyntheticSourceDelegate(FieldAttribute attr, Stri }; /** - * If we have access to SearchStats over a collection of shards, we can make more fine-grained decisions about what can be pushed down. - * This should open up more opportunities for lucene pushdown. + * If we have access to {@link SearchStats} over a collection of shards, we can make more fine-grained decisions about what can be + * pushed down. This should open up more opportunities for lucene pushdown. */ static LucenePushdownPredicates from(SearchStats stats) { + // TODO: use FieldAttribute#fieldName, otherwise this doesn't apply to field attributes used for union types. + // C.f. https://github.com/elastic/elasticsearch/issues/128905 return new LucenePushdownPredicates() { @Override public boolean hasExactSubfield(FieldAttribute attr) { - return stats.hasExactSubfield(attr.name()); + return stats.hasExactSubfield(new FieldAttribute.FieldName(attr.name())); } @Override @@ -141,17 +143,19 @@ public boolean isIndexedAndHasDocValues(FieldAttribute attr) { // We still consider the value of isAggregatable here, because some fields like ScriptFieldTypes are always aggregatable // But this could hide issues with fields that are not indexed but are aggregatable // This is the original behaviour for ES|QL, but is it correct? - return attr.field().isAggregatable() || stats.isIndexed(attr.name()) && stats.hasDocValues(attr.name()); + return attr.field().isAggregatable() + || stats.isIndexed(new FieldAttribute.FieldName(attr.name())) + && stats.hasDocValues(new FieldAttribute.FieldName(attr.name())); } @Override public boolean isIndexed(FieldAttribute attr) { - return stats.isIndexed(attr.name()); + return stats.isIndexed(new FieldAttribute.FieldName(attr.name())); } @Override public boolean canUseEqualityOnSyntheticSourceDelegate(FieldAttribute attr, String value) { - return stats.canUseEqualityOnSyntheticSourceDelegate(attr.field().getName(), value); + return stats.canUseEqualityOnSyntheticSourceDelegate(attr.fieldName(), value); } }; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java index ed97ec5553fcc..0fff9e233e956 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java @@ -97,7 +97,7 @@ private Tuple, List> pushableStats( if (target instanceof FieldAttribute fa) { var fName = fa.fieldName(); if (context.searchStats().isSingleValue(fName)) { - fieldName = fName; + fieldName = fName.string(); query = QueryBuilders.existsQuery(fieldName); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java index 1e2fe9d47d7e4..37db3de5c954e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java @@ -143,7 +143,7 @@ public final PhysicalOperation fieldExtractPhysicalOperation(FieldExtractExec fi MappedFieldType.FieldExtractPreference fieldExtractPreference = fieldExtractExec.fieldExtractPreference(attr); ElementType elementType = PlannerUtils.toElementType(dataType, fieldExtractPreference); // Do not use the field attribute name, this can deviate from the field name for union types. - String fieldName = attr instanceof FieldAttribute fa ? fa.fieldName() : attr.name(); + String fieldName = attr instanceof FieldAttribute fa ? fa.fieldName().string() : attr.name(); boolean isUnsupported = dataType == DataType.UNSUPPORTED; IntFunction loader = s -> getBlockLoaderFor(s, fieldName, isUnsupported, fieldExtractPreference, unionTypes); fields.add(new ValuesSourceReaderOperator.FieldInfo(fieldName, elementType, loader)); @@ -274,7 +274,7 @@ public final Operator.OperatorFactory ordinalGroupingOperatorFactory( boolean isUnsupported = attrSource.dataType() == DataType.UNSUPPORTED; var unionTypes = findUnionTypes(attrSource); // Do not use the field attribute name, this can deviate from the field name for union types. - String fieldName = attrSource instanceof FieldAttribute fa ? fa.fieldName() : attrSource.name(); + String fieldName = attrSource instanceof FieldAttribute fa ? fa.fieldName().string() : attrSource.name(); return new OrdinalsGroupingOperator.OrdinalsGroupingOperatorFactory( shardIdx -> getBlockLoaderFor(shardIdx, fieldName, isUnsupported, NONE, unionTypes), vsShardContexts, diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchContextStats.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchContextStats.java index e7705496c0c7c..aeb5ec4b4bf4a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchContextStats.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchContextStats.java @@ -27,6 +27,8 @@ import org.elasticsearch.index.mapper.TextFieldMapper; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute.FieldName; import org.elasticsearch.xpack.esql.core.type.DataType; import java.io.IOException; @@ -80,8 +82,9 @@ private SearchContextStats(List contexts) { assert contexts != null && contexts.isEmpty() == false; } - public boolean exists(String field) { - var stat = cache.computeIfAbsent(field, this::makeFieldStats); + @Override + public boolean exists(FieldName field) { + var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats); return stat.config.exists; } @@ -123,21 +126,25 @@ private FieldConfig makeFieldConfig(String field) { } } - public boolean isIndexed(String field) { - var stat = cache.computeIfAbsent(field, this::makeFieldStats); + @Override + public boolean isIndexed(FieldName field) { + var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats); return stat.config.indexed; } - public boolean hasDocValues(String field) { - var stat = cache.computeIfAbsent(field, this::makeFieldStats); + @Override + public boolean hasDocValues(FieldName field) { + var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats); return stat.config.hasDocValues; } - public boolean hasExactSubfield(String field) { - var stat = cache.computeIfAbsent(field, this::makeFieldStats); + @Override + public boolean hasExactSubfield(FieldName field) { + var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats); return stat.config.hasExactSubfield; } + @Override public long count() { var count = new long[] { 0 }; boolean completed = doWithContexts(r -> { @@ -147,12 +154,13 @@ public long count() { return completed ? count[0] : -1; } - public long count(String field) { - var stat = cache.computeIfAbsent(field, this::makeFieldStats); + @Override + public long count(FieldName field) { + var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats); if (stat.count == null) { var count = new long[] { 0 }; boolean completed = doWithContexts(r -> { - count[0] += countEntries(r, field); + count[0] += countEntries(r, field.string()); return true; }, false); stat.count = completed ? count[0] : -1; @@ -160,9 +168,10 @@ public long count(String field) { return stat.count; } - public long count(String field, BytesRef value) { + @Override + public long count(FieldName field, BytesRef value) { var count = new long[] { 0 }; - Term term = new Term(field, value); + Term term = new Term(field.string(), value); boolean completed = doWithContexts(r -> { count[0] += r.docFreq(term); return true; @@ -170,12 +179,13 @@ public long count(String field, BytesRef value) { return completed ? count[0] : -1; } - public byte[] min(String field, DataType dataType) { - var stat = cache.computeIfAbsent(field, this::makeFieldStats); + @Override + public byte[] min(FieldName field, DataType dataType) { + var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats); if (stat.min == null) { var min = new byte[][] { null }; doWithContexts(r -> { - byte[] localMin = PointValues.getMinPackedValue(r, field); + byte[] localMin = PointValues.getMinPackedValue(r, field.string()); // TODO: how to compare with the previous min if (localMin != null) { if (min[0] == null) { @@ -192,12 +202,13 @@ public byte[] min(String field, DataType dataType) { return null; } - public byte[] max(String field, DataType dataType) { - var stat = cache.computeIfAbsent(field, this::makeFieldStats); + @Override + public byte[] max(FieldName field, DataType dataType) { + var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats); if (stat.max == null) { var max = new byte[][] { null }; doWithContexts(r -> { - byte[] localMax = PointValues.getMaxPackedValue(r, field); + byte[] localMax = PointValues.getMaxPackedValue(r, field.string()); // TODO: how to compare with the previous max if (localMax != null) { if (max[0] == null) { @@ -214,8 +225,10 @@ public byte[] max(String field, DataType dataType) { return null; } - public boolean isSingleValue(String field) { - var stat = cache.computeIfAbsent(field, this::makeFieldStats); + @Override + public boolean isSingleValue(FieldName field) { + String fieldName = field.string(); + var stat = cache.computeIfAbsent(fieldName, this::makeFieldStats); if (stat.singleValue == null) { // there's no such field so no need to worry about multi-value fields if (exists(field) == false) { @@ -224,11 +237,11 @@ public boolean isSingleValue(String field) { // fields are MV per default var sv = new boolean[] { false }; for (SearchExecutionContext context : contexts) { - MappedFieldType mappedType = context.isFieldMapped(field) ? context.getFieldType(field) : null; + MappedFieldType mappedType = context.isFieldMapped(fieldName) ? context.getFieldType(fieldName) : null; if (mappedType != null) { sv[0] = true; doWithContexts(r -> { - sv[0] &= detectSingleValue(r, mappedType, field); + sv[0] &= detectSingleValue(r, mappedType, fieldName); return sv[0]; }, true); break; @@ -288,9 +301,9 @@ private boolean detectSingleValue(IndexReader r, MappedFieldType fieldType, Stri } @Override - public boolean canUseEqualityOnSyntheticSourceDelegate(String name, String value) { + public boolean canUseEqualityOnSyntheticSourceDelegate(FieldAttribute.FieldName name, String value) { for (SearchExecutionContext ctx : contexts) { - MappedFieldType type = ctx.getFieldType(name); + MappedFieldType type = ctx.getFieldType(name.string()); if (type == null) { return false; } @@ -305,10 +318,11 @@ public boolean canUseEqualityOnSyntheticSourceDelegate(String name, String value return true; } - public String constantValue(String name) { + @Override + public String constantValue(FieldName name) { String val = null; for (SearchExecutionContext ctx : contexts) { - MappedFieldType f = ctx.getFieldType(name); + MappedFieldType f = ctx.getFieldType(name.string()); if (f == null) { return null; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchStats.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchStats.java index 748ed826836e7..ff1701104eca9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchStats.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchStats.java @@ -8,6 +8,8 @@ package org.elasticsearch.xpack.esql.stats; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute.FieldName; import org.elasticsearch.xpack.esql.core.type.DataType; /** @@ -17,33 +19,33 @@ public interface SearchStats { SearchStats EMPTY = new EmptySearchStats(); - boolean exists(String field); + boolean exists(FieldName field); - boolean isIndexed(String field); + boolean isIndexed(FieldName field); - boolean hasDocValues(String field); + boolean hasDocValues(FieldName field); - boolean hasExactSubfield(String field); + boolean hasExactSubfield(FieldName field); long count(); - long count(String field); + long count(FieldName field); - long count(String field, BytesRef value); + long count(FieldName field, BytesRef value); - byte[] min(String field, DataType dataType); + byte[] min(FieldName field, DataType dataType); - byte[] max(String field, DataType dataType); + byte[] max(FieldName field, DataType dataType); - boolean isSingleValue(String field); + boolean isSingleValue(FieldName field); - boolean canUseEqualityOnSyntheticSourceDelegate(String name, String value); + boolean canUseEqualityOnSyntheticSourceDelegate(FieldName name, String value); /** * Returns the value for a field if it's a constant (eg. a constant_keyword with only one value for the involved indices). * NULL if the field is not a constant. */ - default String constantValue(String name) { + default String constantValue(FieldAttribute.FieldName name) { return null; } @@ -53,22 +55,22 @@ default String constantValue(String name) { record EmptySearchStats() implements SearchStats { @Override - public boolean exists(String field) { + public boolean exists(FieldName field) { return false; } @Override - public boolean isIndexed(String field) { + public boolean isIndexed(FieldName field) { return false; } @Override - public boolean hasDocValues(String field) { + public boolean hasDocValues(FieldName field) { return false; } @Override - public boolean hasExactSubfield(String field) { + public boolean hasExactSubfield(FieldName field) { return false; } @@ -78,32 +80,32 @@ public long count() { } @Override - public long count(String field) { + public long count(FieldName field) { return 0; } @Override - public long count(String field, BytesRef value) { + public long count(FieldName field, BytesRef value) { return 0; } @Override - public byte[] min(String field, DataType dataType) { + public byte[] min(FieldName field, DataType dataType) { return null; } @Override - public byte[] max(String field, DataType dataType) { + public byte[] max(FieldName field, DataType dataType) { return null; } @Override - public boolean isSingleValue(String field) { + public boolean isSingleValue(FieldName field) { return true; } @Override - public boolean canUseEqualityOnSyntheticSourceDelegate(String name, String value) { + public boolean canUseEqualityOnSyntheticSourceDelegate(FieldName name, String value) { return false; } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java index b8321829b7689..c4369519d0956 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java @@ -28,6 +28,7 @@ import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.core.type.EsField; +import org.elasticsearch.xpack.esql.core.type.InvalidMappedField; import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; import org.elasticsearch.xpack.esql.expression.function.scalar.conditional.Case; import org.elasticsearch.xpack.esql.expression.function.scalar.nulls.Coalesce; @@ -41,6 +42,7 @@ import org.elasticsearch.xpack.esql.index.IndexResolution; import org.elasticsearch.xpack.esql.optimizer.rules.logical.local.InferIsNotNull; import org.elasticsearch.xpack.esql.parser.EsqlParser; +import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.EsRelation; import org.elasticsearch.xpack.esql.plan.logical.Eval; import org.elasticsearch.xpack.esql.plan.logical.Filter; @@ -60,6 +62,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import static java.util.Collections.emptyMap; import static org.elasticsearch.xpack.esql.EsqlTestUtils.L; @@ -636,7 +639,7 @@ public void testReplaceUpperStringCasinqgWithInsensitiveRLike() { var filter = as(limit.child(), Filter.class); var rlike = as(filter.condition(), RLike.class); var field = as(rlike.field(), FieldAttribute.class); - assertThat(field.fieldName(), is("first_name")); + assertThat(field.fieldName().string(), is("first_name")); assertThat(rlike.pattern().pattern(), is("VALÜ*")); assertThat(rlike.caseInsensitive(), is(true)); var source = as(filter.child(), EsRelation.class); @@ -650,7 +653,7 @@ public void testReplaceLowerStringCasingWithInsensitiveRLike() { var filter = as(limit.child(), Filter.class); var rlike = as(filter.condition(), RLike.class); var field = as(rlike.field(), FieldAttribute.class); - assertThat(field.fieldName(), is("first_name")); + assertThat(field.fieldName().string(), is("first_name")); assertThat(rlike.pattern().pattern(), is("valü*")); assertThat(rlike.caseInsensitive(), is(true)); var source = as(filter.child(), EsRelation.class); @@ -675,7 +678,7 @@ public void testReplaceUpperStringCasingWithInsensitiveLike() { var filter = as(limit.child(), Filter.class); var wlike = as(filter.condition(), WildcardLike.class); var field = as(wlike.field(), FieldAttribute.class); - assertThat(field.fieldName(), is("first_name")); + assertThat(field.fieldName().string(), is("first_name")); assertThat(wlike.pattern().pattern(), is("VALÜ*")); assertThat(wlike.caseInsensitive(), is(true)); var source = as(filter.child(), EsRelation.class); @@ -689,7 +692,7 @@ public void testReplaceLowerStringCasingWithInsensitiveLike() { var filter = as(limit.child(), Filter.class); var wlike = as(filter.condition(), WildcardLike.class); var field = as(wlike.field(), FieldAttribute.class); - assertThat(field.fieldName(), is("first_name")); + assertThat(field.fieldName().string(), is("first_name")); assertThat(wlike.pattern().pattern(), is("valü*")); assertThat(wlike.caseInsensitive(), is(true)); var source = as(filter.child(), EsRelation.class); @@ -706,6 +709,27 @@ public void testReplaceStringCasingAndLikeWithLocalRelation() { assertThat(local.supplier(), equalTo(LocalSupplier.EMPTY)); } + /** + * Limit[1000[INTEGER],false] + * \_Aggregate[[],[SUM($$integer_long_field$converted_to$long{f$}#5,true[BOOLEAN]) AS sum(integer_long_field::long)#3]] + * \_Filter[ISNOTNULL($$integer_long_field$converted_to$long{f$}#5)] + * \_EsRelation[test*][!integer_long_field, $$integer_long_field$converted..] + */ + public void testUnionTypesInferNonNullAggConstraint() { + LogicalPlan coordinatorOptimized = plan("FROM test* | STATS sum(integer_long_field::long)", analyzerWithUnionTypeMapping()); + var plan = localPlan(coordinatorOptimized, TEST_SEARCH_STATS); + + var limit = asLimit(plan, 1000); + var agg = as(limit.child(), Aggregate.class); + var filter = as(agg.child(), Filter.class); + var relation = as(filter.child(), EsRelation.class); + + var isNotNull = as(filter.condition(), IsNotNull.class); + var unionTypeField = as(isNotNull.field(), FieldAttribute.class); + assertEquals("$$integer_long_field$converted_to$long", unionTypeField.name()); + assertEquals("integer_long_field", unionTypeField.fieldName().string()); + } + private IsNotNull isNotNull(Expression field) { return new IsNotNull(EMPTY, field); } @@ -716,7 +740,7 @@ private LocalRelation asEmptyRelation(Object o) { return empty; } - private LogicalPlan plan(String query) { + private LogicalPlan plan(String query, Analyzer analyzer) { var analyzed = analyzer.analyze(parser.createStatement(query)); // System.out.println(analyzed); var optimized = logicalOptimizer.optimize(analyzed); @@ -724,6 +748,10 @@ private LogicalPlan plan(String query) { return optimized; } + private LogicalPlan plan(String query) { + return plan(query, analyzer); + } + private LogicalPlan localPlan(LogicalPlan plan, SearchStats searchStats) { var localContext = new LocalLogicalOptimizerContext(EsqlTestUtils.TEST_CFG, FoldContext.small(), searchStats); // System.out.println(plan); @@ -736,6 +764,31 @@ private LogicalPlan localPlan(String query) { return localPlan(plan(query), TEST_SEARCH_STATS); } + private static Analyzer analyzerWithUnionTypeMapping() { + InvalidMappedField unionTypeField = new InvalidMappedField( + "integer_long_field", + Map.of("integer", Set.of("test1"), "long", Set.of("test2")) + ); + + EsIndex test = new EsIndex( + "test*", + Map.of("integer_long_field", unionTypeField), + Map.of("test1", IndexMode.STANDARD, "test2", IndexMode.STANDARD) + ); + IndexResolution getIndexResult = IndexResolution.valid(test); + + return new Analyzer( + new AnalyzerContext( + EsqlTestUtils.TEST_CFG, + new EsqlFunctionRegistry(), + getIndexResult, + emptyPolicyResolution(), + emptyInferenceResolution() + ), + TEST_VERIFIER + ); + } + @Override protected List filteredWarnings() { return withDefaultLimitWarning(super.filteredWarnings()); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java index a1b5b994850c3..422a765b976c6 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java @@ -156,20 +156,20 @@ public class LocalPhysicalPlanOptimizerTests extends MapperServiceTestCase { private final Configuration config; private final SearchStats IS_SV_STATS = new TestSearchStats() { @Override - public boolean isSingleValue(String field) { + public boolean isSingleValue(FieldAttribute.FieldName field) { return true; } }; private final SearchStats CONSTANT_K_STATS = new TestSearchStats() { @Override - public boolean isSingleValue(String field) { + public boolean isSingleValue(FieldAttribute.FieldName field) { return true; } @Override - public String constantValue(String name) { - return name.startsWith("constant_keyword") ? "foo" : null; + public String constantValue(FieldAttribute.FieldName name) { + return name.string().startsWith("constant_keyword") ? "foo" : null; } }; @@ -533,8 +533,8 @@ public void testCountFieldsAndAllWithFilter() { public void testLocalAggOptimizedToLocalRelation() { var stats = new TestSearchStats() { @Override - public boolean exists(String field) { - return "emp_no".equals(field) == false; + public boolean exists(FieldAttribute.FieldName field) { + return "emp_no".equals(field.string()) == false; } }; @@ -1840,10 +1840,10 @@ public void testToDateNanosPushDown() { assertEquals(2, projections.size()); FieldAttribute fa = as(projections.get(0), FieldAttribute.class); assertEquals(DATE_NANOS, fa.dataType()); - assertEquals("date_and_date_nanos", fa.fieldName()); + assertEquals("date_and_date_nanos", fa.fieldName().string()); assertTrue(isMultiTypeEsField(fa)); // mixed date and date_nanos are auto-casted UnsupportedAttribute ua = as(projections.get(1), UnsupportedAttribute.class); // mixed date, date_nanos and long are not auto-casted - assertEquals("date_and_date_nanos_and_long", ua.fieldName()); + assertEquals("date_and_date_nanos_and_long", ua.fieldName().string()); var limit = as(project.child(), LimitExec.class); var exchange = as(limit.child(), ExchangeExec.class); project = as(exchange.child(), ProjectExec.class); @@ -1854,7 +1854,7 @@ public void testToDateNanosPushDown() { GreaterThanOrEqual gt = as(filter.condition(), GreaterThanOrEqual.class); fa = as(gt.left(), FieldAttribute.class); assertTrue(isMultiTypeEsField(fa)); - assertEquals("date_and_date_nanos_and_long", fa.fieldName()); + assertEquals("date_and_date_nanos_and_long", fa.fieldName().string()); fieldExtract = as(filter.child(), FieldExtractExec.class); // extract date_and_date_nanos_and_long var esQuery = as(fieldExtract.child(), EsQueryExec.class); var source = ((SingleValueQuery.Builder) esQuery.query()).source(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index b461d94432e2b..cc13a3fa0a34a 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -6224,7 +6224,7 @@ public void testReplaceStringCasingWithInsensitiveEqualsUnwrap() { var filter = as(limit.child(), Filter.class); var insensitive = as(filter.condition(), InsensitiveEquals.class); var field = as(insensitive.left(), FieldAttribute.class); - assertThat(field.fieldName(), is("first_name")); + assertThat(field.fieldName().string(), is("first_name")); var bRef = as(insensitive.right().fold(FoldContext.small()), BytesRef.class); assertThat(bRef.utf8ToString(), is("VALÜ")); as(filter.child(), EsRelation.class); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java index 02925fa6a4c80..8a599318e2773 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java @@ -985,12 +985,12 @@ public void testPushAndInequalitiesFilter() { var bq = as(source.query(), BoolQueryBuilder.class); assertThat(bq.must(), hasSize(2)); var first = as(sv(bq.must().get(0), "emp_no"), RangeQueryBuilder.class); - assertThat(first.fieldName(), equalTo("emp_no")); + assertThat(first.fieldName().toString(), equalTo("emp_no")); assertThat(first.from(), equalTo(-1)); assertThat(first.includeLower(), equalTo(false)); assertThat(first.to(), nullValue()); var second = as(sv(bq.must().get(1), "salary"), RangeQueryBuilder.class); - assertThat(second.fieldName(), equalTo("salary")); + assertThat(second.fieldName().toString(), equalTo("salary")); assertThat(second.from(), nullValue()); assertThat(second.to(), equalTo(10)); assertThat(second.includeUpper(), equalTo(false)); @@ -2439,7 +2439,7 @@ public void testPushDownEvalFilter() { assertThat(range2.fieldName(), is("first_name")); var sort = source.sorts(); assertThat(sort.size(), is(1)); - assertThat(sort.get(0).field().fieldName(), is("first_name")); + assertThat(sort.get(0).field().fieldName().string(), is("first_name")); } /** @@ -2498,7 +2498,7 @@ public void testPushDownEvalSwapFilter() { assertThat(exists.fieldName(), is("first_name")); var sort = source.sorts(); assertThat(sort.size(), is(1)); - assertThat(sort.get(0).field().fieldName(), is("last_name")); + assertThat(sort.get(0).field().fieldName().string(), is("last_name")); } /** @@ -3215,8 +3215,9 @@ public void testAggToLocalRelationOnDataNode() { """); var stats = new EsqlTestUtils.TestSearchStats() { - public boolean exists(String field) { - return "salary".equals(field); + @Override + public boolean exists(FieldAttribute.FieldName field) { + return "salary".equals(field.string()); } }; var optimized = optimizedPlan(plan, stats); @@ -5084,7 +5085,7 @@ public void testPushSpatialDistanceEvalToSource() { assertThat(alias.name(), is("distance")); var stDistance = as(alias.child(), StDistance.class); var location = as(stDistance.left(), FieldAttribute.class); - assertThat(location.fieldName(), is("location")); + assertThat(location.fieldName().string(), is("location")); // Validate the filter condition var and = as(filter.condition(), And.class); @@ -5138,7 +5139,7 @@ public void testPushSpatialDistanceMultiEvalToSource() { assertThat(alias2.name(), is("distance")); var stDistance = as(alias2.child(), StDistance.class); var location = as(stDistance.left(), FieldAttribute.class); - assertThat(location.fieldName(), is("location")); + assertThat(location.fieldName().string(), is("location")); var poiRef = as(stDistance.right(), Literal.class); assertThat(poiRef.value(), instanceOf(BytesRef.class)); assertThat(poiRef.value().toString(), is(poi.value().toString())); @@ -6494,7 +6495,7 @@ public void testPushCompoundTopNDistanceWithCompoundFilterAndCompoundEvalToSourc assertThat(alias2.name(), is("distance")); var stDistance = as(alias2.child(), StDistance.class); var location = as(stDistance.left(), FieldAttribute.class); - assertThat(location.fieldName(), is("location")); + assertThat(location.fieldName().string(), is("location")); var poiRef = as(stDistance.right(), Literal.class); assertThat(poiRef.value(), instanceOf(BytesRef.class)); assertThat(poiRef.value().toString(), is(poi.value().toString())); @@ -6680,7 +6681,7 @@ public void testPushCompoundTopNDistanceWithCompoundFilterAndNestedCompoundEvalT assertThat(alias2.name(), is("distance")); var stDistance = as(alias2.child(), StDistance.class); var location = as(stDistance.left(), FieldAttribute.class); - assertThat(location.fieldName(), is("location")); + assertThat(location.fieldName().string(), is("location")); var poiRef = as(stDistance.right(), Literal.class); assertThat(poiRef.value(), instanceOf(BytesRef.class)); assertThat(poiRef.value().toString(), is(poi.value().toString())); @@ -8234,12 +8235,12 @@ protected List filteredWarnings() { private static final SearchStats SEARCH_STATS_SHORT_DELEGATES = new EsqlTestUtils.TestSearchStats() { @Override - public boolean hasExactSubfield(String field) { + public boolean hasExactSubfield(FieldAttribute.FieldName field) { return false; } @Override - public boolean canUseEqualityOnSyntheticSourceDelegate(String name, String value) { + public boolean canUseEqualityOnSyntheticSourceDelegate(FieldAttribute.FieldName name, String value) { return value.length() < 4; } }; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushTopNToSourceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushTopNToSourceTests.java index 71f2215bb0f58..cacf6e422882b 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushTopNToSourceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushTopNToSourceTests.java @@ -451,7 +451,7 @@ private static void assertPushdownSort( String name = ((Attribute) expectedSorts.get(i).child()).name(); EsQueryExec.Sort sort = sorts.get(i); if (sort.field() != null) { - String fieldName = sort.field().fieldName(); + String fieldName = sort.field().fieldName().string(); assertThat("Expect sort[" + i + "] name to match", fieldName, is(sortName(name, fieldMap))); } assertThat("Expect sort[" + i + "] direction to match", sort.direction(), is(expectedSorts.get(i).direction())); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java index 1009eaea9b54c..959759b8b74c0 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java @@ -285,7 +285,7 @@ private Block getBlockForMultiType(DocBlock indexDoc, MultiTypeEsField multiType return nulls.get(); } var field = (FieldAttribute) conversion.field(); - return indexPage.columnIndex(field.fieldName()).isEmpty() + return indexPage.columnIndex(field.fieldName().string()).isEmpty() ? nulls.get() : TypeConverter.fromConvertFunction(conversion).convert(extractBlockForSingleDoc(indexDoc, field.fieldName(), blockCopier)); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/stats/DisabledSearchStats.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/stats/DisabledSearchStats.java index 60848c4ae5d00..6d8f5ca925121 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/stats/DisabledSearchStats.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/stats/DisabledSearchStats.java @@ -8,27 +8,28 @@ package org.elasticsearch.xpack.esql.stats; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute.FieldName; import org.elasticsearch.xpack.esql.core.type.DataType; public class DisabledSearchStats implements SearchStats { @Override - public boolean exists(String field) { + public boolean exists(FieldName field) { return true; } @Override - public boolean isIndexed(String field) { + public boolean isIndexed(FieldName field) { return true; } @Override - public boolean hasDocValues(String field) { + public boolean hasDocValues(FieldName field) { return true; } @Override - public boolean hasExactSubfield(String field) { + public boolean hasExactSubfield(FieldName field) { return true; } @@ -38,32 +39,32 @@ public long count() { } @Override - public long count(String field) { + public long count(FieldName field) { return -1; } @Override - public long count(String field, BytesRef value) { + public long count(FieldName field, BytesRef value) { return -1; } @Override - public byte[] min(String field, DataType dataType) { + public byte[] min(FieldName field, DataType dataType) { return null; } @Override - public byte[] max(String field, DataType dataType) { + public byte[] max(FieldName field, DataType dataType) { return null; } @Override - public boolean isSingleValue(String field) { + public boolean isSingleValue(FieldName field) { return false; } @Override - public boolean canUseEqualityOnSyntheticSourceDelegate(String name, String value) { + public boolean canUseEqualityOnSyntheticSourceDelegate(FieldName name, String value) { return false; } } From 6505101a1bab65a4f5b08b2fb8582d2c531798df Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 11 Jun 2025 18:05:54 +0200 Subject: [PATCH 2/3] Fix some more conflicts --- .../xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java | 3 +-- .../xpack/esql/planner/TestPhysicalOperationProviders.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java index c4369519d0956..58165756e94af 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java @@ -782,8 +782,7 @@ private static Analyzer analyzerWithUnionTypeMapping() { EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, - emptyPolicyResolution(), - emptyInferenceResolution() + EsqlTestUtils.emptyPolicyResolution() ), TEST_VERIFIER ); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java index 959759b8b74c0..404d31842bca9 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java @@ -287,7 +287,7 @@ private Block getBlockForMultiType(DocBlock indexDoc, MultiTypeEsField multiType var field = (FieldAttribute) conversion.field(); return indexPage.columnIndex(field.fieldName().string()).isEmpty() ? nulls.get() - : TypeConverter.fromConvertFunction(conversion).convert(extractBlockForSingleDoc(indexDoc, field.fieldName(), blockCopier)); + : TypeConverter.fromConvertFunction(conversion).convert(extractBlockForSingleDoc(indexDoc, field.fieldName().string(), blockCopier)); } private Block extractBlockForSingleDoc(DocBlock docBlock, String columnName, TestBlockCopier blockCopier) { From 13a3ac618c4d4d50dd1d514aadd482f739554355 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 11 Jun 2025 16:15:01 +0000 Subject: [PATCH 3/3] [CI] Auto commit changes from spotless --- .../esql/optimizer/LocalLogicalPlanOptimizerTests.java | 7 +------ .../xpack/esql/planner/TestPhysicalOperationProviders.java | 3 ++- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java index 58165756e94af..34cfac9605691 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java @@ -778,12 +778,7 @@ private static Analyzer analyzerWithUnionTypeMapping() { IndexResolution getIndexResult = IndexResolution.valid(test); return new Analyzer( - new AnalyzerContext( - EsqlTestUtils.TEST_CFG, - new EsqlFunctionRegistry(), - getIndexResult, - EsqlTestUtils.emptyPolicyResolution() - ), + new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, EsqlTestUtils.emptyPolicyResolution()), TEST_VERIFIER ); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java index 404d31842bca9..1517f17a87c49 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java @@ -287,7 +287,8 @@ private Block getBlockForMultiType(DocBlock indexDoc, MultiTypeEsField multiType var field = (FieldAttribute) conversion.field(); return indexPage.columnIndex(field.fieldName().string()).isEmpty() ? nulls.get() - : TypeConverter.fromConvertFunction(conversion).convert(extractBlockForSingleDoc(indexDoc, field.fieldName().string(), blockCopier)); + : TypeConverter.fromConvertFunction(conversion) + .convert(extractBlockForSingleDoc(indexDoc, field.fieldName().string(), blockCopier)); } private Block extractBlockForSingleDoc(DocBlock docBlock, String columnName, TestBlockCopier blockCopier) {