SOLR-18314: Correct pf boost handling for single-term and clause-less queries

changelog/unreleased/SOLR-18314-empty-phrase-boost-matchalldocs.yml

@@ -0,0 +1,11 @@
+title: Fix the eDisMax parser adding an empty phrase field (pf) boost clause when no phrase qualifies
+  for the boost, such as for single-term queries or q=*:*. The empty SHOULD clause did not affect
+  scoring but defeated the single-clause MatchAllDocsQuery optimization; it is now omitted, so q=*:*
+  reduces to a MatchAllDocsQuery even when pf is set.
+type: fixed
+authors:
+  - name: Ian Springer
+    nick: ianspringer
+links:
+  - name: SOLR-18314
+    url: https://issues.apache.org/jira/browse/SOLR-18314

changelog/unreleased/SOLR-18314-skip-phrase-analysis-single-term.yml

@@ -0,0 +1,13 @@
+title: The DisMax parser no longer applies the phrase field (pf) boost to single-term queries, matching
+  the eDisMax parser. A query is single-term when it is one whitespace-free term that analyzes to a
+  single token in every pf field. A term whose analysis yields multiple tokens (e.g. via CJK, ngram,
+  WordDelimiterGraph, or multi-word synonyms) keeps its phrase boost, as does a multi-word query on
+  a keyword or string pf field. This may change relevancy scores for single-term DisMax queries,
+  including those whose pf fields are not also in qf.
+type: changed
+authors:
+  - name: Ian Springer
+    nick: ianspringer
+links:
+  - name: SOLR-18314
+    url: https://issues.apache.org/jira/browse/SOLR-18314

solr/core/src/java/org/apache/solr/search/DisMaxQParser.java

@@ -16,13 +16,19 @@
  */
 package org.apache.solr.search;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.BoostQuery;
+import org.apache.lucene.search.DisjunctionMaxQuery;
+import org.apache.lucene.search.MultiPhraseQuery;
+import org.apache.lucene.search.PhraseQuery;
 import org.apache.lucene.search.Query;
+import org.apache.lucene.search.SynonymQuery;
+import org.apache.lucene.search.TermQuery;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.DisMaxParams;
 import org.apache.solr.common.params.SolrParams;
@@ -198,7 +204,12 @@ protected boolean addMainQuery(BooleanQuery.Builder query, SolrParams solrParams
       query.add(parsedUserQuery, BooleanClause.Occur.MUST);
 
       Query phrase = getPhraseQuery(userQuery, pp);
-      if (null != phrase) {
+      // A single-term phrase boost imposes no adjacency constraint, so skip it, as eDisMax does.
+      // Require both a single user term and a single analyzed term: analyzers such as CJK, ngram,
+      // WordDelimiterGraph, and multi-word synonyms can split one input term into several tokens,
+      // and a multi-word query can analyze to one token (e.g. an exact-match keyword or string
+      // field). Either way the phrase boost is meaningful.
+      if (null != phrase && !(isSingleUserTerm(userQuery) && isEffectivelySingleTerm(phrase))) {
         query.add(phrase, BooleanClause.Occur.SHOULD);
       }
     }
@@ -235,6 +246,45 @@ protected Query getPhraseQuery(String userQuery, SolrPluginUtils.DisjunctionMaxQ
     return phrase;
   }
 
+  /** Returns true if the user query, ignoring quotes, is a single whitespace-free term. */
+  private static boolean isSingleUserTerm(String userQuery) {
+    String term = userQuery.replace("\"", "").strip();
+    return !term.isEmpty() && term.codePoints().noneMatch(Character::isWhitespace);
+  }
+
+  /**
+   * Returns true if the parsed phrase query boils down to a single analyzed position in every
+   * field, in which case the phrase boost imposes no adjacency constraint. The parser produces a
+   * {@link DisjunctionMaxQuery} over per-field {@link BoostQuery}-wrapped queries; we inspect the
+   * underlying query shape so that analyzers which split a single input term into multiple tokens
+   * keep their phrase boost. Synonyms at a single position count as a single term when built as a
+   * {@link SynonymQuery} ({@code synonymQueryStyle=as_same_term}) or {@link DisjunctionMaxQuery}
+   * ({@code pick_best}); the {@link BooleanQuery} that {@code as_distinct_terms} builds is kept, as
+   * eDisMax's {@code minClauseSize} gate also keeps it. Unrecognized shapes (and {@code null})
+   * conservatively return false (keep the boost).
+   */
+  @VisibleForTesting
+  static boolean isEffectivelySingleTerm(Query query) {
+    if (query instanceof BoostQuery boostQuery) {
+      return isEffectivelySingleTerm(boostQuery.getQuery());
+    }
+    if (query instanceof DisjunctionMaxQuery dmq) {
+      // Every field's clause is a single term (or synonymQueryStyle=pick_best synonyms).
+      return !dmq.getDisjuncts().isEmpty()
+          && dmq.getDisjuncts().stream().allMatch(DisMaxQParser::isEffectivelySingleTerm);
+    }
+    if (query instanceof TermQuery || query instanceof SynonymQuery) {
+      return true;
+    }
+    if (query instanceof PhraseQuery phraseQuery) {
+      return phraseQuery.getTerms().length < 2;
+    }
+    if (query instanceof MultiPhraseQuery multiPhraseQuery) {
+      return multiPhraseQuery.getTermArrays().length < 2;
+    }
+    return false;
+  }
+
   protected Query getUserQuery(
       String userQuery, SolrPluginUtils.DisjunctionMaxQueryParser up, SolrParams solrParams)
       throws SyntaxError {

solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java

@@ -625,8 +625,11 @@ protected void addShingledPhraseQueries(
       final int slop)
       throws SyntaxError {
 
-    if (null == fields || fields.isEmpty() || null == clauses || clauses.size() < shingleSize)
-      return;
+    if (null == fields
+        || fields.isEmpty()
+        || null == clauses
+        || clauses.isEmpty()
+        || clauses.size() < shingleSize) return;
 
     if (0 == shingleSize) shingleSize = clauses.size();
 
@@ -676,7 +679,10 @@ protected void addShingledPhraseQueries(
     // TODO: perhaps we shouldn't use synonyms either...
 
     Query phrase = pp.parse(userPhraseQuery.toString());
-    if (phrase != null) {
+    // When minClauseSize discards every phrase (e.g. single-term queries or q=*:*), parse returns
+    // an empty BooleanQuery rather than null; adding it as a SHOULD clause would defeat the
+    // single-clause MatchAllDocsQuery optimization applied to the main query.
+    if (!isEmpty(phrase)) {
       mainQuery.add(phrase, BooleanClause.Occur.SHOULD);
     }
   }

solr/core/src/test-files/solr/collection1/conf/schema.xml

@@ -312,6 +312,17 @@
     </analyzer>
   </fieldType>
 
+  <!-- WordDelimiter splitting like subword, but with the schema 1.4+ default of autoGeneratePhraseQueries="false" -->
+  <fieldType name="subword_noautophrase" class="solr.TextField" autoGeneratePhraseQueries="false">
+    <analyzer>
+      <tokenizer class="solr.MockTokenizerFactory"/>
+      <filter class="solr.WordDelimiterGraphFilterFactory" generateWordParts="1" generateNumberParts="1" catenateWords="0"
+              catenateNumbers="0" catenateAll="0"/>
+      <filter class="solr.FlattenGraphFilterFactory"/>
+      <filter class="solr.LowerCaseFilterFactory"/>
+    </analyzer>
+  </fieldType>
+
   <fieldType name="numericsubword" class="solr.TextField" multiValued="true" positionIncrementGap="100">
     <analyzer type="index">
       <tokenizer class="solr.MockTokenizerFactory"/>
@@ -632,7 +643,9 @@
   <field name="price" type="float" indexed="true" stored="true" multiValued="false"/>
   <field name="inStock" type="boolean" indexed="true" stored="true"/>
 
+  <field name="text_syn" type="lowerpunctfilt" indexed="true" stored="true"/>
   <field name="subword" type="subword" indexed="true" stored="true"/>
+  <field name="subword_noautophrase" type="subword_noautophrase" indexed="true" stored="true"/>
   <field name="subword_offsets" type="subword" indexed="true" stored="true" termVectors="true" termOffsets="true"/>
   <field name="numericsubword" type="numericsubword" indexed="true" stored="true"/>
   <field name="protectedsubword" type="protectedsubword" indexed="true" stored="true"/>

solr/core/src/test-files/solr/collection1/conf/synonyms.txt

@@ -32,6 +32,7 @@ pixima => pixma
 # multiword synonyms
 wi fi => wifi
 crow blackbird, grackle
+usa => united states of america
 
 # Synonyms used in semantic expansion
 tabby => tabby, cat, feline, animal

solr/core/src/test/org/apache/solr/DisMaxRequestHandlerTest.java

@@ -245,4 +245,126 @@ public void testExtraBlankBQ() throws Exception {
     assertTrue(p.matcher(resp).find());
     assertTrue(p_bool.matcher(resp).find());
   }
+
+  // SOLR-18314: the pf phrase boost is dropped for single-term queries, i.e. a single user term
+  // that also analyzes to a single token. A single input term whose analysis yields multiple tokens
+  // must keep its phrase boost, whether from WordDelimiter splitting "wi-fi" into "wi fi" or a
+  // multi-word synonym expanding "usa" into "united states of america". So must a multi-word query
+  // that analyzes to a single token on a string pf field (an exact-match boost). Unlike eDisMax,
+  // the classic DisMax parser has no minClauseSize gate, so this is enforced by inspecting the
+  // user query and the parsed phrase query.
+  //
+  // CJK is an important example of a query with no spaces that can still parse to multiple terms.
+  // However, we don't add a dedicated CJK test: CJK analysis of a single input term produces the
+  // same multi-term PhraseQuery that the "wi-fi" case already exercises, so it would cover no new
+  // code path while requiring a CJK analyzer to be configured in the test schema.
+  @Test
+  public void testSingleTermPhraseBoost() throws Exception {
+    // Single analyzed token: the pf phrase boost imposes no adjacency, so only the unboosted qf
+    // clause should appear in the parsed query.
+    String resp =
+        h.query(
+            req(
+                "q",
+                "wireless",
+                "qt",
+                "/dismax",
+                "qf",
+                "subword",
+                "pf",
+                "subword^10",
+                CommonParams.DEBUG_QUERY,
+                "true"));
+    assertTrue(
+        "single-term query should keep its qf clause",
+        Pattern.compile("\\(subword:wireless\\)(?!\\^)").matcher(resp).find());
+    assertFalse(
+        "single-term query should not produce a phrase boost",
+        Pattern.compile("subword:wireless\\)\\^10\\.0").matcher(resp).find());
+
+    // Whitespace-free query that analyzes into multiple tokens: the phrase boost is meaningful and
+    // must be retained. The qf clause is also the phrase "wi fi" here (this schema's version
+    // defaults autoGeneratePhraseQueries to true), so match the pf clause by the handler's ps=100
+    // slop and the pf boost.
+    resp =
+        h.query(
+            req(
+                "q",
+                "wi-fi",
+                "qt",
+                "/dismax",
+                "qf",
+                "subword",
+                "pf",
+                "subword^10",
+                CommonParams.DEBUG_QUERY,
+                "true"));
+    assertTrue(
+        "multi-token query should retain its phrase boost",
+        Pattern.compile("subword:\"wi fi\"~100\\)\\^10\\.0").matcher(resp).find());
+
+    // With autoGeneratePhraseQueries="false" (the default since schema 1.4), the qf clause for
+    // "wi-fi" is a plain disjunction of its tokens, so the pf boost is the only adjacency signal.
+    // Gating on the user query's clause count (one, since there's no whitespace) would wrongly drop
+    // it; this is also the shape of CJK bigram queries.
+    resp =
+        h.query(
+            req(
+                "q",
+                "wi-fi",
+                "qt",
+                "/dismax",
+                "qf",
+                "subword_noautophrase",
+                "pf",
+                "subword_noautophrase^10",
+                CommonParams.DEBUG_QUERY,
+                "true"));
+    assertTrue(
+        "qf clause should not be a phrase without autoGeneratePhraseQueries",
+        Pattern.compile("subword_noautophrase:wi subword_noautophrase:fi").matcher(resp).find());
+    assertTrue(
+        "multi-token query should retain its phrase boost without autoGeneratePhraseQueries",
+        Pattern.compile("subword_noautophrase:\"wi fi\"~100\\)\\^10\\.0").matcher(resp).find());
+
+    // A single input term expanded by a multi-word synonym ("usa" => "united states of america")
+    // likewise analyzes into multiple tokens, so its phrase boost must be retained.
+    resp =
+        h.query(
+            req(
+                "q",
+                "usa",
+                "qt",
+                "/dismax",
+                "qf",
+                "text_syn",
+                "pf",
+                "text_syn^10",
+                CommonParams.DEBUG_QUERY,
+                "true"));
+    assertTrue(
+        "multi-word synonym query should retain its phrase boost",
+        Pattern.compile("text_syn:\"united states of america\"~100\\)\\^10\\.0")
+            .matcher(resp)
+            .find());
+
+    // A multi-word query on a string pf field analyzes to a single exact-match term, which is the
+    // only signal ranking an exact match first, so its phrase boost must be retained.
+    resp =
+        h.query(
+            req(
+                "q",
+                "hello world",
+                "qt",
+                "/dismax",
+                "qf",
+                "subword",
+                "pf",
+                "foo_s^10",
+                CommonParams.DEBUG_QUERY,
+                "true"));
+    assertTrue(
+        "multi-word query should retain its exact-match phrase boost",
+        Pattern.compile("foo_s:hello world\\)\\^10\\.0").matcher(resp).find());
+  }
 }