SOLR-7128: Make sure fields aren't duplicated in shard requests

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1662729 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Shalin Shekhar Mangar 2015-02-27 15:50:07 +00:00
parent 544f5bf1e7
commit 3cf40619ca
3 changed files with 53 additions and 25 deletions

View File

@ -583,7 +583,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

View File

@ -882,34 +882,40 @@ public class QueryComponent extends SearchComponent
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);

View File

@ -132,6 +132,18 @@ public class DistributedQueryComponentOptimizationTest extends AbstractFullDistr
// 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 DistributedQueryComponentOptimizationTest extends AbstractFullDistr
// 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 DistributedQueryComponentOptimizationTest extends AbstractFullDistr
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);
}
}
}
}