Skip to content

ESQL: Unmute nullify tests with STATS#140554

Merged
elasticsearchmachine merged 4 commits intoelastic:mainfrom
alex-spies:un-ignore-nullify-stats-tests
Jan 13, 2026
Merged

ESQL: Unmute nullify tests with STATS#140554
elasticsearchmachine merged 4 commits intoelastic:mainfrom
alex-spies:un-ignore-nullify-stats-tests

Conversation

@alex-spies
Copy link
Contributor

@alex-spies alex-spies commented Jan 13, 2026

Relates #138888 and #139797

@alex-spies alex-spies added >test Issues or PRs that are addressing/adding tests auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL v9.3.0 v9.4.0 labels Jan 13, 2026
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jan 13, 2026
@elasticsearchmachine
Copy link
Collaborator

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

@alex-spies alex-spies added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 13, 2026
;

s:long
s:double
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it become double? Looks like x in row could be cast to int or long

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that foo doesn't exist; x is unused.
About why double, I suspect it's a purely technical thing related with this condition:

@Override
public DataType dataType() {
DataType dt = field().dataType();
return dt.isWholeNumber() == false || dt == UNSIGNED_LONG ? DOUBLE : LONG;
}

dt.isWholeNumber() == false -> double. Alex may confirm what is the field type here. If it's NULL, and as NULL isn't a whole number, double it is!

Copy link
Contributor Author

@alex-spies alex-spies Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent question @idegtiarenko .

Ivan's answer is correct, although that boils down to "because we said so" when we implemented SUM.

I'll sleep better after double checking this, but currently this is consistent with ROW x = null | STATS SUM(x).

For sleeping better, I opened #140577.

@elasticsearchmachine elasticsearchmachine merged commit f73e4cc into elastic:main Jan 13, 2026
35 checks passed
@alex-spies alex-spies deleted the un-ignore-nullify-stats-tests branch January 13, 2026 17:26
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
9.3 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 140554

@alex-spies
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
9.3

Questions ?

Please refer to the Backport tool documentation

alex-spies added a commit to alex-spies/elasticsearch that referenced this pull request Jan 14, 2026
spinscale pushed a commit to spinscale/elasticsearch that referenced this pull request Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport pending Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v9.3.0 v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants