ESLQ: Make functions behave well under NULL and narrower types by alex-spies · Pull Request #142657 · elastic/elasticsearch

alex-spies · 2026-02-18T17:18:04Z

Closes #142537 if we decide this is the right way to go.

SET unmapped_fields="nullify" makes it so that any function/operator that can take an ES field as input, can now also be passed a NULL-typed reference attribute. Practically, that happens by injecting an ... | EVAL field = NULL | ... at the beginning of the query. The intent is that queries will still be valid and run fine even if a field is nullified because it's missing from the mapping.

Well, that only works if our functions/operators respect that. Most of them implicitly do, but others deviate in weird ways.

The crux here is that we implicitly have a widening hierarchy between types and NULL is the bottom type that can be widened to any other type.

So, we should expect that we can take any valid function/operator expression and replace any* of its arguments by NULL or a narrower type and still have a valid expression (contra-variance). To not break other expressions that consume the function/operator, the new output type must at most have become narrower or stayed the same, but never widened (co-variance).

Most of our functions already respect that. Let's enforce this with an added test.

*there are 2 exceptions:

arguments that must be non-NULL constants, like the percentage in PERCENTILE
arguments that must either have the same type as other arguments or be NULL, like in COALESCE

TODO:

Add concept of widening to DataType
Add test that runs on all functions/operators to enforce co- and contra-variance
Add javadocs with the 3 rules for functions (see ESQL: consider better behavior of NULL type in expressions #142537 (comment)), e.g. as javadoc for Expression#dataType().
mute tests for any inconsistent functions/operators for now (to be fixed in separate follow-up PRs)
open more specific GH issues for deviations before merging, see ESQL: consider better behavior of NULL type in expressions #142537 (comment).

This was created with help from Cursor (Opus 4.6 (Thinking)).

alex-spies · 2026-02-19T08:35:32Z

Ok, a bunch of functions don't satisfy co-/contra-variance, yet:

Update: Went through all of them. Most are just missing support for some narrowings; tracked the exact deviations here.

CASE, GREATES, LEAST need to have matching output branches. Narrowing only needs to be supported to NULL, not in general.

Some functions want consistent types across several input args. We should still have co+contravariance when narrowing all inputs that have to have the same type. CASE and several others have an additional constraint, which is that even in case of the required uniformity, KEYWORD arguments can still be narrowed to TEXT.

…-contravariance

alex-spies · 2026-02-20T13:03:05Z

muted-tests.yml

  issue: https://github.com/elastic/elasticsearch/issues/141234
+- class: org.elasticsearch.xpack.esql.expression.function.aggregate.SumTests
+  method: testCoAndContraVariance*
+  issue: https://github.com/elastic/elasticsearch/issues/142537


Before we merge this, I plan to open more specific issues and point the mutes to them.

elasticsearchmachine · 2026-02-20T16:14:03Z

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

astefan · 2026-02-23T11:54:13Z

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

+        }
+
+        Set<DataType> nonTrivial = NARROWER_TYPES_MAP.getOrDefault(this, Set.of());
+        return Stream.concat(nonTrivial.stream(), Stream.of(NULL)).collect(Collectors.toUnmodifiableSet());


Imho, the use of Streams here is overkill.

Set<DataType> nonTrivial = NARROWER_TYPES_MAP.getOrDefault(this, Set.of()); nonTrivial.add(NULL); return Collections.unmodifiableSet(nonTrivial);

Also, this Set is created every time the method is called. Can we get away with some static Sets for each type instead?

astefan · 2026-02-23T11:55:26Z

...java/org/elasticsearch/xpack/esql/expression/function/aggregate/PercentileOverTimeTests.java


+    @Override
+    protected void filterCoAndContraVarianceNarrowing(Map<Integer, DataType> positionNarrowing, List<TestCaseSupplier.TypedData> data) {
+        positionNarrowing.entrySet().removeIf(e -> e.getKey() > 0 && e.getValue() == DataType.NULL);


Extract the content of removeIf in a separate method and, also, re-use that in PercentileTests.

astefan · 2026-02-23T11:55:55Z

...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/SampleTests.java


+    @Override
+    protected void filterCoAndContraVarianceNarrowing(Map<Integer, DataType> positionNarrowing, List<TestCaseSupplier.TypedData> data) {
+        positionNarrowing.entrySet().removeIf(e -> e.getKey() == 1 && e.getValue() == DataType.NULL);


Same here about reusing the common code inside removeIf (common with what's in Percentile that is).

astefan · 2026-02-23T12:03:37Z

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

        NAME_OR_ALIAS_TO_TYPE = Collections.unmodifiableMap(map);
    }

+    /**


I think it would help to document here somewhere why some other numerics are not considered in WIDENING_TO, like unsigned_long -> double, float -> double, byte, geo data types (if even the conceptual widening makes sense).

astefan · 2026-02-23T12:09:43Z

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

+     * Randomly narrows one or more input types using the candidates returned by {@code narrowerTypes},
+     * choosing independently for each argument position.
+     */
+    private void checkCoAndContraVariance(java.util.function.Function<DataType, Set<DataType>> narrowerTypes) {


I am wondering why a random approach is used here. Are there so many combinations that we can't test all of them?

I think it's indeed quite a few tests. The function tests have many cases and take 1-2min each on my machine, I didn't dare to try and go through all combinations.

For a function that takes 2 double arguments, being exhaustive would mean that we need to test an additional 15 cases (double->long->int->NULL in 2 positions). Since there are so many test cases for most functions, the randomized approach seems sufficient; and when a function didn't satisfy the condition, I saw test failures quite consistently.

astefan · 2026-02-23T12:15:42Z

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

+        // We allow TEXT to be used where KEYWORD is expected because we load text fields without analysis, so they behave like keywords.
+        // This is only expected to be relevant for fields that are mapped as both text and keyword,
+        // but it is simpler to allow this in general than to special case it just for those fields.
+        TEXT,


It's surprising to me to see the "widening" term being used for non-numerics as well. I don't know enough about how these days TEXT and KEYWORD are handled and what their particularities are in the context of ESQL (a field can be indexed and not stored and vice-versa, source not stored, doc_values enabled, text analyzed, keyword normalized etc.) If you are really certain there are no fishy scenarios about these two interchangeably, it's ok with me, but I am not confident enough on my knowledge about these two not to add this comment here :-)

astefan · 2026-02-23T12:19:02Z

...in/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java

+         * org.elasticsearch.xpack.esql.core.expression.FieldAttribute}, which is more appropriate when the value doesn't
+         * necessarily correspond to an Elasticsearch field.
+         */
+        public Expression asReference() {


Why does it matter if this is a ReferenceAttribute or FieldAttribute? Actually, why our tests make (now) a distinction between the two? Is this relevant to the test itself?

elasticsearchmachine added the v9.4.0 label Feb 18, 2026

Add+test concept of co- and contra-variance

224846b

alex-spies force-pushed the fix-function-co-and-contravariance branch from 702e53e to 224846b Compare February 18, 2026 17:19

alex-spies mentioned this pull request Feb 19, 2026

ESQL: autocasting to long missing from first/last #142583

Closed

Add 2 tests that narrow using only/never NULL

dd87800

alex-spies mentioned this pull request Feb 19, 2026

ESQL: consider better behavior of NULL type in expressions #142537

Open

alex-spies added 2 commits February 19, 2026 12:28

Improve error message

8200194

Start muting tests for non-compliant functions

9d78807

alex-spies added the :Analytics/ES|QL AKA ESQL label Feb 19, 2026

alex-spies and others added 17 commits February 19, 2026 13:23

Fix Add/Sub tests

145c2a0

Exclude branch f'ns from non-NULL narrowing tests

784a995

CASE, GREATES, LEAST need to have matching output branches. Narrowing only needs to be supported to NULL, not in general.

Mute some more

82d4ad1

Skip more tests

af03438

Consider an additional widening TEXT->KEYWORD

e75a0f0

Skip more tests

6dfc239

Fix some mutes

5b6c8a1

More mutes

5f506d6

Respect isForceLiteral

6506be2

Fix PercentileTests

392ca5d

More mutes

353d767

Improve error message when the expression doesn't even resolve

8b1003e

More fixes

a1b46ab

More mutes

8929a13

[CI] Auto commit changes from spotless

41cd140

Merge remote-tracking branch 'upstream/main' into fix-function-co-and…

a65b418

…-contravariance

alex-spies commented Feb 20, 2026

View reviewed changes

alex-spies added 2 commits February 20, 2026 14:17

Fix TopTests

9f659c6

Fix PercentileOverTimeTests

3a90390

alex-spies and others added 9 commits February 20, 2026 14:34

Fix DateParseTests

9f8371d

Fix some more tests

9f6b419

Mute co-/contravar tests for to_dense_vector

d988227

Fix more tests

c9fd324

[CI] Auto commit changes from spotless

4ba29bc

Fix TopSnippetsTests

1b5d70d

Merge branch 'main' into fix-function-co-and-contravariance

cac6dc1

[CI] Auto commit changes from spotless

7f530dc

Improve javadoc

b7ff9ca

alex-spies marked this pull request as ready for review February 20, 2026 16:13

alex-spies requested review from astefan and fang-xing-esql February 20, 2026 16:13

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 20, 2026

alex-spies requested a review from quackaplop February 20, 2026 16:31

alex-spies added the >non-issue label Feb 20, 2026

astefan reviewed Feb 23, 2026

View reviewed changes

alex-spies mentioned this pull request Feb 23, 2026

ESQL: change point value must be numeric for a NULL data type #142858

Closed

alex-spies mentioned this pull request Mar 16, 2026

ES|QL: Implicit numeric widening for union types #144288

Draft

alex-spies mentioned this pull request Mar 26, 2026

ESQL: Correctly manage NULL data type for SUM #144942

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESLQ: Make functions behave well under NULL and narrower types#142657

ESLQ: Make functions behave well under NULL and narrower types#142657
alex-spies wants to merge 32 commits intoelastic:mainfrom
alex-spies:fix-function-co-and-contravariance

alex-spies commented Feb 18, 2026 •

edited

Loading

Uh oh!

alex-spies commented Feb 19, 2026 •

edited

Loading

Uh oh!

alex-spies Feb 20, 2026

Uh oh!

elasticsearchmachine commented Feb 20, 2026

Uh oh!

astefan Feb 23, 2026

Uh oh!

astefan Feb 23, 2026

Uh oh!

astefan Feb 23, 2026

Uh oh!

astefan Feb 23, 2026

Uh oh!

astefan Feb 23, 2026

Uh oh!

astefan Feb 23, 2026

Uh oh!

alex-spies Feb 23, 2026

Uh oh!

astefan Feb 23, 2026

Uh oh!

astefan Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

alex-spies commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alex-spies commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Feb 20, 2026

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alex-spies commented Feb 18, 2026 •

edited

Loading

alex-spies commented Feb 19, 2026 •

edited

Loading