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

Regression on AggregationRange - incorrect types for from and to fields #2641

Open
pgayvallet opened this issue Jun 24, 2024 · 1 comment · May be fixed by #2725
Open

Regression on AggregationRange - incorrect types for from and to fields #2641

pgayvallet opened this issue Jun 24, 2024 · 1 comment · May be fixed by #2725
Labels
wrong type

Comments

@pgayvallet
Copy link
Contributor

pgayvallet commented Jun 24, 2024 •
edited
Loading

🐛 Wrong type

AggregationRange.from and AggregationRange.to were changed from string | number to number in #2552

From the PR:

AggregationRange accepting both double and string was probably established because at some point it was also used for RangeQuery, which accepts Objects for parameters to and from. But in the spec we just use it in GeoDistanceAggregation and RangeAggregation, both accepting only doubles. Since the java client generalizes string | double as just string, this results in the server throwing exceptions when called.

This was supposed to be done to address elastic/elasticsearch-java#666, but the assumption that was used for the PR is wrong, For RangeAggrations at least, from and to do support string type value, for patterns such as now or now+X.

E.g from the Kibana repo:

https://github.com/elastic/kibana/blob/1c1e20afdb883fc279c5a06d4b71f8e577f8dad6/x-pack/plugins/task_manager/server/monitoring/workload_statistics.ts#L169-L180
@pgayvallet pgayvallet changed the title Regression on RangeAggregation - incorrect types for from and to fields Regression on AggregationRange - incorrect types for from and to fields Jun 24, 2024
@flobernd
Copy link
Member

I think this type follows the same semantics as RangeQuery which means that it should accept arbitrary typed values for the from and to fields.

The corresponding RangeAggregate looks suspicious to me as well. We should double check the server code here as well.

@l-trotta l-trotta linked a pull request Jul 16, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
wrong type
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

making range aggregation untagged union elastic/elasticsearch-specification
2 participants
@pgayvallet @flobernd