tomglk commented on a change in pull request #151:
URL: https://github.com/apache/solr/pull/151#discussion_r646032875
##########
File path:
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
##########
@@ -999,20 +1013,34 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest
sreq) {
shardDoc.sortFieldValues = unmarshalledSortFieldValues;
- queue.insertWithOverflow(shardDoc);
+ if(reRankQueue != null && docCounter++ <= reRankDocsSize) {
+ ShardDoc droppedShardDoc =
reRankQueue.insertWithOverflow(shardDoc);
+ // FIXME: Only works if the original request does not sort by
score
Review comment:
Hi @cpoerschke ,
thank you for the reassurance. :)
I just found the timing quite unlucky because we were suggesting an
inclusion of this PR in 8.9 just before the "break" and I have no idea when 8.9
may be released.
So I cannot estimate the time pressure we would be under if we decide for a
split of the PRs and aim for the 8.9 release.
Do you have any information on this? Maybe just a broad timeframe like 2
weeks / 2 months?
Having said that,
> maybe then loop back to and continue with your idea of a multi-part
solution? I.e. this pull request to fix for use cases with a score-free sort
and future pull request(s) for use cases whose sort contains a score (and which
therefore requires access to the original score in the coordinator but where it
is currently not available).
I think that this approach would make the most sense.
We were looking into different methods to gain access to the original score
during the week but do not have a good solution yet.
Just your point
> if existing behaviour is maintained for the 'not (yet) fixed' scenario
is something we would have to look at again. Currently the existing behavior
and the sort by score with our fix are broken, but I don't think that they are
broken in the same way.
And this point
> if it can be clearly documented/communicated what is fixed and what is not
fixed
really is important. I do not know where we would communicate this best, but
we definitely should make really clear, what was not working, what is working
after this PR and what will need another PR to work.
This
> fix for the second half is not going to break/change things for anyone who
benefitted from the first fix
should be no problem. The first PR would only work for people with sort !=
score and the second PR would only provide benefit for people with sort ==
score. Returning information for the original score also should not break
anything for people benefiting from this PR because that is just a bit more
information than can, but does not have to be used.
Regarding
> future pull request(s)
I think that the access to the original score and allowing the sort by score
are very intertwined. So as soon as we solve the access to the score, the
sorting by score is no longer a big problem. Therefore this could be one PR, in
my opinion.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
