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

TopHitsAggregationBuilder adds duplicate _score sort clauses #42154

Closed
soinio opened this issue May 15, 2019 · 2 comments · Fixed by #42179
Closed

TopHitsAggregationBuilder adds duplicate _score sort clauses #42154

soinio opened this issue May 15, 2019 · 2 comments · Fixed by #42179

Comments

@soinio
Copy link

soinio commented May 15, 2019

Elasticsearch version 6.5.4, also current master

Description of the problem:

When using High Level Rest Client Java API to produce search query, using
AggregationBuilders.topHits("th").sort("_score", SortOrder.DESC)
causes query to contain duplicate sort clauses:

                        "sort": [
                            {
                                "_score": {
                                    "order": "desc"
                                }
                            },
                            {
                                "_score": {
                                    "order": "desc"
                                }
                            }
                        ],

This also seems to break search, causing many scores to return as null.

It seems that this is because at the following locations ifs are missing their elses:
https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java#L233-L236
https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java#L250-L253

Simple work around is to use directly AggregationBuilders.topHits("th").sort(SortBuilders.scoreSort().order(SortOrder.DESC)).

@jimczi jimczi added the :Analytics/Aggregations Aggregations label May 15, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@Hohol
Copy link
Contributor

Hohol commented May 16, 2019

It looked trivial, so I've made a PR #42179

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

Successfully merging a pull request may close this issue.

7 participants