From 83520603c1cf2125d2f48f66cbd951e41b93f54e Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Mon, 3 Aug 2015 16:14:52 +0000 Subject: [PATCH] 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 | 2 + .../search/join/ToParentBlockJoinQuery.java | 2 +- solr/CHANGES.txt | 2 + .../search/join/BlockJoinChildQParser.java | 5 +- .../search/join/BlockJoinParentQParser.java | 8 +-- .../join/BlockJoinParentQParserPlugin.java | 4 +- .../search/join/ScoreJoinQParserPlugin.java | 27 ++-------- .../solr/search/join/ScoreModeParser.java | 54 +++++++++++++++++++ .../solr/search/join/BJQParserTest.java | 53 ++++++++++++++++++ .../search/join/TestScoreJoinQPScore.java | 14 +++-- 10 files changed, 131 insertions(+), 40 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/search/join/ScoreModeParser.java diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index aa465e06fbf..0b6c4e3c60e 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -307,6 +307,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 diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index fd6256b6b1b..8d4d093c34c 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/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; } diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 7ff90b2ab88..abc7c47cefb 100644 --- a/solr/CHANGES.txt +++ b/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 ---------------------- diff --git a/solr/core/src/java/org/apache/solr/search/join/BlockJoinChildQParser.java b/solr/core/src/java/org/apache/solr/search/join/BlockJoinChildQParser.java index 9c3ba641620..696b6ab7ac6 100644 --- a/solr/core/src/java/org/apache/solr/search/join/BlockJoinChildQParser.java +++ b/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"; } } - - diff --git a/solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java b/solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java index d9175b0fbb5..abdce80dfc0 100644 --- a/solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java +++ b/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) { - return new ToParentBlockJoinQuery(query, getFilter(parentList).filter, ScoreMode.None); + protected Query createQuery(Query parentList, Query query, String scoreMode) throws SyntaxError { + return new ToParentBlockJoinQuery(query, getFilter(parentList).filter, + ScoreModeParser.parse(scoreMode)); } BitDocIdSetFilterWrapper getFilter(Query parentList) { diff --git a/solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParserPlugin.java index 14813bcff36..22d9fffe36b 100644 --- a/solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParserPlugin.java +++ b/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 score 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"; diff --git a/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java index 9d15c075335..72a95b2e0dc 100644 --- a/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java +++ b/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 lowercase = Collections.unmodifiableMap( new HashMap() { - { - 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; - } }; } } + + diff --git a/solr/core/src/java/org/apache/solr/search/join/ScoreModeParser.java b/solr/core/src/java/org/apache/solr/search/join/ScoreModeParser.java new file mode 100644 index 00000000000..ab778376208 --- /dev/null +++ b/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 lowerAndCapitalCase = + Collections.unmodifiableMap( new HashMap() { + { + 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; + } + +} diff --git a/solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java b/solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java index 3d8086201e7..0d0d6c3a6fd 100644 --- a/solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java +++ b/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 { @@ -164,7 +171,53 @@ public class BJQParserTest extends SolrTestCaseJ4 { "parent_s:(e b)", "chq", "child_s:l", "pq", "parent_s:[* TO *]"), 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 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( diff --git a/solr/core/src/test/org/apache/solr/search/join/TestScoreJoinQPScore.java b/solr/core/src/test/org/apache/solr/search/join/TestScoreJoinQPScore.java index 1692b0c72d8..3938fd2d260 100644 --- a/solr/core/src/test/org/apache/solr/search/join/TestScoreJoinQPScore.java +++ b/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 { - h.query(req("q", "{!join from=" + from + " to=" + to + " score=" + score.substring(0, score.length() - 1) + - "}" + q, "fl", "id", "omitHeader", "true") - ); - fail("excpecting exception"); - } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("ScoreMode")); - } + final String aMod = score.substring(0, score.length() - 1); + assertQEx("exception on "+aMod, "ScoreMode", + req("q", "{!join from=" + from + " to=" + to + " score=" + aMod + + "}" + q, "fl", "id", "omitHeader", "true"), + SolrException.ErrorCode.BAD_REQUEST); } // this queries are not overlap, with other in this test case. // however it might be better to extract this method into the separate suite