Skip to content

Conversation

not-napoleon
Copy link
Member

While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it.

It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now.

Copy link
Contributor

github-actions bot commented Oct 2, 2024

Documentation preview:

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Oct 2, 2024
@elasticsearchmachine
Copy link
Collaborator

Hi @not-napoleon, I've created a changelog YAML for you.

@not-napoleon not-napoleon requested a review from astefan October 2, 2024 15:55
@not-napoleon
Copy link
Member Author

@elasticmachine update branch

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM. I think we're ok not writing another CSV test for this.

I could see a world where we try to build CSV-style tests from our unit test scenarios. But that's for later i think.

@nik9000
Copy link
Member

nik9000 commented Oct 3, 2024

LGTM. I think we're ok not writing another CSV test for this.

You did write a CSV test and I scrolled by. Sorry!

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

@not-napoleon not-napoleon merged commit 60ae746 into elastic:main Oct 4, 2024
16 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

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

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

not-napoleon added a commit to not-napoleon/elasticsearch that referenced this pull request Oct 4, 2024
…astic#113961)

While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it.

It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now.

---------

Co-authored-by: Elastic Machine <[email protected]>
not-napoleon added a commit to not-napoleon/elasticsearch that referenced this pull request Oct 4, 2024
…astic#113961)

While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it.

It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now.

---------

Co-authored-by: Elastic Machine <[email protected]>
elasticsearchmachine pushed a commit that referenced this pull request Oct 4, 2024
…13961) (#114130)

While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it.

It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now.

---------

Co-authored-by: Elastic Machine <[email protected]>
elasticsearchmachine pushed a commit that referenced this pull request Oct 4, 2024
…13961) (#114138)

While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it.

It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now.

---------

Co-authored-by: Elastic Machine <[email protected]>
matthewabbott pushed a commit to matthewabbott/elasticsearch that referenced this pull request Oct 10, 2024
…astic#113961)

While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it.

It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now.

---------

Co-authored-by: Elastic Machine <[email protected]>
not-napoleon added a commit that referenced this pull request Oct 22, 2024
Resolves #109998

For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in #113961. I've added CSV tests and unit tests for all the functions listed in the original ticket.


---------

Co-authored-by: Elastic Machine <[email protected]>
not-napoleon added a commit to not-napoleon/elasticsearch that referenced this pull request Oct 22, 2024
…114056)

Resolves elastic#109998

For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in elastic#113961. I've added CSV tests and unit tests for all the functions listed in the original ticket.


---------

Co-authored-by: Elastic Machine <[email protected]>
elasticsearchmachine pushed a commit that referenced this pull request Oct 24, 2024
…14056) (#115351)

* [ESQL] Support date_nanos on functions that take "any" type (#114056)

Resolves #109998

For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in #113961. I've added CSV tests and unit tests for all the functions listed in the original ticket.


---------

Co-authored-by: Elastic Machine <[email protected]>

* Mute failing watcher test

Cherry-pick
f8e931d#diff-41386766c394f14f5f205f92bb26eb1420b80af0057c78b2842fcc7ddd3d67aaR326

For whatever reason, git cherry-pick is having some difficulty with
this, so I just hand copied the mute.

* pull in another mute

---------

Co-authored-by: Elastic Machine <[email protected]>
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Oct 25, 2024
…114056)

Resolves elastic#109998

For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in elastic#113961. I've added CSV tests and unit tests for all the functions listed in the original ticket.


---------

Co-authored-by: Elastic Machine <[email protected]>
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Nov 4, 2024
…114056)

Resolves elastic#109998

For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in elastic#113961. I've added CSV tests and unit tests for all the functions listed in the original ticket.


---------

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL backport pending >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.15.3 v8.16.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants