You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by va...@apache.org on 2018/08/20 22:33:01 UTC

lucene-solr:branch_7x: SOLR-12683: HashQuery will throw an exception if more than 4 partitionKeys is specified. Earlier after the 4th partitionKey the keys would be silently ignored.

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_7x 82c64af84 -> b5e79d0db


SOLR-12683: HashQuery will throw an exception if more than 4 partitionKeys is specified. Earlier after the 4th partitionKey the keys would be silently ignored.

(cherry picked from commit 5eab1c3)


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/b5e79d0d
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/b5e79d0d
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/b5e79d0d

Branch: refs/heads/branch_7x
Commit: b5e79d0db0a884c454f8b6002cbabda5e82ee391
Parents: 82c64af
Author: Varun Thacker <va...@apache.org>
Authored: Mon Aug 20 18:21:19 2018 -0700
Committer: Varun Thacker <va...@apache.org>
Committed: Mon Aug 20 15:32:45 2018 -0700

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  3 ++
 .../apache/solr/search/HashQParserPlugin.java   | 19 ++++++------
 .../solr/search/TestHashQParserPlugin.java      | 31 ++++++++++++++++++++
 3 files changed, 44 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b5e79d0d/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 87131bc..1d45fbd 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -208,6 +208,9 @@ Bug Fixes
 
 * SOLR-12598: Do not fetch non-stored fields (Nikolay Khitrin, Erick Erickson)
 
+* SOLR-12683: HashQuery will throw an exception if more than 4 partitionKeys is specified.
+  Earlier after the 4th partitionKey the keys would be silently ignored. (Varun Thacker)
+
 Optimizations
 ----------------------
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b5e79d0d/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java
index d1e1b45..7356f62 100644
--- a/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java
@@ -72,15 +72,18 @@ public class HashQParserPlugin extends QParserPlugin {
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "workers needs to be more than 1");
       }
       int worker = localParams.getInt("worker", 0);
-      String keys = params.get("partitionKeys");
-      keys = keys.replace(" ", "");
+      String keyParam = params.get("partitionKeys");
+      String[] keys = keyParam.replace(" ", "").split(",");
+      if (keys.length > 4) {
+        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "HashQuery supports upto 4 partitionKeys");
+      }
       return new HashQuery(keys, workers, worker);
     }
   }
 
   private static class HashQuery extends ExtendedQueryBase implements PostFilter {
 
-    private String keysParam;
+    private String[] keys;
     private int workers;
     private int worker;
 
@@ -94,7 +97,7 @@ public class HashQParserPlugin extends QParserPlugin {
 
     public int hashCode() {
       return classHash() + 
-          31 * keysParam.hashCode() + 
+          31 * keys.hashCode() +
           31 * workers + 
           31 * worker;
     }
@@ -105,20 +108,19 @@ public class HashQParserPlugin extends QParserPlugin {
     }
 
     private boolean equalsTo(HashQuery other) {
-      return keysParam.equals(other.keysParam) && 
+      return keys.equals(other.keys) &&
              workers == other.workers && 
              worker == other.worker;
     }
 
-    public HashQuery(String keysParam, int workers, int worker) {
-      this.keysParam = keysParam;
+    public HashQuery(String[] keys, int workers, int worker) {
+      this.keys = keys;
       this.workers = workers;
       this.worker = worker;
     }
 
     public Weight createWeight(IndexSearcher searcher, boolean needsScores, float boost) throws IOException {
 
-      String[] keys = keysParam.split(",");
       SolrIndexSearcher solrIndexSearcher = (SolrIndexSearcher)searcher;
       IndexReaderContext context = solrIndexSearcher.getTopReaderContext();
 
@@ -222,7 +224,6 @@ public class HashQParserPlugin extends QParserPlugin {
     }
 
     public DelegatingCollector getFilterCollector(IndexSearcher indexSearcher) {
-      String[] keys = keysParam.split(",");
       HashKey[] hashKeys = new HashKey[keys.length];
       SolrIndexSearcher searcher = (SolrIndexSearcher)indexSearcher;
       IndexSchema schema = searcher.getSchema();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b5e79d0d/solr/core/src/test/org/apache/solr/search/TestHashQParserPlugin.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestHashQParserPlugin.java b/solr/core/src/test/org/apache/solr/search/TestHashQParserPlugin.java
index 06c2097..03e68de 100644
--- a/solr/core/src/test/org/apache/solr/search/TestHashQParserPlugin.java
+++ b/solr/core/src/test/org/apache/solr/search/TestHashQParserPlugin.java
@@ -18,6 +18,7 @@ package org.apache.solr.search;
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -55,6 +56,36 @@ public class TestHashQParserPlugin extends SolrTestCaseJ4 {
   }
 
   @Test
+  public void testManyHashPartitions() throws Exception {
+    ModifiableSolrParams params = new ModifiableSolrParams();
+    params.add("q", "*:*");
+    params.add("fq", "{!hash worker=0 workers=2 cost="+getCost(random())+"}");
+    params.add("partitionKeys", "a_i,a_s,a_i,a_s");
+    params.add("wt", "xml");
+    String response = h.query(req(params));
+    h.validateXPath(response, "//*[@numFound='0']");
+
+    params = new ModifiableSolrParams();
+    params.add("q", "*:*");
+    params.add("fq", "{!hash worker=0 workers=2 cost="+getCost(random())+"}");
+    params.add("partitionKeys", "a_i,a_s,a_i,a_s,a_i");
+    params.add("wt", "xml");
+    ModifiableSolrParams finalParams = params;
+    expectThrows(SolrException.class, () -> h.query(req(finalParams)));
+  }
+
+  @Test
+  public void testLessWorkers() {
+    ModifiableSolrParams params = new ModifiableSolrParams();
+    params.add("q", "*:*");
+    params.add("fq", "{!hash worker=0 workers=1 cost="+getCost(random())+"}");
+    params.add("partitionKeys", "a_i");
+    params.add("wt", "xml");
+    ModifiableSolrParams finalParams = params;
+    expectThrows(SolrException.class, () -> h.query(req(finalParams)));
+  }
+
+  @Test
   public void testHashPartitionWithEmptyValues() throws Exception {
 
     assertU(adoc("id", "1", "a_s", "one", "a_i" , "1"));