SOLR-2133: check garbage after func(s), allow parsing comma delimited list of functions

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

solr/CHANGES.txt

@@ -272,6 +272,13 @@ New Features
   Example: q=add($v1,$v2)&v1=mul(popularity,5)&v2=20.0
   (yonik)
 
+* SOLR-2133: Function query parser can now parse multiple coma separated
+  value sources.  It also now fails if there is extra unexpected text
+  after parsing the functions, instead of silently ignoring it.
+  This allows expressions like q=dist(2,vector(1,2),$pt)&pt=3,4   (yonik)
+
+
+
 Optimizations
 ----------------------
 

solr/src/java/org/apache/solr/search/FunctionQParser.java

@@ -29,25 +29,71 @@ import java.util.List;
 
 public class FunctionQParser extends QParser {
 
-  protected QueryParsing.StrParser sp;
+  /** @lucene.internal */
+  public QueryParsing.StrParser sp;
+  boolean parseMultipleSources = true;
+  boolean parseToEnd = true;
 
   public FunctionQParser(String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
     super(qstr, localParams, params, req);
   }
 
+  public void setParseMultipleSources(boolean parseMultipleSources) {
+    this.parseMultipleSources = parseMultipleSources;  
+  }
+
+  /** parse multiple comma separated value sources */
+  public boolean getParseMultipleSources() {
+    return parseMultipleSources;
+  }
+
+  public void setParseToEnd(boolean parseToEnd) {
+    this.parseMultipleSources = parseMultipleSources;
+  }
+
+  /** throw exception if there is extra stuff at the end of the parsed valuesource(s). */
+  public boolean getParseToEnd() {
+    return parseMultipleSources;
+  }
+
   public Query parse() throws ParseException {
     sp = new QueryParsing.StrParser(getString());
-    ValueSource vs = parseValueSource();
 
-    /***  boost promoted to top-level query type to avoid this hack 
+    ValueSource vs = null;
+    List<ValueSource> lst = null;
 
-    // HACK - if this is a boosted query wrapped in a value-source, return
-    // that boosted query instead of a FunctionQuery
-    if (vs instanceof QueryValueSource) {
-      Query q = ((QueryValueSource)vs).getQuery();
-      if (q instanceof BoostedQuery) return q;
+    for(;;) {
+      ValueSource valsource = parseValueSource(false);
+      sp.eatws();
+      if (!parseMultipleSources) {
+        vs = valsource; 
+        break;
+      } else {
+        if (lst != null) {
+          lst.add(valsource);
+        } else {
+          vs = valsource;
+        }
+      }
+
+      // check if there is a "," separator
+      if (sp.peek() != ',') break;
+
+      consumeArgumentDelimiter();
+
+      if (lst == null) {
+        lst = new ArrayList<ValueSource>(2);
+        lst.add(valsource);
+      }
+    }
+
+    if (parseToEnd && sp.pos < sp.end) {
+      throw new ParseException("Unexpected text after function: " + sp.val.substring(sp.pos, sp.end));
+    }
+
+    if (lst != null) {
+      vs = new VectorValueSource(lst);
     }
-    ***/
 
     return new FunctionQuery(vs);
   }
@@ -264,6 +310,9 @@ public class FunctionQParser extends QParser {
       }
 
       QParser subParser = subQuery(val, "func");
+      if (subParser instanceof FunctionQParser) {
+        ((FunctionQParser)subParser).setParseMultipleSources(true);
+      }
       Query subQuery = subParser.getQuery();
       if (subQuery instanceof FunctionQuery) {
         valueSource = ((FunctionQuery) subQuery).getValueSource();

solr/src/test/org/apache/solr/search/QueryParsingTest.java

@@ -133,13 +133,7 @@ public class QueryParsingTest extends SolrTestCaseJ4 {
     flds = sort.getSort();
     assertEquals(flds[0].getType(), SortField.FLOAT);
     assertEquals(flds[0].getField(), "weight");
-
-    try {
-      //bad number of parens, but the function parser can handle an extra close
-      sort = QueryParsing.parseSort("pow(weight,2)) desc, bday asc", schema);
-    } catch (SolrException e) {
-      assertTrue(false);
-    }
+gvim
     //Test literals in functions
     sort = QueryParsing.parseSort("strdist(foo_s, \"junk\", jw) desc", schema);
     flds = sort.getSort();

solr/src/test/org/apache/solr/search/function/TestFunctionQuery.java

@@ -23,6 +23,7 @@ import org.apache.lucene.search.Similarity;
 import org.apache.solr.SolrTestCaseJ4;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.junit.internal.runners.statements.Fail;
 
 import java.io.FileOutputStream;
 import java.io.OutputStreamWriter;
@@ -343,6 +344,18 @@ public class TestFunctionQuery extends SolrTestCaseJ4 {
     assertQ(req("fl","*,score","q", "{!func}add($v1,$v2)", "v1","add($v3,$v4)", "v2","1", "v3","2", "v4","5"
         , "fq","id:1"), "//float[@name='score']='8.0'");
 
+    // test ability to parse multiple values
+    assertQ(req("fl","*,score","q", "{!func}dist(2,vector(1,1),$pt)", "pt","3,1"
+        , "fq","id:1"), "//float[@name='score']='2.0'");
+
+    // test that extra stuff after a function causes an error
+    try {
+      assertQ(req("fl","*,score","q", "{!func}10 wow dude ignore_exception"));
+      fail();
+    } catch (Exception e) {
+      // OK
+    }
+
     purgeFieldCache(FieldCache.DEFAULT);   // avoid FC insanity
   }