Skip to content

[ESQL] Implement missing methods of TimeSeriesAggregate #130218

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
Merged
merged 2 commits into from
Jun 27, 2025
Merged

[ESQL] Implement missing methods of TimeSeriesAggregate #130218

merged 2 commits into from
Jun 27, 2025

Conversation

@ivancea
Copy link
Contributor

@ivancea ivancea commented Jun 27, 2025 •
edited
Loading

The base methods expressionsResolved(), hashCode() and equals() weren't checking timeBucket

@ivancea ivancea requested a review from dnhatn June 27, 2025 14:20
@ivancea ivancea added >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.2.0 labels Jun 27, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jun 27, 2025

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

dnhatn
dnhatn approved these changes Jun 27, 2025
View reviewed changes
Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Ivan!

@ivancea ivancea enabled auto-merge (squash) June 27, 2025 14:22
@ivancea ivancea mentioned this pull request Jun 27, 2025
Draft
@ivancea ivancea merged commit 287d32d into elastic:main Jun 27, 2025
32 checks passed
@ivancea ivancea deleted the esql-fix-time-series-agg branch June 27, 2025 16:59
@ivancea ivancea mentioned this pull request Jun 30, 2025
Merged
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jul 3, 2025
@ivancea @mridula-s109
[ESQL] Implement missing methods of TimeSeriesAggregate (elastic#130218)
eb2b42a 
The base methods `expressionsResolved()`, `hashCode()` and `equals()` weren't checking `timeBucket`
ivancea added a commit that referenced this pull request Jul 7, 2025
@ivancea
ESQL: Separate Aggregate and TimeSeriesAggregate tests to cover timeB…
8f6ac25 
…ucket (#130327)

Continuation of #130218

The previous PR fixed a missing equals/hashCode. This PR adds the test that would have caught that.

Technically the test existed, but wasn't covering the `timeBucket` field
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dnhatn dnhatn dnhatn approved these changes

Assignees

No one assigned

Labels

:Analytics/ES|QL AKA ESQL >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ivancea @elasticsearchmachine @dnhatn