Skip to content

Conversation

@MattAlp
Copy link
Contributor

@MattAlp MattAlp commented Oct 23, 2025

Addresses #131232 by checking the path provided for a reverse_nested agg against the parent's aggregation path & throwing an IllegalArgumentException if appropriate.

@MattAlp MattAlp added >bug :Analytics/Aggregations Aggregations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) auto-backport Automatically create backport pull requests when merged labels Oct 23, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

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.

Looks good to me. @martijnvg should also look because he knows the nested stuff better than anyone.

- match: { aggregations.courses.highpass_filter.unnest.department.buckets.1.doc_count: 1 }
---
"Illegal reverse nested aggregation to a child nested object":
- do:
Copy link
Member

Choose a reason for hiding this comment

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

This'll also want a capability. Those live in SearchCapabilities. And, of course, it's not an enum.

@nik9000 nik9000 requested a review from martijnvg October 24, 2025 14:58
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

It has been a long time since I looked at this code. However so far I can this LGTM.

@MattAlp MattAlp enabled auto-merge (squash) October 28, 2025 19:59
@MattAlp MattAlp merged commit b46f245 into elastic:main Oct 29, 2025
34 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

The backport operation could not be completed due to the following error:

There are no branches to backport to. Aborting.

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

@MattAlp
Copy link
Contributor Author

MattAlp commented Oct 30, 2025

💚 All backports created successfully

Status Branch Result
9.2

Questions ?

Please refer to the Backport tool documentation

MattAlp added a commit to MattAlp/elasticsearch that referenced this pull request Oct 30, 2025
MattAlp added a commit that referenced this pull request Oct 31, 2025
MattAlp added a commit to MattAlp/elasticsearch that referenced this pull request Oct 31, 2025
@MattAlp
Copy link
Contributor Author

MattAlp commented Oct 31, 2025

💚 All backports created successfully

Status Branch Result
8.19

Questions ?

Please refer to the Backport tool documentation

MattAlp added a commit that referenced this pull request Oct 31, 2025
* Reject invalid `reverse_nested` aggs (#137047)

(cherry picked from commit b46f245)
chrisparrinello pushed a commit to chrisparrinello/elasticsearch that referenced this pull request Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations auto-backport Automatically create backport pull requests when merged backport pending >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants