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 0632f21436c2a..832745f7cb15e 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); @@ -136,15 +147,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 321c79ee13a83..6a46cb979e105 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 @@ -117,7 +117,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 ebada01e72b41..7b5843704eac8 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 @@ -52,6 +52,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; @@ -248,22 +249,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); } @@ -273,32 +274,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; } } @@ -349,23 +350,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 @@ -500,8 +501,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 a4fdb108e639d..7faab1493096a 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 @@ -1744,7 +1744,7 @@ private Expression resolveConvertFunction(ConvertFunction convert, List, 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 1c2bbe36db9ea..0b2fafcf2df2e 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 @@ -152,7 +152,7 @@ public final PhysicalOperation fieldExtractPhysicalOperation(FieldExtractExec fi private static String getFieldName(Attribute attr) { // Do not use the field attribute name, this can deviate from the field name for union types. - return attr instanceof FieldAttribute fa ? fa.fieldName() : attr.name(); + return attr instanceof FieldAttribute fa ? fa.fieldName().string() : attr.name(); } private BlockLoader getBlockLoaderFor(int shardId, Attribute attr, MappedFieldType.FieldExtractPreference fieldExtractPreference) { 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 8cb10d14bb578..c9766bad1c602 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; @@ -126,21 +128,25 @@ private boolean fastNoCacheFieldExists(String field) { return false; } - public boolean exists(String field) { - var stat = cache.get(field); - return stat != null ? stat.config.exists : fastNoCacheFieldExists(field); + @Override + public boolean exists(FieldName field) { + var stat = cache.get(field.string()); + return stat != null ? stat.config.exists : fastNoCacheFieldExists(field.string()); } - public boolean isIndexed(String field) { - return cache.computeIfAbsent(field, this::makeFieldStats).config.indexed; + @Override + public boolean isIndexed(FieldName field) { + return cache.computeIfAbsent(field.string(), this::makeFieldStats).config.indexed; } - public boolean hasDocValues(String field) { - return cache.computeIfAbsent(field, this::makeFieldStats).config.hasDocValues; + @Override + public boolean hasDocValues(FieldName field) { + return cache.computeIfAbsent(field.string(), this::makeFieldStats).config.hasDocValues; } - public boolean hasExactSubfield(String field) { - return cache.computeIfAbsent(field, this::makeFieldStats).config.hasExactSubfield; + @Override + public boolean hasExactSubfield(FieldName field) { + return cache.computeIfAbsent(field.string(), this::makeFieldStats).config.hasExactSubfield; } public long count() { @@ -152,12 +158,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; @@ -165,9 +172,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; @@ -175,12 +183,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) { @@ -197,12 +206,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) { @@ -219,8 +229,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 (stat.config.exists == false) { @@ -229,11 +241,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; @@ -293,9 +305,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; } 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 0901ff3d15a91..8251611dcfe47 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; @@ -650,7 +653,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); @@ -664,7 +667,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); @@ -689,7 +692,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); @@ -703,7 +706,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); @@ -720,6 +723,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); } @@ -730,7 +754,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); @@ -738,6 +762,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); @@ -750,6 +778,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 b0f16b384a489..047d5a54976d8 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 @@ -165,20 +165,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; } }; @@ -565,8 +565,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; } }; @@ -2024,10 +2024,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); @@ -2038,7 +2038,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 926fdd75cac6a..6d66a187e6168 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 @@ -6341,7 +6341,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 cbca9b3eaae88..26a2de5b0c8dc 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 @@ -988,12 +988,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)); @@ -2442,7 +2442,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")); } /** @@ -2501,7 +2501,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")); } /** @@ -3223,8 +3223,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); @@ -5092,7 +5093,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); @@ -5146,7 +5147,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())); @@ -6502,7 +6503,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())); @@ -6688,7 +6689,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())); @@ -8282,12 +8283,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 2e3d6482440dd..d5aa1af7feec2 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 @@ -328,7 +328,7 @@ private Block getBlockForMultiType(DocBlock indexDoc, MultiTypeEsField multiType if (conversion == null) { return getNullsBlock(indexDoc); } - return switch (extractBlockForSingleDoc(indexDoc, ((FieldAttribute) conversion.field()).fieldName(), blockCopier)) { + return switch (extractBlockForSingleDoc(indexDoc, ((FieldAttribute) conversion.field()).fieldName().string(), blockCopier)) { case BlockResultMissing unused -> getNullsBlock(indexDoc); case BlockResultSuccess success -> TypeConverter.fromConvertFunction(conversion).convert(success.block); }; 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; } }