ESQL: Replace "representable" type error messages by ivancea · Pull Request #131775 · elastic/elasticsearch

ivancea · 2025-07-23T15:25:54Z

Continuation of #131694 (Issue: #112006)

Moved more messages from "representable" to "any type but counter types".
Added missing docs on Count
Added unsigned long and missing docs on Sample and Values
Fixed Sample throwing error 500 on non-representable types

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvMax.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvMin.java

…e their error messages

elasticsearchmachine · 2025-07-23T15:26:20Z

Hi @ivancea, I've created a changelog YAML for you.

ivancea · 2025-07-23T15:26:41Z

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

+    /**
+     * @see DataType#isRepresentable(DataType)
+     */
+    public static TypeResolution isRepresentableExceptCounters(Expression e, String operationName, ParamOrdinal paramOrd) {


Added some helpers to centralize the logic and messages here

ivancea · 2025-07-23T15:27:06Z

...gin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java

+                "cartesian_shape",
                "date",
+                "date_nanos",
                "double",
                "geo_point",
+                "geo_shape",


Missing types for the docs

Huh. I thought the tests would catch that.

Mostly. We check that every type in tests appears there, but if it isn't tested, we do nothing. And Count didn't have "error types" tests to check that we tested all the types

ivancea · 2025-07-23T15:28:00Z

...in/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sample.java

            "ip",
            "keyword",
            "long",
+            "unsigned_long",


This trivially allowed unsigned longs, so just allowed it

I mostly copied this from Values, assuming that was correct

ivancea · 2025-07-23T15:28:14Z

...in/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Values.java

    private static final Map<DataType, Supplier<AggregatorFunctionSupplier>> SUPPLIERS = Map.ofEntries(
        Map.entry(DataType.INTEGER, ValuesIntAggregatorFunctionSupplier::new),
        Map.entry(DataType.LONG, ValuesLongAggregatorFunctionSupplier::new),
+        Map.entry(DataType.UNSIGNED_LONG, ValuesLongAggregatorFunctionSupplier::new),


This trivially allowed unsigned longs, so just allowed it

ivancea · 2025-07-23T15:36:23Z

...in/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sample.java

            return new TypeResolution("Unresolved children");
        }
-        var typeResolution = isType(field(), dt -> dt != DataType.UNSIGNED_LONG, sourceText(), FIRST, "any type except unsigned_long").and(
+        var typeResolution = isRepresentableExceptCounters(field(), sourceText(), FIRST).and(


This was technically allowing counters before, but I'm not sure we actually wanted it (?)

Also, it allowed things like date period, which led to illegal data type [date_period] 500 errors

ivancea · 2025-07-23T15:39:42Z

.../src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvMax.java

-            "version",
-            "numeric except counter types"
-        );
+        return isRepresentableExceptCountersAndSpatial(field(), sourceText(), DEFAULT);


I'm changing this list of most types to "any but counters and spatial", which should be enough here

ivancea · 2025-07-23T15:41:08Z

...src/test/java/org/elasticsearch/xpack/esql/expression/function/MultiRowTestCaseSupplier.java


    private MultiRowTestCaseSupplier() {}

+    public static List<TypedDataSupplier> nullCases(int minRows, int maxRows) {


Pretty weird, but I needed it to make type tests work for Count

elasticsearchmachine · 2025-07-23T16:30:03Z

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

jan-elastic

LGTM on the changes to SAMPLE

I'll leave reviewing the remainder to someone more knowledgeable about the internal data types.

astefan

LGTM

nik9000 · 2025-07-24T13:32:19Z

...gin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java

+                "cartesian_shape",
                "date",
+                "date_nanos",
                "double",
                "geo_point",
+                "geo_shape",


Huh. I thought the tests would catch that.

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sample.java

ivancea and others added 12 commits July 22, 2025 15:46

ESQL: Accept unsigned longs on MAX and MIN aggregations

6a4b727

Update docs/changelog/131694.yaml

9a12405

Update tests

3cab2c1

Updated Max/MinOverTime and fixed error tests

cd63166

Merge branch 'main' into max-min-unsigned-long

e5a0a78

ESQL: Simplify representable types error message

b9ff56f

Renamed isPresentable to be more specific with counter types

48cf5de

Format

82e84b1

Update Count, Sample and Values to accept unsigned_long, and normaliz…

9a5c860

…e their error messages

Update Count types docs and tests

861f108

Added and updated error tests for Sample, Values and Count

6886dff

ivancea added >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.2.0 labels Jul 23, 2025

Update docs/changelog/131775.yaml

df624be

ivancea commented Jul 23, 2025

View reviewed changes

Added csv tests and capability for sample and values on unsigned longs

12ccac0

ivancea requested review from astefan, idegtiarenko and jan-elastic July 23, 2025 16:29

ivancea marked this pull request as ready for review July 23, 2025 16:29

jan-elastic reviewed Jul 24, 2025

View reviewed changes

ivancea and others added 3 commits July 24, 2025 12:06

Updated MvMax and MvMin error tests

dffe6a6

Merge branch 'main' into esql-types-representable-message

a8c7040

Update CountOverTime docs

896daad

astefan approved these changes Jul 24, 2025

View reviewed changes

nik9000 approved these changes Jul 24, 2025

View reviewed changes

ivancea enabled auto-merge (squash) July 24, 2025 13:36

ivancea and others added 3 commits July 24, 2025 16:29

Merge branch 'main' into esql-types-representable-message

9e9161e

Merge branch 'main' into esql-types-representable-message

28229a4

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sample.java

Fixed test

f5233ac

ivancea merged commit 8ab6751 into elastic:main Jul 28, 2025
33 checks passed

ivancea deleted the esql-types-representable-message branch July 28, 2025 13:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESQL: Replace "representable" type error messages#131775

ESQL: Replace "representable" type error messages#131775
ivancea merged 20 commits intoelastic:mainfrom
ivancea:esql-types-representable-message

ivancea commented Jul 23, 2025 •

edited

Loading

elasticsearchmachine commented Jul 23, 2025

ivancea Jul 23, 2025

ivancea Jul 23, 2025

nik9000 Jul 24, 2025

ivancea Jul 24, 2025

ivancea Jul 23, 2025

jan-elastic Jul 24, 2025

ivancea Jul 23, 2025

ivancea Jul 23, 2025

jan-elastic Jul 24, 2025

ivancea Jul 23, 2025

ivancea Jul 23, 2025

elasticsearchmachine commented Jul 23, 2025

jan-elastic left a comment

astefan left a comment

nik9000 Jul 24, 2025

Uh oh!

Labels

5 participants


		private MultiRowTestCaseSupplier() {}

		public static List<TypedDataSupplier> nullCases(int minRows, int maxRows) {

Conversation

ivancea commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Jul 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticsearchmachine commented Jul 23, 2025

jan-elastic left a comment

Choose a reason for hiding this comment

astefan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Labels

5 participants

ivancea commented Jul 23, 2025 •

edited

Loading