[GitHub] [lucene] jtibshirani commented on a change in pull request #185: LUCENE-9999: CombinedFieldQuery can fail with an exception when document is missing fields

[GitBox <gi...@apache.org>]

jtibshirani commented on a change in pull request #185:
URL: https://github.com/apache/lucene/pull/185#discussion_r654110030



##########
File path: lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestCombinedFieldQuery.java
##########
@@ -85,7 +86,7 @@ public void testToString() {
     assertEquals("CombinedFieldQuery((foo title^3.0)(bar baz))", builder.build().toString());
   }
 
-  public void testSameScore() throws IOException {
+  public void testSparseFieldSameScore() throws IOException {

Review comment:
       It's a little unclear to me what we aim to test with the 'same score' checks. The `testCopyField` test seems to best capture the core behavior -- we could have a version of `testCopyField` where one field is sparse?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/MultiNormsLeafSimScorer.java
##########
@@ -80,9 +80,7 @@
   }
 
   private long getNormValue(int doc) throws IOException {
-    if (norms != null) {
-      boolean found = norms.advanceExact(doc);
-      assert found;
+    if (norms != null && norms.advanceExact(doc)) {

Review comment:
       +1 from me to disallow it upfront, the combined score won't be meaningful if there is a mix of norms being disabled/ enabled.




-- 
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:
users@infra.apache.org



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