cpoerschke commented on a change in pull request #151:
URL: https://github.com/apache/solr/pull/151#discussion_r641713249
##########
File path: solr/contrib/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java
##########
@@ -70,6 +72,63 @@ public void tearDown() throws Exception {
super.tearDown();
}
+ @Test
+ public void testSimpleQueryCustomSort() throws Exception {
+ SolrQuery query = new SolrQuery("*:*");
+ query.setRequestHandler("/query");
+ query.setFields("*,score,[shard]");
Review comment:
Please excuse me if it's too early to bring up also since you already
mentioned the _"The current solution only works if the original sort did not
sort by score. ..."_ qualification elsewhere in the pull request.
Have you considered
```suggestion
query.setFields("*,[shard]");
```
i.e. the federator likely needing to ask the shard for the `score`
if/because it will be needed but the solr client might not have requested it in
their request?
And again, perhaps too early to bring up, but since re-ranking is not
limited to LTR it would be great to eventually have test coverage in
`solr/core` (though I don't know where might be a good fit, going by test class
names only DistributedQueryComponentCustomSortTest might be a match).
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
