Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ES|QL: Support ::date in inline cast #123460

Merged
merged 9 commits into from
Mar 11, 2025
Merged

ES|QL: Support ::date in inline cast #123460

merged 9 commits into from
Mar 11, 2025

Conversation

kanoshiou
Copy link
Contributor

Closes #116746

@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.1.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Feb 26, 2025
@kingherc kingherc added the :Analytics/ES|QL AKA ESQL label Mar 4, 2025
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed needs:triage Requires assignment of a team area label labels Mar 4, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@costin costin requested a review from fang-xing-esql March 5, 2025 05:30
@fang-xing-esql fang-xing-esql self-assigned this Mar 6, 2025
@fang-xing-esql
Copy link
Member

buildkite test this

@fang-xing-esql fang-xing-esql added auto-backport Automatically create backport pull requests when merged ES|QL-ui Impacts ES|QL UI labels Mar 6, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/kibana-esql (ES|QL-ui)

fang-xing-esql
fang-xing-esql reviewed Mar 6, 2025
View reviewed changes
Copy link
Member

@fang-xing-esql fang-xing-esql left a comment

Choose a reason for hiding this comment

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

Thank you for contributing to Elasticsearch @kanoshiou ! I added some comments around the new tests.

x-pack/plugin/esql/qa/testFixtures/src/main/resources/convert.csv-spec Outdated
@@ -74,6 +74,15 @@ ROW date="1985-01-01T00:00:00Z"::datetime, zero=0::datetime
1985-01-01T00:00:00.000Z|1970-01-01T00:00:00.000Z
;

convertToDate
required_capability: casting_operator
Copy link
Member

Choose a reason for hiding this comment

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

We'll need a new entry in EsqlCapabilities for ::date's tests, as the older versions don't recognize ::date.

And can we add more tests for ::date? A date field can appear in a lot of places of a query, there are some queries with ::datetime used with + and - can be used as references, it can be used in eval, where etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially thought it was sufficient to test only the basic casting functionality, as ::date utilizes the same function as ::datetime.

However, you are absolutely right—there’s no guarantee that this situation will remain unchanged forever. Therefore, adding additional tests is essential.

@kanoshiou kanoshiou requested a review from fang-xing-esql March 7, 2025 14:27
fang-xing-esql
fang-xing-esql reviewed Mar 11, 2025
View reviewed changes
Copy link
Member

@fang-xing-esql fang-xing-esql left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for addressing the comments @kanoshiou! It looks pretty good to me.

@kanoshiou
Copy link
Contributor Author

Thank you for reviewing @fang-xing-esql! I have updated the test, please take another look when you have the time.

@fang-xing-esql
Copy link
Member

buildkite test this

@fang-xing-esql
Copy link
Member

buildkite test this

fang-xing-esql
fang-xing-esql approved these changes Mar 11, 2025
View reviewed changes
Copy link
Member

@fang-xing-esql fang-xing-esql left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @kanoshiou!

@fang-xing-esql fang-xing-esql merged commit deff3df into elastic:main Mar 11, 2025
19 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
❌ 8.x Commit could not be cherrypicked due to conflicts

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

@fang-xing-esql fang-xing-esql mentioned this pull request Mar 11, 2025
Merged
@fang-xing-esql
Copy link
Member

💚 All backports created successfully

Status Branch Result
✅ 8.x

Questions ?

Please refer to the Backport tool documentation

elasticsearchmachine pushed a commit that referenced this pull request Mar 11, 2025
@fang-xing-esql @kanoshiou
ES|QL: Support ::date in inline cast (#123460) (#124605)
42400f4 
* Inline cast to date

* Update docs/changelog/123460.yaml

* New capability for `::date` casting

* More tests

* Update tests

---------

Co-authored-by: Fang Xing <155562079+fang-xing-esql@users.noreply.github.com>
(cherry picked from commit deff3df)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

Co-authored-by: kanoshiou <73424326+kanoshiou@users.noreply.github.com>
albertzaharovits pushed a commit to albertzaharovits/elasticsearch that referenced this pull request Mar 13, 2025
@kanoshiou @fang-xing-esql @albertzaharovits
ES|QL: Support ::date in inline cast (elastic#123460)
1d0c23f 
* Inline cast to date

* Update docs/changelog/123460.yaml

* New capability for `::date` casting

* More tests

* Update tests

---------

Co-authored-by: Fang Xing <155562079+fang-xing-esql@users.noreply.github.com>
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Mar 13, 2025
@kanoshiou @fang-xing-esql @jfreden
ES|QL: Support ::date in inline cast (elastic#123460)
2ee88b8 
* Inline cast to date

* Update docs/changelog/123460.yaml

* New capability for `::date` casting

* More tests

* Update tests

---------

Co-authored-by: Fang Xing <155562079+fang-xing-esql@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fang-xing-esql fang-xing-esql fang-xing-esql approved these changes

Assignees

@fang-xing-esql fang-xing-esql

Labels
:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged backport pending >enhancement ES|QL-ui Impacts ES|QL UI external-contributor Pull request authored by a developer outside the Elasticsearch team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ES|QL] Support ::date in inline cast
4 participants
@kanoshiou @elasticsearchmachine @fang-xing-esql @kingherc