Skip to content

ES|QL: fix folding of case() function with date period and time duration#141157

Merged
luigidellaquila merged 9 commits intoelastic:mainfrom
luigidellaquila:esql/fix_case_temporal
Jan 26, 2026
Merged

ES|QL: fix folding of case() function with date period and time duration#141157
luigidellaquila merged 9 commits intoelastic:mainfrom
luigidellaquila:esql/fix_case_temporal

Conversation

@luigidellaquila
Copy link
Contributor

Temporal amounts (date_period, time_duration) don't have an associated ElementType (yet), so we can't use evaluators to fold case() when they are involved. We have to do it manually.

Fixes: #138415

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@ivancea ivancea left a comment

Choose a reason for hiding this comment

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

Thanks!

@Override
public Object fold(FoldContext ctx) {
DataType type = dataType();
if (type == DataType.DATE_PERIOD || type == DataType.TIME_DURATION) {
Copy link
Contributor

Choose a reason for hiding this comment

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

At this point, do we need this if? This fold looks generic enough. I'm not much into having custom fold()s if the Evaluator one works, but this one here is required anyway, so I would go with it (Still, leaving the comment so we don't remove it in the future)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a valid point, let me simplify it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately the "custom" approach loses all the warnings, I'll have to fall back to the evaluator as much as possible for now (until I find a way to replicate the same warnings here)

@luigidellaquila luigidellaquila enabled auto-merge (squash) January 23, 2026 12:20
@luigidellaquila luigidellaquila merged commit 1b89494 into elastic:main Jan 26, 2026
35 checks passed
luigidellaquila added a commit to luigidellaquila/elasticsearch that referenced this pull request Jan 27, 2026
luigidellaquila added a commit to luigidellaquila/elasticsearch that referenced this pull request Jan 27, 2026
luigidellaquila added a commit to luigidellaquila/elasticsearch that referenced this pull request Jan 27, 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 >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.11 v9.1.11 v9.2.5 v9.3.1 v9.4.0

4 participants