You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2014/12/02 15:04:35 UTC

svn commit: r1642873 - in /lucene/dev/trunk/solr: CHANGES.txt core/src/java/org/apache/solr/handler/component/QueryComponent.java core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java

Author: shalin
Date: Tue Dec  2 14:04:35 2014
New Revision: 1642873

URL: http://svn.apache.org/r1642873
Log:
SOLR-6796: distrib.singlePass does not return correct set of fields for multi-fl-parameter requests

Modified:
    lucene/dev/trunk/solr/CHANGES.txt
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
    lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java

Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1642873&r1=1642872&r2=1642873&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Tue Dec  2 14:04:35 2014
@@ -506,6 +506,9 @@ Bug Fixes
 * SOLR-6795: distrib.singlePass returns score even though not asked for.
   (Per Steffensen via shalin)
 
+* SOLR-6796: distrib.singlePass does not return correct set of fields for multi-fl-parameter
+  requests. (Per Steffensen via shalin)
+
 Other Changes
 ----------------------
 

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java?rev=1642873&r1=1642872&r2=1642873&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java Tue Dec  2 14:04:35 2014
@@ -30,7 +30,6 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.regex.Pattern;
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.IndexReaderContext;
@@ -839,51 +838,33 @@ public class QueryComponent extends Sear
 
     boolean shardQueryIncludeScore = (rb.getFieldFlags() & SolrIndexSearcher.GET_SCORES) != 0 || rb.getSortSpec().includesScore();
     if (distribSinglePass) {
-      String fl = rb.req.getParams().get(CommonParams.FL);
-      if (fl == null) {
-        if (fields.getRequestedFieldNames() == null && fields.wantsAllFields()) {
-          fl = "*";
-        } else  {
-          fl = "";
-          for (String s : fields.getRequestedFieldNames()) {
-            fl += s + ",";
-          }
-        }
-      }
-      if (!fields.wantsField(keyFieldName))  {
-        // the user has not requested the unique key but
-        // we still need to add it otherwise mergeIds can't work
-        if (fl.endsWith(",")) {
-          fl += keyFieldName;
-        } else  {
-          fl += "," + keyFieldName;
-        }
-      }
-      sreq.params.set(CommonParams.FL, updateFl(fl, shardQueryIncludeScore));
-    } else {
-      // in this first phase, request only the unique key field and any fields needed for merging.
-      if (shardQueryIncludeScore) {
-        sreq.params.set(CommonParams.FL, keyFieldName + ",score");
+      String[] fls = rb.req.getParams().getParams(CommonParams.FL);
+      if (fls != null && fls.length > 0 && (fls.length != 1 || !fls[0].isEmpty())) {
+        // If the outer request contains actual FL's use them...
+        sreq.params.set(CommonParams.FL, fls);
       } else {
-        sreq.params.set(CommonParams.FL, keyFieldName);
+        // ... else we need to explicitly ask for all fields, because we are going to add
+        // additional fields below
+        sreq.params.set(CommonParams.FL, "*");
       }
     }
+    StringBuilder additionalFL = new StringBuilder();
+    boolean additionalAdded = false;
+    if (!distribSinglePass || !fields.wantsField(keyFieldName)) 
+      additionalAdded = addFL(additionalFL, keyFieldName, additionalAdded);
+    if ((!distribSinglePass || !fields.wantsScore()) && shardQueryIncludeScore) 
+      additionalAdded = addFL(additionalFL, "score", additionalAdded);
+    if (additionalAdded) sreq.params.add(CommonParams.FL, additionalFL.toString());
 
     rb.addRequest(this, sreq);
   }
-
-
-  String updateFl(String originalFields, boolean includeScoreIfMissing) {
-    if (includeScoreIfMissing && !scorePattern.matcher(originalFields).find()) {
-      return originalFields + ",score";
-    } else {
-      return originalFields;
-    }
+  
+  private boolean addFL(StringBuilder fl, String field, boolean additionalAdded) {
+    if (additionalAdded) fl.append(",");
+    fl.append(field);
+    return true;
   }
 
-  private static final Pattern scorePattern = Pattern.compile("\\bscore\\b");
-
-
   private void mergeIds(ResponseBuilder rb, ShardRequest sreq) {
       List<MergeStrategy> mergeStrategies = rb.getMergeStrategies();
       if(mergeStrategies != null) {

Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java?rev=1642873&r1=1642872&r2=1642873&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java Tue Dec  2 14:04:35 2014
@@ -118,12 +118,16 @@ public class DistributedQueryComponentOp
     verifySinglePass("q", "id:19", "fl", "id,*a_sS", "sort", "payload asc", "distrib.singlePass", "true");
     verifySinglePass("q", "id:19", "fl", "id,dynamic,cat*", "sort", "payload asc", "distrib.singlePass", "true");
 
+    // see SOLR-6795, distrib.singlePass=true would return score even when not asked for
     handle.clear();
     handle.put("timestamp", SKIPVAL);
     handle.put("_version_", SKIPVAL);
     // we don't to compare maxScore because most distributed requests return it anyway (just because they have score already)
     handle.put("maxScore", SKIPVAL);
     query("q", "{!func}id", ShardParams.DISTRIB_SINGLE_PASS, "true");
+
+    // fix for a bug where not all fields are returned if using multiple fl parameters, see SOLR-6796
+    query("q","*:*", "fl", "id", "fl","dynamic","sort","payload desc", ShardParams.DISTRIB_SINGLE_PASS, "true");
   }
 
   private void verifySinglePass(String... q) throws SolrServerException {