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 2015/02/27 16:53:34 UTC
svn commit: r1662730 - in /lucene/dev/branches/branch_5x: ./ solr/
solr/core/ solr/core/src/java/org/apache/solr/handler/component/
solr/core/src/test/org/apache/solr/handler/component/
Author: shalin
Date: Fri Feb 27 15:53:34 2015
New Revision: 1662730
URL: http://svn.apache.org/r1662730
Log:
SOLR-7128: Make sure fields aren't duplicated in shard requests
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/solr/ (props changed)
lucene/dev/branches/branch_5x/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/solr/core/ (props changed)
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java
Modified: lucene/dev/branches/branch_5x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/CHANGES.txt?rev=1662730&r1=1662729&r2=1662730&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Fri Feb 27 15:53:34 2015
@@ -530,7 +530,7 @@ Bug Fixes
* SOLR-6631: DistributedQueue spinning on calling zookeeper getChildren()
(Jessica Cheng Mallet, Mark Miller, Timothy Potter)
-* SOLR-6579:SnapPuller Replication blocks clean shutdown of tomcat
+* SOLR-6579: SnapPuller Replication blocks clean shutdown of tomcat
(Philip Black-Knight via Noble Paul)
* SOLR-6721: ZkController.ensureReplicaInLeaderInitiatedRecovery puts replica
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java?rev=1662730&r1=1662729&r2=1662730&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java Fri Feb 27 15:53:34 2015
@@ -882,34 +882,40 @@ public class QueryComponent extends Sear
sreq.params.set(ResponseBuilder.FIELD_SORT_VALUES,"true");
- // TODO: should this really sendGlobalDfs if just includeScore?
boolean shardQueryIncludeScore = (rb.getFieldFlags() & SolrIndexSearcher.GET_SCORES) != 0 || rb.getSortSpec().includesScore();
- if (shardQueryIncludeScore) {
- sreq.params.set(CommonParams.FL, rb.req.getSchema().getUniqueKeyField().getName() + ",score");
- StatsCache statsCache = rb.req.getCore().getStatsCache();
- statsCache.sendGlobalStats(rb, sreq);
- } else {
- // reset so that only unique key is requested in shard requests
- sreq.params.set(CommonParams.FL, rb.req.getSchema().getUniqueKeyField().getName());
- }
-
- if (distribSinglePass) {
+ StringBuilder additionalFL = new StringBuilder();
+ boolean additionalAdded = false;
+ if (distribSinglePass) {
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);
+ if (!fields.wantsField(keyFieldName)) {
+ additionalAdded = addFL(additionalFL, keyFieldName, additionalAdded);
+ }
} else {
// ... else we need to explicitly ask for all fields, because we are going to add
// additional fields below
sreq.params.set(CommonParams.FL, "*");
}
+ if (!fields.wantsScore() && shardQueryIncludeScore) {
+ additionalAdded = addFL(additionalFL, "score", additionalAdded);
+ }
+ } else {
+ // reset so that only unique key is requested in shard requests
+ sreq.params.set(CommonParams.FL, rb.req.getSchema().getUniqueKeyField().getName());
+ if (shardQueryIncludeScore) {
+ additionalAdded = addFL(additionalFL, "score", additionalAdded);
+ }
}
- 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);
+
+ // TODO: should this really sendGlobalDfs if just includeScore?
+
+ if (shardQueryIncludeScore) {
+ StatsCache statsCache = rb.req.getCore().getStatsCache();
+ statsCache.sendGlobalStats(rb, sreq);
+ }
+
if (additionalAdded) sreq.params.add(CommonParams.FL, additionalFL.toString());
rb.addRequest(this, sreq);
Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java?rev=1662730&r1=1662729&r2=1662730&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentOptimizationTest.java Fri Feb 27 15:53:34 2015
@@ -132,6 +132,18 @@ public class DistributedQueryComponentOp
// fix for a bug where not all fields are returned if using multiple fl parameters, see SOLR-6796
queryWithAsserts("q", "*:*", "fl", "id", "fl", "dynamic", "sort", "payload desc", ShardParams.DISTRIB_SINGLE_PASS, "true");
+
+ // missing fl with sort
+ queryWithAsserts("q", "*:*", "sort", "payload desc", ShardParams.DISTRIB_SINGLE_PASS, "true");
+ queryWithAsserts("q", "*:*", "sort", "payload desc");
+
+ // fl=*
+ queryWithAsserts("q", "*:*", "fl", "*", "sort", "payload desc", ShardParams.DISTRIB_SINGLE_PASS, "true");
+ queryWithAsserts("q", "*:*", "fl", "*", "sort", "payload desc");
+
+ // fl=*,score
+ queryWithAsserts("q", "*:*", "fl", "*,score", "sort", "payload desc", ShardParams.DISTRIB_SINGLE_PASS, "true");
+ queryWithAsserts("q", "*:*", "fl", "*,score", "sort", "payload desc");
}
/**
@@ -196,7 +208,7 @@ public class DistributedQueryComponentOp
// score is optional, requested only if sorted by score
if (fls.contains("score") || sortFields.contains("score")) idScoreFields.add("score");
- if (idScoreFields.containsAll(fls)) {
+ if (idScoreFields.containsAll(fls) && !fls.isEmpty()) {
// if id and/or score are the only fields being requested then we implicitly turn on distribSinglePass=true
distribSinglePass = true;
}
@@ -259,13 +271,23 @@ public class DistributedQueryComponentOp
fail("Expected non-zero number of '" + paramName + "' parameters in request");
}
Set<String> requestedFields = new HashSet<>();
- for (String p : params) {
- requestedFields.addAll(StrUtils.splitSmart(p, ','));
+ if (params != null) {
+ for (String p : params) {
+ List<String> list = StrUtils.splitSmart(p, ',');
+ for (String s : list) {
+ // make sure field names aren't duplicated in the parameters
+ assertTrue("Field name " + s + " was requested multiple times: params = " + requestAndParams.params,
+ requestedFields.add(s));
+ }
+ }
}
- assertEquals("Number of requested fields do not match with expectations", expectedCount, requestedFields.size());
- for (String field : values) {
- if (!requestedFields.contains(field)) {
- fail("Field " + field + " not found in param: " + paramName + " request had " + paramName + "=" + requestedFields);
+ // if a wildcard ALL field is requested then we don't need to match exact number of params
+ if (!requestedFields.contains("*")) {
+ assertEquals("Number of requested fields do not match with expectations", expectedCount, requestedFields.size());
+ for (String field : values) {
+ if (!requestedFields.contains(field)) {
+ fail("Field " + field + " not found in param: " + paramName + " request had " + paramName + "=" + requestedFields);
+ }
}
}
}