Esql dimension aware attributes #131463

not-napoleon · 2025-07-17T15:48:40Z

Relates to #128621

This PR adds more building blocks for working with dimensions in ESQL. Specifically, it makes the TSDB metadata available on attributes.

I did not add new tests for this yet. That's on purpose, as we have more components we need before we can really wire this up the way we want to.

Conflicts: docs/reference/query-languages/esql/kibana/docs/operators/is_null.md

Conflicts: x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java

not-napoleon · 2025-07-22T15:31:49Z

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java

@@ -83,13 +123,8 @@ public static Reader<? extends EsField> getReader(String name) {
    private final Map<String, EsField> properties;
    private final String name;
    private final boolean isAlias;
-    // Because the subclasses all reimplement serialization, this needs to be writeable from subclass constructors


(Comment was outdated - I ended up not reimplementing the serialization in the sub classes, but forgot to delete this in the previous PR)

not-napoleon · 2025-07-22T17:05:17Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java

+                    timeSeriesFieldType = timeSeriesFieldType.merge(EsField.TimeSeriesFieldType.fromIndexFieldCapabilities(fc));
+                } catch (IllegalArgumentException e) {
+                    return new InvalidMappedField(name, e.getMessage());
+                }


This is the core thing this PR does. As we are loading the fields, we collect their time series metadata, and merge it across indices. This is then made available to the field attribute.

not-napoleon · 2025-07-22T17:17:47Z

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/ClusterRequestTests.java

@@ -165,7 +165,6 @@ public void testFallbackIndicesOptions() throws Exception {
        assertThat(cloned.clusterAlias(), equalTo(request.clusterAlias()));
        assertThat(cloned.sessionId(), equalTo(request.sessionId()));
        RemoteClusterPlan plan = cloned.remoteClusterPlan();
-        assertThat(plan.plan(), equalTo(request.remoteClusterPlan().plan()));


Test was failing on this line because plans serialized with older transport versions are not equal to plans serialized with the new transport version. This is expected, since when translating from the old version, we default the ts metadata to unknown, while the current version serializes and deserializes an actual value.

I discussed this failure with @dnhatn , and we concluded this assert wasn't adding any value in this test.

elasticsearchmachine · 2025-07-22T17:18:28Z

Pinging @elastic/es-analytical-engine (Team:Analytics)

elasticsearchmachine · 2025-07-22T17:18:29Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

dnhatn

I have one question, but the changes look good. I look forward to seeing the next part. I think we might use this information only on the coordinator; hence I'm not sure if we need to serialize it, but I'm okay with this approach as well.

dnhatn · 2025-07-24T18:12:59Z

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java

+                if (other != METRIC) {
+                    return DIMENSION;
+                }
+                throw new IllegalStateException("Time Series Metadata conflict.  Cannot merge [" + other + "] with [DIMENSION].");


To confirm, we do not allow mixing dimensions and metrics, but we do allow NONE with dimensions. I think the GAUGE type is optional - should we treat it similarly to NONE?

My main concern here is allowing for a migration from a label to a dimension. Labels would have neither a metric nor a dimension type, and would show up as NONE here, but if a later mapping marks them as dimensions, we should pick that up.

alex-spies

Heya, small drive-by review because I saw that we're making changes to the Attribute hierarchy. This part of the change looks good to me, but I left a small number of thoughts. Didn't review the rest.

alex-spies · 2025-07-28T12:47:12Z

...sql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java

@@ -107,6 +107,11 @@ protected String label() {
        return UNRESOLVED_PREFIX;
    }

+    @Override
+    public boolean isDimension() {
+        return false;


thought: maybe this should throw UOE, because we can't determine this on an unresolved attr.

alex-spies · 2025-07-28T12:49:19Z

...esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/ReferenceAttribute.java

+
+    @Override
+    public boolean isDimension() {
+        return false;


thought: it might get tricky when dealing with renames; if you have a FROM idx | RENAME dimension_field AS foo, then the reference attribute for foo will claim not to be a dimension - so downstream plan nodes may need to resolve aliases to correctly determine if they're looking at a dimension or not.

My understanding from @kkrik-es is that, at least for the initial version, that's an acceptable restriction. I think ReferenceAttributes are used for the output of EVAL and similar commands, and at least for the initial version of this we do not want to treat those as dimensions.

alex-spies · 2025-07-28T12:49:41Z

...k/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java

+    /**
+     * @return true if the attribute represents a TSDB dimension type
+     */
+    public abstract boolean isDimension();


I wonder if this should actually be ternary, resp. return Boolean (capital B) so that attributes can return null to say "I don't know if I refer to a dimension" rather than claiming not to.

Otherwise 👍

What would be the behavior associated with the null version? The intention here is that we want to only allow filtering by dimensions, and enable automatic grouping by all dimensions. If a field returns null here, we would then need to decide if "null" behaved as "this is a dimension" or "this is not a dimension" for these cases.

Unless you're proposing that we fail (or null-with-warning) any query where there is ambiguity about the dimensions? I'm not sure I agree with that.

dnhatn

LGTM. Let's get this in. Thanks Mark!

Relates to elastic#128621 This PR adds more building blocks for working with dimensions in ESQL. Specifically, it makes the TSDB metadata available on attributes.

not-napoleon added 18 commits May 7, 2025 09:58

abstract function

05a4561

Merge branch 'main' into esql-dimension-aware-attributes

5f8e350

Merge branch 'main' into esql-dimension-aware-attributes

0c0b5bd

stub in field attribute

9d79b2f

Merge branch 'main' into esql-dimension-aware-attributes

2f9e7d2

begin wiring up the time series types

c11fa12

stub in abstract methods

28a3aea

explicit metadata type for some benchmarks

001047d

explicit metadata type

3397960

stuff

038f743

Merge branch 'main' into esql-dimension-aware-attributes

d726c75

Conflicts: docs/reference/query-languages/esql/kibana/docs/operators/is_null.md

explicit default for a bunch of constructors

37849ed

fix the other constructor

cf48733

multi-type fields

f246efa

text-field subclass

3703533

keyword fields

47df3b4

date fields

5250dce

equals and hashcode

c7dfe13

not-napoleon added >non-issue :StorageEngine/TSDB You know, for Metrics :Analytics/ES|QL AKA ESQL v9.2.0 labels Jul 17, 2025

not-napoleon added 4 commits July 17, 2025 12:53

fix a couple of tests

ec5dfd4

Remove unnecessary check

2f638e7

Merge branch 'main' into esql-dimension-aware-attributes

1a16702

Conflicts: x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java

revert unexpected docs changes

e7b7e7c

not-napoleon commented Jul 22, 2025

View reviewed changes

not-napoleon marked this pull request as ready for review July 22, 2025 17:17

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 22, 2025

elasticsearchmachine added the Team:StorageEngine label Jul 22, 2025

not-napoleon requested a review from dnhatn July 23, 2025 18:45

Merge branch 'main' into esql-dimension-aware-attributes

1b1afe5

dnhatn reviewed Jul 24, 2025

View reviewed changes

alex-spies reviewed Jul 28, 2025

View reviewed changes

not-napoleon added 2 commits August 14, 2025 12:45

Merge branch 'main' into esql-dimension-aware-attributes

89c2054

Merge branch 'main' into esql-dimension-aware-attributes

3b4dc17

dnhatn approved these changes Aug 15, 2025

View reviewed changes

not-napoleon enabled auto-merge (squash) August 15, 2025 17:05

Merge branch 'main' into esql-dimension-aware-attributes

9ab5c0f

not-napoleon merged commit b53d5f7 into elastic:main Aug 18, 2025
34 checks passed

Esql dimension aware attributes #131463

Esql dimension aware attributes #131463

Uh oh!

Conversation

not-napoleon commented Jul 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Jul 22, 2025

Uh oh!

elasticsearchmachine commented Jul 22, 2025

Uh oh!

dnhatn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-spies left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnhatn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!