You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mu...@apache.org on 2019/06/29 16:40:54 UTC

[lucene-solr] branch branch_8x updated: SOLR-9409: improve error message on unsupported types in collapsing

This is an automated email from the ASF dual-hosted git repository.

munendrasn pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 6c83e39  SOLR-9409: improve error message on unsupported types in collapsing
6c83e39 is described below

commit 6c83e39dcced18a247449166d454eb18b59e00b9
Author: Munendra S N <mu...@apache.org>
AuthorDate: Sat Jun 29 21:37:09 2019 +0530

    SOLR-9409: improve error message on unsupported types in collapsing
    
    * Improve error message when collapsing is not supported on given
      fieldtype
    * Return 400 error code when unsupported value are passed for max,min
      or in case of syntax error
---
 solr/CHANGES.txt                                   |  3 ++
 .../solr/search/CollapsingQParserPlugin.java       | 33 ++++++++++------------
 .../solr/search/TestCollapseQParserPlugin.java     | 23 ++++++---------
 3 files changed, 26 insertions(+), 33 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index b764009..a89e848 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -154,6 +154,9 @@ Bug Fixes
   
 * SOLR-13280: Strengthen ScheduledTrigger's preferredOperation parameter validation. (Christine Poerschke)
 
+* SOLR-9409: Improve error message for unsupported field types and return 400 error code on
+  unsupported values in collapsing (hossman, Munendra S N)
+
 Other Changes
 ----------------------
 
diff --git a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
index c99fc85..b63c119 100644
--- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
@@ -389,10 +389,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
                                              boostDocsMap,
                                              searcher);
 
-      } catch (SolrException e) {
-        // handle SolrException separately
-        throw e;
-      } catch (Exception e) {
+      } catch (IOException e) {
         throw new RuntimeException(e);
       }
     }
@@ -973,7 +970,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
       } else {
         NumberType numType = fieldType.getNumberType();
         if (null == numType) {
-          throw new IOException("min/max must be either Int/Long/Float based field types");
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "min/max must be either Int/Long/Float based field types");
         }
         switch (numType) {
           case INTEGER: {
@@ -989,7 +986,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
             break;
           }
           default: {
-            throw new IOException("min/max must be either Int/Long/Float field types");
+            throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "min/max must be either Int/Long/Float field types");
           }
         }
       }
@@ -1170,7 +1167,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
             break;
           }
           default: {
-            throw new IOException("min/max must be Int or Float field types when collapsing on numeric fields");
+            throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+                "min/max must be Int or Float field types when collapsing on numeric fields");
           }
         }
       }
@@ -1314,7 +1312,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
         }
       } else {
         if(HINT_TOP_FC.equals(hint)) {
-          throw new IOException("top_fc hint is only supported when collapsing on String Fields");
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+              "top_fc hint is only supported when collapsing on String Fields");
         }
       }
 
@@ -1324,16 +1323,12 @@ public class CollapsingQParserPlugin extends QParserPlugin {
         if (text.indexOf("(") == -1) {
           minMaxFieldType = searcher.getSchema().getField(text).getType();
         } else {
-          LocalSolrQueryRequest request = null;
-          try {
-            SolrParams params = new ModifiableSolrParams();
-            request = new LocalSolrQueryRequest(searcher.getCore(), params);
+          SolrParams params = new ModifiableSolrParams();
+          try (SolrQueryRequest request = new LocalSolrQueryRequest(searcher.getCore(), params)) {
             FunctionQParser functionQParser = new FunctionQParser(text, null, null,request);
             funcQuery = (FunctionQuery)functionQParser.parse();
-          } catch (Exception e) {
-            throw new IOException(e);
-          } finally {
-            request.close();
+          } catch (SyntaxError e) {
+            throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
           }
         }
       }
@@ -1367,7 +1362,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
           return new IntScoreCollector(maxDoc, leafCount, nullValue, nullPolicy, size, collapseField, boostDocs);
 
         } else {
-          throw new IOException("64 bit numeric collapse fields are not supported");
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+              "Collapsing field should be of either String, Int or Float type");
         }
         
       } else { // min, max, sort, etc.. something other then just "score"
@@ -1419,7 +1415,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
                                             funcQuery,
                                             searcher);
         } else {
-          throw new IOException("64 bit numeric collapse fields are not supported");
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+              "Collapsing field should be of either String, Int or Float type");
         }
         
       }
diff --git a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
index 16790c0..2753542 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
@@ -16,7 +16,6 @@
  */
 package org.apache.solr.search;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -970,19 +969,13 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
 
   @Test
   public void test64BitCollapseFieldException() {
-    ModifiableSolrParams doubleParams = new ModifiableSolrParams();
-    doubleParams.add("q", "*:*");
-    doubleParams.add("fq", "{!collapse field=group_d}");
-    expectThrows(RuntimeException.class, IOException.class, () -> h.query(req(doubleParams)));
-
-    ModifiableSolrParams dateParams = new ModifiableSolrParams();
-    dateParams.add("q", "*:*");
-    dateParams.add("fq", "{!collapse field=group_dt}");
-    expectThrows(RuntimeException.class, IOException.class, () -> h.query(req(dateParams)));
-
-    ModifiableSolrParams longParams = new ModifiableSolrParams();
-    longParams.add("q", "*:*");
-    longParams.add("fq", "{!collapse field=group_l}");
-    expectThrows(RuntimeException.class, IOException.class, () -> h.query(req(longParams)));
+    assertQEx("Should Fail For collapsing on Long fields", "Collapsing field should be of either String, Int or Float type",
+        req("q", "*:*", "fq", "{!collapse field=group_l}"), SolrException.ErrorCode.BAD_REQUEST);
+
+    assertQEx("Should Fail For collapsing on Double fields", "Collapsing field should be of either String, Int or Float type",
+        req("q", "*:*", "fq", "{!collapse field=group_d}"), SolrException.ErrorCode.BAD_REQUEST);
+
+    assertQEx("Should Fail For collapsing on Date fields", "Collapsing field should be of either String, Int or Float type",
+        req("q", "*:*", "fq", "{!collapse field=group_dt}"), SolrException.ErrorCode.BAD_REQUEST);
   }
 }