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.

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

[BUG] The .order() method of aggregation does not work with composite aggregations #957

Closed
SavvasUmangKamdar opened this issue Apr 24, 2024 · 6 comments · Fixed by #967
Closed
Labels
bug Something isn't working untriaged

Comments

@SavvasUmangKamdar
Copy link

What is the bug?

The .order() method of aggregation expects input parameters of Map<> or List type only which throws error while doing the composite aggregation.

How can one reproduce the bug?

Aggregation aggregation = new Aggregation.Builder().terms(aggField -> aggField.field("my_name.keyword").order(Map.of("_key", SortOrder.Desc))).build();

This will throw an error when used with composite aggregation. Error:
[x_content_parse_exception] [1:125] [composite] failed to parse field [sources]
with this root cause
[1:137] [terms] order doesn't support values of type: START_ARRAY

What is the expected behavior?

I think the .order() method should just accept string values too

What is your host/environment?

Using opensearch-java library in my spring boot application

Do you have any additional context?

Pls refer this stackoverflow link for detailed question:
Link

@SavvasUmangKamdar SavvasUmangKamdar added bug Something isn't working untriaged labels Apr 24, 2024
@consulthys
Copy link

consulthys commented Apr 24, 2024

The issue might stem from the fact that the CompositeAggregationSource class expects a TermsAggregation when terms is specified in a composite source, i.e. the same TermsAggregation structure that is used when specifying standalone terms aggregations. TermsAggregation has a different way of supplying the sort order, i.e using List<Map<String, SortOrder>>

On the server-side, the TermsValuesSourceBuilder class expects a simple SortOrder value

@SavvasUmangKamdar
Copy link
Author

Hey @consulthys, thanks for replying. It would be helpful if you have any solution on how to handle this case?

@consulthys
Copy link

@SavvasUmangKamdar It's a bug in the OS Java client that needs to be fixed. TermsAggregation needs to be specialized to be used in a CompositeAggregationSource

@dblock
Copy link
Member

dblock commented Apr 24, 2024

@SavvasUmangKamdar want to give it a try? Maybe start by writing a (failing) test for it?

@SavvasUmangKamdar
Copy link
Author

@dblock sorry but am completely new to opensearch, so not much aware of this!

@dblock
Copy link
Member

dblock commented Apr 24, 2024

@dblock sorry but am completely new to opensearch, so not much aware of this!

Totally. Give it a shot though, start with https://github.com/opensearch-project/opensearch-java/blob/main/DEVELOPER_GUIDE.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working untriaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants