SOLR-16289: Null check in transformers of ltr module
https://issues.apache.org/jira/browse/SOLR-16289
Description
In SolrCloud (with enabled two-stage shard request), LTRInterleavingScoringQuery#getPickedInterleavingDocIds may be null. More details, see original JIRA ticket.
Solution
Add null check.
Tests
No tests are added yet. I ran the patched SolrCloud and checked that it works well.
Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the
mainbranch. - [ ] I have run
./gradlew check. - [ ] I have added tests for my changes.
- [ ] I have added documentation for the Reference Guide
Thanks for the patch!
Can you add a unit test demonstrating this failure? I think from there it will be easier to brainstorm a fuller solution in case you think the null check is insufficient.
Thanks for reviewing @madrob!
I added two unit tests to demonstrate the failure. I think it's not look good way to use DocTransformer that uses scoring informations in SolrCloud. I try to fix this problem with implementing SearchComponent, but it has became a very complicated implementation...
My bad, it seems this information is not in the documentation (I may have just put it in the original blog post telling the story of the contribution). If these changes don't make sense anymore, we can just adapt this Pull Request to add the missing piece in the documentation.
+1 to updating the documentation if the info isn't already in there (I haven't looked).
Though even if it's documented as unsupported then a null pointer exception in response to attempted use still to me would seem a poor user experience. I'd love to see both the documentation update and the avoidance of the NPE.
Yes, ideally I would like to see an exception saying "interleaving is not supported in SolrCloud", I don't have time now to work on that but the author of this PR could re-purpose it to that direction and I'll be happy to review!
+1 I agree with both to update documentation and to throw an exception. I will add these updates. However, I want to support interleaving in SolrCloud ideally.
My idea is,
- Support the caching across multi-stage requests. This feature helps to pass interleaving results calculated by
ResponseBuilder.STAGE_EXECUTE_QUERYstage toResponseBuilder.STAGE_GET_FIELDSstage. - Customize
MergeStrategyto merge interleaving results returned from each shard.
Please review these ways to fix this problem.
This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the [email protected] mailing list. Thank you for your contribution!