Skip to content

Add cartesian_bounds, cartesian_centroid and change_point aggregations #5418

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 4 commits into from
Oct 7, 2025
Merged

Add cartesian_bounds, cartesian_centroid and change_point aggregations #5418

merged 4 commits into from
Oct 7, 2025

Conversation

l-trotta
Copy link
Contributor

@l-trotta l-trotta commented Oct 6, 2025

These should all be safe to backport.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 6, 2025 •
edited
Loading

Following you can find the validation changes against the target branch for the APIs.

API Status Request Response
indices.create 🔴 1347/1377 → 1352/1377 1377/1377
indices.put_mapping 🔴 → 🟢 139/145 → 145/145 148/148
reindex 🔴 → 🟢 20/21 → 21/21 20/20
search 🔴 2551/2608 → 2569/2608 2608/2608

You can validate these APIs yourself by using the make validate target.

pquentin
pquentin reviewed Oct 7, 2025
View reviewed changes
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Great work, thank you! I especially like the careful specification for the IMO tricky change point aggregation.

specification/_types/aggregations/AggregationContainer.ts
cardinality?: CardinalityAggregation
/**
* A metric aggregation that computes the spatial bounding box containing all values for a Point or Shape field.
* @ext_doc_id search-aggregations-metrics-cartesian-bounds-aggregation
Copy link
Member

Choose a reason for hiding this comment

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

I believe you need an entry in specification/_doc_ids/table.csv for ext_doc_id to fully work. This also applies to change_point and cartesian_centroid.

specification/_types/aggregations/Aggregate.ts
export class NonStationary {
p_value: double
r_value: double
trend: string
Copy link
Member

Choose a reason for hiding this comment

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

We could decide to make this an enum with increasing and decreasing values: https://github.com/elastic/elasticsearch/blob/414972d5c7f65fec11c9ac2b4f6c0baaf456abdc/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/ChangeDetector.java#L482

Copy link
Contributor Author

@l-trotta l-trotta Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

in this case I was referring to https://github.com/elastic/elasticsearch/blob/f9c72c17d18859e3511bd17fbff759ebe965b812/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/ChangeType.java#L254, which is what's used by the response

Copy link
Member

Choose a reason for hiding this comment

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

Right, I noticed trend is a string in the server code, but in practice, there are only two values. Well, it's an implementation detail, so it's likely better not to rely on it. You're right.

l-trotta and others added 2 commits October 7, 2025 10:43
@pquentin pquentin changed the title Missing aggregations Add cartesian_bounds, cartesian_centroid and change_point aggregations Oct 7, 2025
pquentin
pquentin approved these changes Oct 7, 2025
View reviewed changes
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@pquentin pquentin merged commit 23222d9 into main Oct 7, 2025
8 checks passed
@pquentin pquentin deleted the missing-aggregations branch October 7, 2025 09:23
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 7, 2025

The backport to 8.19 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-8.19 8.19
# Navigate to the new working tree
cd .worktrees/backport-8.19
# Create a new branch
git switch --create backport-5418-to-8.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 23222d914f80f9858ad508ca6fb5b8749955d7bf
# Push it to GitHub
git push --set-upstream origin backport-5418-to-8.19
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.19

Then, create a pull request where the base branch is 8.19 and the compare/head branch is backport-5418-to-8.19.

github-actions bot pushed a commit that referenced this pull request Oct 7, 2025
@l-trotta @pquentin @github-actions
Add cartesian_bounds, cartesian_centroid and change_point aggregations (
0c48cc3 
#5418)

* fix search validation

* add variants to changetype

* Update specification/_types/aggregations/AggregationContainer.ts

Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>

* add ext docs to table csv

---------

Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
(cherry picked from commit 23222d9)
@github-actions github-actions bot mentioned this pull request Oct 7, 2025
Merged
github-actions bot pushed a commit that referenced this pull request Oct 7, 2025
@l-trotta @pquentin @github-actions
Add cartesian_bounds, cartesian_centroid and change_point aggregations (
4ccdd5b 
#5418)

* fix search validation

* add variants to changetype

* Update specification/_types/aggregations/AggregationContainer.ts

Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>

* add ext docs to table csv

---------

Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
(cherry picked from commit 23222d9)
@github-actions github-actions bot mentioned this pull request Oct 7, 2025
Merged
github-actions bot pushed a commit that referenced this pull request Oct 7, 2025
@l-trotta @pquentin @github-actions
Add cartesian_bounds, cartesian_centroid and change_point aggregations (
76c9b79 
#5418)

* fix search validation

* add variants to changetype

* Update specification/_types/aggregations/AggregationContainer.ts

Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>

* add ext docs to table csv

---------

Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
(cherry picked from commit 23222d9)
@github-actions github-actions bot mentioned this pull request Oct 7, 2025
Merged
l-trotta added a commit that referenced this pull request Oct 7, 2025
@github-actions @l-trotta @pquentin
Add cartesian_bounds, cartesian_centroid and change_point aggregations (
4348d09 
#5418) (#5438)

* fix search validation

* add variants to changetype

* Update specification/_types/aggregations/AggregationContainer.ts



* add ext docs to table csv

---------


(cherry picked from commit 23222d9)

Co-authored-by: Laura Trotta <153528055+l-trotta@users.noreply.github.com>
Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
l-trotta added a commit that referenced this pull request Oct 7, 2025
@github-actions @l-trotta @pquentin
Add cartesian_bounds, cartesian_centroid and change_point aggregations (
a41a045 
#5418) (#5437)

* fix search validation

* add variants to changetype

* Update specification/_types/aggregations/AggregationContainer.ts



* add ext docs to table csv

---------


(cherry picked from commit 23222d9)

Co-authored-by: Laura Trotta <153528055+l-trotta@users.noreply.github.com>
Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
l-trotta added a commit that referenced this pull request Oct 7, 2025
@github-actions @l-trotta @pquentin
Add cartesian_bounds, cartesian_centroid and change_point aggregations (
231d2ab 
#5418) (#5436)

* fix search validation

* add variants to changetype

* Update specification/_types/aggregations/AggregationContainer.ts



* add ext docs to table csv

---------


(cherry picked from commit 23222d9)

Co-authored-by: Laura Trotta <153528055+l-trotta@users.noreply.github.com>
Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
l-trotta added a commit that referenced this pull request Oct 7, 2025
@l-trotta @pquentin
Add cartesian_bounds, cartesian_centroid and change_point aggregations (
96e1bfe 
#5418) (#5436)

* fix search validation

* add variants to changetype

* Update specification/_types/aggregations/AggregationContainer.ts

* add ext docs to table csv

---------

(cherry picked from commit 23222d9)

Co-authored-by: Laura Trotta <153528055+l-trotta@users.noreply.github.com>
Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
@l-trotta l-trotta mentioned this pull request Oct 7, 2025
Merged
l-trotta added a commit that referenced this pull request Oct 7, 2025
@l-trotta @pquentin
[Backport 8.19] Add cartesian_bounds, cartesian_centroid and change_p…
617466c 
…oint aggregations (#5439)

* Add cartesian_bounds, cartesian_centroid and change_point aggregations (#5418) (#5436)

* fix search validation

* add variants to changetype

* Update specification/_types/aggregations/AggregationContainer.ts

* add ext docs to table csv

---------

(cherry picked from commit 23222d9)

Co-authored-by: Laura Trotta <153528055+l-trotta@users.noreply.github.com>
Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>

* update ext doc table

---------

Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
@l-trotta l-trotta mentioned this pull request Oct 7, 2025
Merged
@l-trotta
Copy link
Contributor Author

l-trotta commented Oct 7, 2025

manually backported 8.19 #5439

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pquentin pquentin pquentin approved these changes

Assignees

No one assigned

Labels

backport 8.19 backport 9.0 backport 9.1 backport 9.2 specification

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@l-trotta @pquentin