dsmiley commented on code in PR #3343:
URL: https://github.com/apache/solr/pull/3343#discussion_r2078575879


##########
solr/core/src/java/org/apache/solr/search/SortSpecParsing.java:
##########
@@ -220,6 +185,40 @@ private static SortSpec parseSortSpecImpl(
     return new SortSpec(s, fields);
   }
 
+  static ValueSource parseValueSource(QParser parser, StrParser sp, int start) 
throws SyntaxError {

Review Comment:
   de-duplicated this code, and now using `fparser.parseAsValueSource`.  Only 2 
callers within Solr so using package scope / internal.



##########
solr/core/src/java/org/apache/solr/search/QParser.java:
##########
@@ -326,6 +331,21 @@ public int getPrefixQueryMinPrefixLength() {
     return getReq().getCore().getSolrConfig().prefixQueryMinPrefixLength;
   }
 
+  /**
+   * Parse the string into a {@link ValueSource} <em>instead of a {@link 
Query}</em>. Solr calls
+   * this in most places that "function queries" go. Overridden by {@link 
FunctionQParser}.
+   */
+  public ValueSource parseAsValueSource() throws SyntaxError {
+    Query q = getQuery();
+    return switch (q) {

Review Comment:
   I'm loving Java 21



##########
solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java:
##########
@@ -532,13 +531,11 @@ protected List<ValueSource> getMultiplicativeBoosts() 
throws SyntaxError {
     List<ValueSource> boosts = new ArrayList<>();
     if (config.hasMultiplicativeBoosts()) {
       for (String boostStr : config.multBoosts) {
-        if (boostStr == null || boostStr.length() == 0) continue;
-        Query boost = subQuery(boostStr, 
FunctionQParserPlugin.NAME).getQuery();
-        ValueSource vs;
-        if (boost instanceof FunctionQuery) {
-          vs = ((FunctionQuery) boost).getValueSource();
-        } else {
-          vs = new QueryValueSource(boost, 1.0f);
+        if (boostStr == null || boostStr.isEmpty()) continue;
+        ValueSource vs = subQuery(boostStr, 
FunctionQParserPlugin.NAME).parseAsValueSource();
+        // the default score should be 1, not 0
+        if (vs instanceof QueryValueSource qvs && qvs.getDefaultValue() == 
0.0f) {
+          vs = new QueryValueSource(qvs.getQuery(), 1.0f);

Review Comment:
   not pretty but only had to do this here, so maybe doesn't warrant anything 
more elegant



##########
solr/core/src/java/org/apache/solr/search/FunctionQParser.java:
##########
@@ -91,6 +106,18 @@ public boolean getParseToEnd() {
 
   @Override
   public Query parse() throws SyntaxError {
+    return new FunctionQuery(parseAsValueSource());
+  }
+
+  /**
+   * Parses as a ValueSource, not a Query. <em>NOT</em> intended to be called 
by {@link
+   * ValueSourceParser#parse(FunctionQParser)}; it's intended for general code 
that has a {@link
+   * QParser} but actually wants to parse a ValueSource.
+   *
+   * @return A {@link VectorValueSource} for multiple VS, otherwise just the 
single VS.
+   */
+  @Override
+  public ValueSource parseAsValueSource() throws SyntaxError {

Review Comment:
   During dev I once used the existing method `parseValueSource()` but that 
provided problematic as there are different expectations between VSP callers 
and QP callers.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to