You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/07/12 17:29:06 UTC

[GitHub] [solr] chlorochrule opened a new pull request, #937: SOLR-16289: Null check in transformers of ltr module

chlorochrule opened a new pull request, #937:
URL: https://github.com/apache/solr/pull/937

   https://issues.apache.org/jira/browse/SOLR-16289
   
   <!--
   _(If you are a project committer then you may remove some/all of the following template.)_
   
   Before creating a pull request, please file an issue in the ASF Jira system for Solr:
   
   * https://issues.apache.org/jira/projects/SOLR
   
   For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA. 
   To create a Jira issue, you will need to create an account there first.
   
   The title of the PR should reference the Jira issue number in the form:
   
   * SOLR-####: <short description of problem or changes>
   
   SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
   
   Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
   
   
   # 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](https://wiki.apache.org/solr/HowToContribute) 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](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [x] I have developed this patch against the `main` branch.
   - [x] I have run `./gradlew check`.
   - [ ] I have added tests for my changes.
   - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chlorochrule commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by GitBox <gi...@apache.org>.
chlorochrule commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r921117413


##########
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java:
##########
@@ -322,6 +323,29 @@ public void testSimpleQuery() throws Exception {
     assertEquals(result7_features, queryResponse.getResults().get(7).get("features").toString());
   }
 
+  @Test
+  public void testInterleaving() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add("rq", "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+
+    solrCluster.getSolrClient().query(COLLECTION, query);  // NullPointerException at LTRInterleavingTransformerFactory.java:103
+  }

Review Comment:
   This test fails with NPE



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chlorochrule commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by GitBox <gi...@apache.org>.
chlorochrule commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r921120099


##########
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java:
##########
@@ -322,6 +323,29 @@ public void testSimpleQuery() throws Exception {
     assertEquals(result7_features, queryResponse.getResults().get(7).get("features").toString());
   }
 
+  @Test
+  public void testInterleaving() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add("rq", "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+
+    solrCluster.getSolrClient().query(COLLECTION, query);  // NullPointerException at LTRInterleavingTransformerFactory.java:103
+  }
+
+  @Test
+  public void testInterleavingWithDistribSinglePass() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add("rq", "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+    query.setParam(ShardParams.DISTRIB_SINGLE_PASS, true);
+
+    QueryResponse queryResponse = solrCluster.getSolrClient().query(COLLECTION, query);  // works well
+    assertTrue(queryResponse.getResults()
+        .get(0).getFieldValue("[interleaving]").toString().startsWith("powpularityS-model"));
+  }

Review Comment:
   This test doesn't fail because `distrib.singlePass=true` parameter is specified



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chlorochrule commented on pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by GitBox <gi...@apache.org>.
chlorochrule commented on PR #937:
URL: https://github.com/apache/solr/pull/937#issuecomment-1184420046

   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...


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r1114576195


##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -362,9 +363,13 @@ private void implTransform(SolrDocument doc, int docid, Float score) throws IOEx
       LTRScoringQuery rerankingQuery = rerankingQueries[0];
       LTRScoringQuery.ModelWeight rerankingModelWeight = modelWeights[0];
       for (int i = 1; i < rerankingQueries.length; i++) {
-        if (((LTRInterleavingScoringQuery) rerankingQueriesFromContext[i])
-            .getPickedInterleavingDocIds()
-            .contains(docid)) {
+        Set<Integer> pickedInterleavingDocIds =
+            ((LTRInterleavingScoringQuery) rerankingQueriesFromContext[i])
+                .getPickedInterleavingDocIds();
+        if (pickedInterleavingDocIds == null) {
+          return;
+        }
+        if (pickedInterleavingDocIds.contains(docid)) {

Review Comment:
   I wonder if this could be combined here? Will add a commit to explore, feel free to revert.
   ```suggestion
           if (pickedInterleavingDocIds != null &&
               pickedInterleavingDocIds.contains(docid)) {
   ```



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-16289: Null check in transformers of ltr module [solr]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #937:
URL: https://github.com/apache/solr/pull/937#issuecomment-1951491384

   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 dev@solr.apache.org mailing list. Thank you for your contribution!


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r1114582258


##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -362,9 +363,13 @@ private void implTransform(SolrDocument doc, int docid, Float score) throws IOEx
       LTRScoringQuery rerankingQuery = rerankingQueries[0];
       LTRScoringQuery.ModelWeight rerankingModelWeight = modelWeights[0];
       for (int i = 1; i < rerankingQueries.length; i++) {
-        if (((LTRInterleavingScoringQuery) rerankingQueriesFromContext[i])
-            .getPickedInterleavingDocIds()
-            .contains(docid)) {
+        Set<Integer> pickedInterleavingDocIds =
+            ((LTRInterleavingScoringQuery) rerankingQueriesFromContext[i])
+                .getPickedInterleavingDocIds();
+        if (pickedInterleavingDocIds == null) {
+          return;
+        }
+        if (pickedInterleavingDocIds.contains(docid)) {

Review Comment:
   > ... Will add a commit to explore, feel free to revert. ...
   
   https://github.com/apache/solr/pull/937/commits/c7f194fc58a599616cb822ce2c291cae7f693fc8 is the commit.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chlorochrule commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by GitBox <gi...@apache.org>.
chlorochrule commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r921138101


##########
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java:
##########
@@ -322,6 +323,43 @@ public void testSimpleQuery() throws Exception {
     assertEquals(result7_features, queryResponse.getResults().get(7).get("features").toString());
   }
 
+  @Test
+  public void testInterleaving() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add(
+        "rq",
+        "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+
+    solrCluster
+        .getSolrClient()
+        .query(
+            COLLECTION,
+            query); // NullPointerException at LTRInterleavingTransformerFactory.java:103
+  }

Review Comment:
   However, this branch(SOLR-16289) already fixed NPE. To reproduce the problem, checkout https://github.com/chlorochrule/solr/commit/33d1aa939a175310c1eb6f0b773a5c2d3a4ddb41



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chlorochrule commented on pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by "chlorochrule (via GitHub)" <gi...@apache.org>.
chlorochrule commented on PR #937:
URL: https://github.com/apache/solr/pull/937#issuecomment-1448240364

   +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_QUERY` stage to `ResponseBuilder.STAGE_GET_FIELDS` stage.
   - Customize `MergeStrategy` to merge interleaving results returned from each shard.
   
   Please review these ways to fix this problem.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chlorochrule commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by GitBox <gi...@apache.org>.
chlorochrule commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r921117413


##########
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java:
##########
@@ -322,6 +323,29 @@ public void testSimpleQuery() throws Exception {
     assertEquals(result7_features, queryResponse.getResults().get(7).get("features").toString());
   }
 
+  @Test
+  public void testInterleaving() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add("rq", "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+
+    solrCluster.getSolrClient().query(COLLECTION, query);  // NullPointerException at LTRInterleavingTransformerFactory.java:103
+  }

Review Comment:
   This test failed with NPE



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chlorochrule commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by GitBox <gi...@apache.org>.
chlorochrule commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r921117413


##########
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java:
##########
@@ -322,6 +323,29 @@ public void testSimpleQuery() throws Exception {
     assertEquals(result7_features, queryResponse.getResults().get(7).get("features").toString());
   }
 
+  @Test
+  public void testInterleaving() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add("rq", "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+
+    solrCluster.getSolrClient().query(COLLECTION, query);  // NullPointerException at LTRInterleavingTransformerFactory.java:103
+  }

Review Comment:
   This test fails with NPE



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] madrob commented on pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by GitBox <gi...@apache.org>.
madrob commented on PR #937:
URL: https://github.com/apache/solr/pull/937#issuecomment-1183479704

   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.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] alessandrobenedetti commented on pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #937:
URL: https://github.com/apache/solr/pull/937#issuecomment-1440410507

   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 misisng piece in the documentation. 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke commented on pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on PR #937:
URL: https://github.com/apache/solr/pull/937#issuecomment-1443356342

   +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.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] alessandrobenedetti commented on pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #937:
URL: https://github.com/apache/solr/pull/937#issuecomment-1443514418

   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!


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r1114585921


##########
solr/modules/ltr/src/java/org/apache/solr/ltr/interleaving/LTRInterleavingScoringQuery.java:
##########
@@ -25,7 +26,7 @@
 public class LTRInterleavingScoringQuery extends LTRScoringQuery {
 
   // Model was picked for this Docs
-  private Set<Integer> pickedInterleavingDocIds;
+  private Set<Integer> pickedInterleavingDocIds = Collections.emptySet();

Review Comment:
   This seems to be another way to avoid the NPE (thanks for adding the tests!) and it would remove the need for `*TransformerFactory` changes ...
   
   Though maybe it's a bit unconventional/controversial to initialise like this? 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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chlorochrule commented on a diff in pull request #937: SOLR-16289: Null check in transformers of ltr module

Posted by GitBox <gi...@apache.org>.
chlorochrule commented on code in PR #937:
URL: https://github.com/apache/solr/pull/937#discussion_r921131573


##########
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java:
##########
@@ -322,6 +323,43 @@ public void testSimpleQuery() throws Exception {
     assertEquals(result7_features, queryResponse.getResults().get(7).get("features").toString());
   }
 
+  @Test
+  public void testInterleaving() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add(
+        "rq",
+        "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+
+    solrCluster
+        .getSolrClient()
+        .query(
+            COLLECTION,
+            query); // NullPointerException at LTRInterleavingTransformerFactory.java:103
+  }

Review Comment:
   This test fails with NPE



##########
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java:
##########
@@ -322,6 +323,43 @@ public void testSimpleQuery() throws Exception {
     assertEquals(result7_features, queryResponse.getResults().get(7).get("features").toString());
   }
 
+  @Test
+  public void testInterleaving() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add(
+        "rq",
+        "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+
+    solrCluster
+        .getSolrClient()
+        .query(
+            COLLECTION,
+            query); // NullPointerException at LTRInterleavingTransformerFactory.java:103
+  }
+
+  @Test
+  public void testInterleavingWithDistribSinglePass() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add(
+        "rq",
+        "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+    query.setParam(ShardParams.DISTRIB_SINGLE_PASS, true);
+
+    QueryResponse queryResponse =
+        solrCluster.getSolrClient().query(COLLECTION, query); // works well
+    assertTrue(
+        queryResponse
+            .getResults()
+            .get(0)
+            .getFieldValue("[interleaving]")
+            .toString()
+            .startsWith("powpularityS-model"));
+  }

Review Comment:
   This test doesn't fail because distrib.singlePass=true parameter is specified



##########
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTROnSolrCloud.java:
##########
@@ -322,6 +323,29 @@ public void testSimpleQuery() throws Exception {
     assertEquals(result7_features, queryResponse.getResults().get(7).get("features").toString());
   }
 
+  @Test
+  public void testInterleaving() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add("rq", "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+
+    solrCluster.getSolrClient().query(COLLECTION, query);  // NullPointerException at LTRInterleavingTransformerFactory.java:103
+  }
+
+  @Test
+  public void testInterleavingWithDistribSinglePass() throws Exception {
+    SolrQuery query = new SolrQuery("{!func}sub(8,field(popularity))");
+    query.setRequestHandler("/query");
+    query.setFields("*,score,[interleaving]");
+    query.add("rq", "{!ltr model=powpularityS-model model=powpularityS-model-for-interleaving reRankDocs=8}");
+    query.setParam(ShardParams.DISTRIB_SINGLE_PASS, true);
+
+    QueryResponse queryResponse = solrCluster.getSolrClient().query(COLLECTION, query);  // works well
+    assertTrue(queryResponse.getResults()
+        .get(0).getFieldValue("[interleaving]").toString().startsWith("powpularityS-model"));
+  }

Review Comment:
   This test doesn't fail because `distrib.singlePass=true` parameter is specified



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org