Skip to content

Conversation

@drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented Aug 1, 2025

Summary

Closes #223037

The main change here is changing the "timeInterval" AST item to be two types of new literal items that share mostly the same structure as the other literals and no longer require special handling during validation!

A few validation checks around these were also removed. More about that in the comments.

Checklist

evalExpectErrors(`from a_index | eval col0 = dateField - 1 ${unit.toUpperCase()}`, []);
evalExpectErrors(`from a_index | eval col0 = dateField - 1 ${capitalize(unit)}`, []);
evalExpectErrors(`from a_index | eval col0 = dateField + 1 ${unit}`, []);
evalExpectErrors(`from a_index | eval 1 ${unit} + 1 year`, []);
Copy link
Contributor Author

@drewdaemon drewdaemon Aug 1, 2025

Choose a reason for hiding this comment

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

This is cool... with the old logic, we were incorrectly allowing this case. But now that we've tightened up the typing, the validator correctly flagged it based on the + function definition

});
});

describe('time interval', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer a special case!

@drewdaemon drewdaemon added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana t// v9.2.0 labels Aug 1, 2025
@drewdaemon drewdaemon changed the title [ES|QL] treat time_duration and date_period correctly [ES|QL] treat time_duration and date_period separately Aug 1, 2025
@drewdaemon drewdaemon marked this pull request as ready for review August 2, 2025 02:37
@drewdaemon drewdaemon requested review from a team as code owners August 2, 2025 02:37
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

LGTM, code review only!

@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

@drewdaemon drewdaemon enabled auto-merge (squash) August 5, 2025 16:40
@drewdaemon drewdaemon merged commit d20a895 into elastic:main Aug 5, 2025
12 checks passed
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/esql-ast 526 521 -5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
esql 260.7KB 260.7KB +6.0B
onechat 707.6KB 706.9KB -663.0B
securitySolution 10.3MB 10.3MB -81.0B
total -738.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/esql-ast 37 39 +2

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-srcJs 3.8MB 3.8MB -1.3KB
Unknown metric groups

API count

id before after diff
@kbn/esql-ast 639 634 -5

History

delanni pushed a commit to delanni/kibana that referenced this pull request Aug 5, 2025
…30276)

## Summary

Closes elastic#223037

The main change here is changing the "timeInterval" AST item to be two
types of new literal items that share mostly the same structure as the
other literals and no longer require special handling during validation!

A few validation checks around these were also removed. More about that
in the comments.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@wildemat wildemat mentioned this pull request Aug 7, 2025
10 tasks
NicholasPeretti pushed a commit to NicholasPeretti/kibana that referenced this pull request Aug 18, 2025
…30276)

## Summary

Closes elastic#223037

The main change here is changing the "timeInterval" AST item to be two
types of new literal items that share mostly the same structure as the
other literals and no longer require special handling during validation!

A few validation checks around these were also removed. More about that
in the comments.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@drewdaemon drewdaemon deleted the 223037/remove-timespan-literal branch October 7, 2025 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:ES|QL ES|QL related features in Kibana release_note:skip Skip the PR/issue when compiling release notes Team:ESQL ES|QL related features in Kibana t// v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ES|QL] remove time_literal

4 participants