SOLR-5882: introducing local param {!parent score=..}..

fixing ScoreMode.Min for ToParentBlockJoinQuery
fixing ScoreMode parsing exception in ScoreJoinQParserPlugin

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1693926 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -308,6 +308,8 @@ Bug fixes
 * LUCENE-6704: GeoPointDistanceQuery was visiting too many term ranges,
   consuming too much heap for a large radius (Nick Knize via Mike McCandless)
   
+* SOLR-5882: fix ScoreMode.Min at ToParentBlockJoinQuery (Mikhail Khludnev)
+
 Changes in Runtime Behavior
 
 * LUCENE-6501: The subreader structure in ParallelCompositeReader

lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java

@@ -297,7 +297,7 @@ public class ToParentBlockJoinQuery extends Query {
             pendingChildScores[childDocUpto] = childScore;
           }
           maxScore = Math.max(childScore, maxScore);
-          minScore = Math.min(childFreq, minScore);
+          minScore = Math.min(childScore, minScore);
           totalScore += childScore;
           parentFreq += childFreq;
         }

solr/CHANGES.txt

@@ -174,6 +174,8 @@ New Features
 
 * SOLR-6234: Scoring for query time join (Mikhail Khludnev)
 
+* SOLR-5882: score local parameter for block join query parser {!parent} (Andrey Kudryavtsev, Mikhail Khludnev)
+
 Bug Fixes
 ----------------------
 

solr/core/src/java/org/apache/solr/search/join/BlockJoinChildQParser.java

@@ -28,7 +28,8 @@ public class BlockJoinChildQParser extends BlockJoinParentQParser {
     super(qstr, localParams, params, req);
   }
 
-  protected Query createQuery(Query parentListQuery, Query query) {
+  @Override
+  protected Query createQuery(Query parentListQuery, Query query, String scoreMode) {
     return new ToChildBlockJoinQuery(query, getFilter(parentListQuery).filter);
   }
 
@@ -37,5 +38,3 @@ public class BlockJoinChildQParser extends BlockJoinParentQParser {
     return "of";
   }
 }
-
-

solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java

@@ -55,6 +55,7 @@ class BlockJoinParentQParser extends QParser {
   @Override
   public Query parse() throws SyntaxError {
     String filter = localParams.get(getParentFilterLocalParamName());
+    String scoreMode = localParams.get("score", ScoreMode.None.name());
     QParser parentParser = subQuery(filter, null);
     Query parentQ = parentParser.getQuery();
 
@@ -67,11 +68,12 @@ class BlockJoinParentQParser extends QParser {
     }
     QParser childrenParser = subQuery(queryText, null);
     Query childrenQuery = childrenParser.getQuery();
-    return createQuery(parentQ, childrenQuery);
+    return createQuery(parentQ, childrenQuery, scoreMode);
   }
 
-  protected Query createQuery(Query parentList, Query query) {
+  protected Query createQuery(Query parentList, Query query, String scoreMode) throws SyntaxError {
-    return new ToParentBlockJoinQuery(query, getFilter(parentList).filter, ScoreMode.None);
+    return new ToParentBlockJoinQuery(query, getFilter(parentList).filter, 
+        ScoreModeParser.parse(scoreMode));
   }
 
   BitDocIdSetFilterWrapper getFilter(Query parentList) {

solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParserPlugin.java

@@ -17,6 +17,7 @@
 
 package org.apache.solr.search.join;
 
+import org.apache.lucene.search.join.ScoreMode;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.request.SolrQueryRequest;
@@ -25,7 +26,8 @@ import org.apache.solr.search.QParserPlugin;
 
 /**
  * Usage: {!parent which="PARENT:true"}CHILD_PRICE:10
- *
+ * supports optional <code>score</code> parameter with one of {@link ScoreMode} values:
+ *  None,Avg,Total,Min,Max. Lowercase is also accepted.
  **/
 public class BlockJoinParentQParserPlugin extends QParserPlugin {
   public static final String NAME = "parent";

solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java

@@ -18,10 +18,6 @@
 package org.apache.solr.search.join;
 
 import java.io.IOException;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Locale;
-import java.util.Map;
 
 import org.apache.lucene.index.DocValuesType;
 import org.apache.lucene.index.IndexReader;
@@ -207,15 +203,6 @@ public class ScoreJoinQParserPlugin extends QParserPlugin {
     }
   }
 
-  final static Map<String, ScoreMode> lowercase = Collections.unmodifiableMap( new HashMap<String, ScoreMode>() {
-    {
-      for (ScoreMode s : ScoreMode.values()) {
-        put(s.name().toLowerCase(Locale.ROOT), s);
-        put(s.name(), s);
-      }
-    }
-  });
-
   @Override
   public void init(NamedList args) {
   }
@@ -229,7 +216,7 @@ public class ScoreJoinQParserPlugin extends QParserPlugin {
         final String fromField = localParams.get("from");
         final String fromIndex = localParams.get("fromIndex");
         final String toField = localParams.get("to");
-        final ScoreMode scoreMode = parseScore();
+        final ScoreMode scoreMode = ScoreModeParser.parse(getParam(SCORE));
 
         final String v = localParams.get(CommonParams.VALUE);
 
@@ -279,16 +266,8 @@ public class ScoreJoinQParserPlugin extends QParserPlugin {
           return new SameCoreJoinQuery(fromQuery, fromField, toField, scoreMode);
         }
       }
-
-      private ScoreMode parseScore() {
-
-        String score = getParam(SCORE);
-        final ScoreMode scoreMode = lowercase.get(score);
-        if (scoreMode == null) {
-          throw new IllegalArgumentException("Unable to parse ScoreMode from: " + score);
-        }
-        return scoreMode;
-      }
     };
   }
 }
+
+

solr/core/src/java/org/apache/solr/search/join/ScoreModeParser.java

@@ -0,0 +1,54 @@
+package org.apache.solr.search.join;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+
+import org.apache.lucene.search.join.ScoreMode;
+import org.apache.solr.search.SyntaxError;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class ScoreModeParser {
+  final private static Map<String, ScoreMode> lowerAndCapitalCase = 
+      Collections.unmodifiableMap( new HashMap<String, ScoreMode>() {
+    {
+      for (ScoreMode s : ScoreMode.values()) {
+        put(s.name().toLowerCase(Locale.ROOT), s);
+        put(s.name(), s);
+      }
+    }
+  });
+
+  private ScoreModeParser(){}
+  
+  /** 
+   * recognizes as-is {@link ScoreMode} names, and lowercase as well, 
+   * otherwise throws exception 
+   * @throws SyntaxError when it's unable to parse
+   * */
+  static ScoreMode parse(String score) throws SyntaxError {
+    final ScoreMode scoreMode = lowerAndCapitalCase.get(score);
+    if (scoreMode == null) {
+      throw new SyntaxError("Unable to parse ScoreMode from: " + score);
+    }
+    return scoreMode;
+  }
+  
+}

solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java

@@ -17,9 +17,12 @@
 
 package org.apache.solr.search.join;
 
+import org.apache.lucene.search.join.ScoreMode;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.BaseTestHarness;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -29,6 +32,10 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.ListIterator;
+import java.util.Locale;
+
+import javax.xml.namespace.QName;
+import javax.xml.xpath.XPathConstants;
 
 public class BJQParserTest extends SolrTestCaseJ4 {
   
@@ -165,6 +172,52 @@ public class BJQParserTest extends SolrTestCaseJ4 {
         beParents);
   }
 
+  public void testScoreNoneScoringForParent() throws Exception {
+    assertQ("score=none yields 0.0 score",
+        req("q", "{!parent which=\"parent_s:[* TO *]\" "+(
+            rarely()? "":(rarely()? "score=None":"score=none")
+            )+"}child_s:l","fl","score"),
+        "//*[@numFound='6']",
+        "(//float[@name='score'])["+(random().nextInt(6)+1)+"]=0.0");
+  }
+
+  public void testWrongScoreExceptionForParent() throws Exception {
+    final String aMode = ScoreMode.values()[random().nextInt(ScoreMode.values().length)].name();
+    final String wrongMode = rarely()? "":(rarely()? " ":
+      rarely()? aMode.substring(1):aMode.toUpperCase(Locale.ROOT));
+    assertQEx("wrong score mode", 
+        req("q", "{!parent which=\"parent_s:[* TO *]\" score="+wrongMode+"}child_s:l","fl","score")
+        , SolrException.ErrorCode.BAD_REQUEST.code);
+  }
+
+  public void testScoresForParent() throws Exception{
+    final ArrayList<ScoreMode> noNone = new ArrayList<>(Arrays.asList(ScoreMode.values()));
+    noNone.remove(ScoreMode.None);
+    final String notNoneMode = (noNone.get(random().nextInt(noNone.size()))).name();
+    
+    String leastScore = getLeastScore("child_s:l");
+    assertTrue(leastScore+" > 0.0", Float.parseFloat(leastScore)>0.0);
+    final String notNoneLower = usually() ? notNoneMode: notNoneMode.toLowerCase(Locale.ROOT);
+    
+    assertQ(req("q", "{!parent which=\"parent_s:[* TO *]\" score="+notNoneLower+"}child_s:l","fl","score"),
+        "//*[@numFound='6']","(//float[@name='score'])["+(random().nextInt(6)+1)+"]>='"+leastScore+"'");
+  }
+  
+  public void testScoresForChild() throws Exception{ 
+    String leastScore = getLeastScore("parent_s:a");
+      assertTrue(leastScore+" > 0.0", Float.parseFloat(leastScore)>0.0);
+      assertQ(
+          req("q", "{!child of=\"parent_s:[* TO *]\"}parent_s:a","fl","score"), 
+          "//*[@numFound='6']","(//float[@name='score'])["+(random().nextInt(6)+1)+"]>='"+leastScore+"'");
+  }
+  
+  private String getLeastScore(String query) throws Exception {
+    final String resp = h.query(req("q",query, "sort","score asc", "fl","score"));
+    return (String) BaseTestHarness.
+        evaluateXPath(resp,"(//float[@name='score'])[1]/text()", 
+            XPathConstants.STRING);
+  }
+
   @Test
   public void testFq() {
     assertQ(

solr/core/src/test/org/apache/solr/search/join/TestScoreJoinQPScore.java

@@ -27,6 +27,7 @@ import java.util.Random;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.join.ScoreMode;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrRequestInfo;
@@ -245,14 +246,11 @@ public class TestScoreJoinQPScore extends SolrTestCaseJ4 {
 
       assertEquals("lowercase shouldn't change anything", resp, repeat);
 
-      try {
+        final String aMod = score.substring(0, score.length() - 1);
-        h.query(req("q", "{!join from=" + from + " to=" + to + " score=" + score.substring(0, score.length() - 1) +
+        assertQEx("exception on "+aMod, "ScoreMode", 
-                "}" + q, "fl", "id", "omitHeader", "true")
+            req("q", "{!join from=" + from + " to=" + to + " score=" + aMod +
-        );
+                "}" + q, "fl", "id", "omitHeader", "true"), 
-        fail("excpecting exception");
+                SolrException.ErrorCode.BAD_REQUEST);
-      } catch (IllegalArgumentException e) {
-        assertTrue(e.getMessage().contains("ScoreMode"));
-      }
     }
     // this queries are not overlap, with other in this test case. 
     // however it might be better to extract this method into the separate suite