You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2015/12/18 03:21:27 UTC
svn commit: r1720715 - 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: dsmiley
Date: Fri Dec 18 02:21:27 2015
New Revision: 1720715
URL: http://svn.apache.org/viewvc?rev=1720715&view=rev
Log:
SOLR-8419: TermVectorComponent: fix accidental inclusion of docs when distrib.singlePass. Remove 'uniqueKeyField' from response. distrib now requires a schema unique key.
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/TermVectorComponent.java
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentDistributedTest.java
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentTest.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=1720715&r1=1720714&r2=1720715&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Fri Dec 18 02:21:27 2015
@@ -65,6 +65,9 @@ Bug Fixes
and then that replica is brought down, it will think it is up-to-date when
restarted. (shalin, Mark Miller, yonik)
+* SOLR-8419: TermVectorComponent for distributed search when distrib.singlePass could include term
+ vectors for documents that matched the query yet weren't in the returned documents. (David Smiley)
+
Other Changes
----------------------
@@ -139,6 +142,9 @@ Other Changes
* SOLR-8410: Add all read paths to 'read' permission in RuleBasedAuthorizationPlugin (noble)
+* SOLR-8419: TermVectorComponent for distributed search now requires a uniqueKey in the schema. Also, it no longer
+ returns "uniqueKeyField" in the response. (David Smiley)
+
================== 5.4.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java?rev=1720715&r1=1720714&r2=1720715&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java Fri Dec 18 02:21:27 2015
@@ -83,9 +83,12 @@ public class TermVectorComponent extends
public static final String COMPONENT_NAME = "tv";
- protected NamedList initParams;
public static final String TERM_VECTORS = "termVectors";
+ private static final String TV_KEY_WARNINGS = "warnings";
+
+ protected NamedList initParams;
+
/**
* Helper method for determining the list of fields that we should
* try to find term vectors on.
@@ -147,7 +150,6 @@ public class TermVectorComponent extends
String uniqFieldName = null;
if (keyField != null) {
uniqFieldName = keyField.getName();
- termVectors.add("uniqueKeyFieldName", uniqFieldName);
}
FieldOptions allFields = new FieldOptions();
@@ -182,7 +184,7 @@ public class TermVectorComponent extends
//we have specific fields to retrieve, or no fields
for (String field : fields) {
- // workarround SOLR-3523
+ // workaround SOLR-3523
if (null == field || "score".equals(field)) continue;
// we don't want to issue warnings about the uniqueKey field
@@ -226,29 +228,24 @@ public class TermVectorComponent extends
}
} //else, deal with all fields
- // NOTE: currently all typs of warnings are schema driven, and garunteed
+ // NOTE: currently all types of warnings are schema driven, and guaranteed
// to be consistent across all shards - if additional types of warnings
- // are added that might be differnet between shards, finishStage() needs
+ // are added that might be different between shards, finishStage() needs
// to be changed to account for that.
- boolean hasWarnings = false;
if (!noTV.isEmpty()) {
warnings.add("noTermVectors", noTV);
- hasWarnings = true;
}
if (!noPos.isEmpty()) {
warnings.add("noPositions", noPos);
- hasWarnings = true;
}
if (!noOff.isEmpty()) {
warnings.add("noOffsets", noOff);
- hasWarnings = true;
}
if (!noPay.isEmpty()) {
warnings.add("noPayloads", noPay);
- hasWarnings = true;
}
- if (hasWarnings) {
- termVectors.add("warnings", warnings);
+ if (warnings.size() > 0) {
+ termVectors.add(TV_KEY_WARNINGS, warnings);
}
DocListAndSet listAndSet = rb.getResults();
@@ -442,7 +439,7 @@ public class TermVectorComponent extends
public void finishStage(ResponseBuilder rb) {
if (rb.stage == ResponseBuilder.STAGE_GET_FIELDS) {
- NamedList<Object> termVectors = new NamedList<>();
+ NamedList<Object> termVectorsNL = new NamedList<>();
Map.Entry<String, Object>[] arr = new NamedList.NamedListEntry[rb.resultIds.size()];
for (ShardRequest sreq : rb.finished) {
@@ -451,15 +448,19 @@ public class TermVectorComponent extends
}
for (ShardResponse srsp : sreq.responses) {
NamedList<Object> nl = (NamedList<Object>)srsp.getSolrResponse().getResponse().get(TERM_VECTORS);
+
+ // Add metadata (that which isn't a uniqueKey value):
+ Object warningsNL = nl.get(TV_KEY_WARNINGS);
+ // assume if that if warnings is already present; we don't need to merge.
+ if (warningsNL != null && termVectorsNL.indexOf(TV_KEY_WARNINGS, 0) < 0) {
+ termVectorsNL.add(TV_KEY_WARNINGS, warningsNL);
+ }
+
+ // UniqueKey data
for (int i=0; i < nl.size(); i++) {
String key = nl.getName(i);
ShardDoc sdoc = rb.resultIds.get(key);
- if (null == sdoc) {
- // metadata, only need from one node, leave in order
- if (termVectors.indexOf(key,0) < 0) {
- termVectors.add(key, nl.getVal(i));
- }
- } else {
+ if (sdoc != null) {// can be null when rb.onePassDistributedQuery
int idx = sdoc.positionInResponse;
arr[idx] = new NamedList.NamedListEntry<>(key, nl.getVal(i));
}
@@ -467,8 +468,8 @@ public class TermVectorComponent extends
}
}
// remove nulls in case not all docs were able to be retrieved
- termVectors.addAll(SolrPluginUtils.removeNulls(arr, new NamedList<Object>()));
- rb.rsp.add(TERM_VECTORS, termVectors);
+ SolrPluginUtils.removeNulls(arr, termVectorsNL);
+ rb.rsp.add(TERM_VECTORS, termVectorsNL);
}
}
Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentDistributedTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentDistributedTest.java?rev=1720715&r1=1720714&r2=1720715&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentDistributedTest.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentDistributedTest.java Fri Dec 18 02:21:27 2015
@@ -20,6 +20,7 @@ package org.apache.solr.handler.componen
import org.apache.lucene.util.Constants;
import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.common.params.ShardParams;
import org.apache.solr.common.params.TermVectorParams;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -188,7 +189,15 @@ public class TermVectorComponentDistribu
"q", q,
TermVectorComponent.COMPONENT_NAME, "true",
TermVectorParams.ALL, "true");
-
+
+ query("sort", "id desc",
+ "qt",tv,
+ "q", q,
+ "rows", 1,
+ ShardParams.DISTRIB_SINGLE_PASS, "true",
+ TermVectorComponent.COMPONENT_NAME, "true",
+ TermVectorParams.ALL, "true");
+
// per field stuff
query("sort", "id desc",
Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentTest.java?rev=1720715&r1=1720714&r2=1720715&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentTest.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/component/TermVectorComponentTest.java Fri Dec 18 02:21:27 2015
@@ -130,8 +130,7 @@ public class TermVectorComponentTest ext
" 'test_offtv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
" 'test_posofftv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
" 'test_posoffpaytv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
- " 'test_postv':{'anoth':{'tf':1},'titl':{'tf':2}}}," +
- " 'uniqueKeyFieldName':'id'}"
+ " 'test_postv':{'anoth':{'tf':1},'titl':{'tf':2}}}}"
);
// tv.fl diff from fl
assertJQ(req("json.nl","map",
@@ -143,8 +142,7 @@ public class TermVectorComponentTest ext
TermVectorParams.TF, "true")
,"/termVectors=={'0':{'uniqueKey':'0'," +
" 'test_basictv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
- " 'test_offtv':{'anoth':{'tf':1},'titl':{'tf':2}}}," +
- " 'uniqueKeyFieldName':'id'}"
+ " 'test_offtv':{'anoth':{'tf':1},'titl':{'tf':2}}}}"
);
// multi-valued tv.fl
assertJQ(req("json.nl","map",
@@ -157,8 +155,7 @@ public class TermVectorComponentTest ext
TermVectorParams.TF, "true")
,"/termVectors=={'0':{'uniqueKey':'0'," +
" 'test_basictv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
- " 'test_offtv':{'anoth':{'tf':1},'titl':{'tf':2}}}," +
- " 'uniqueKeyFieldName':'id'}"
+ " 'test_offtv':{'anoth':{'tf':1},'titl':{'tf':2}}}}"
);
// re-use fl glob
assertJQ(req("json.nl","map",
@@ -172,8 +169,7 @@ public class TermVectorComponentTest ext
" 'test_offtv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
" 'test_posofftv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
" 'test_posoffpaytv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
- " 'test_postv':{'anoth':{'tf':1},'titl':{'tf':2}}}," +
- " 'uniqueKeyFieldName':'id'}"
+ " 'test_postv':{'anoth':{'tf':1},'titl':{'tf':2}}}}"
);
// re-use fl, ignore things we can't handle
assertJQ(req("json.nl","map",
@@ -184,8 +180,7 @@ public class TermVectorComponentTest ext
TermVectorParams.TF, "true")
,"/termVectors=={'0':{'uniqueKey':'0'," +
" 'test_basictv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
- " 'test_postv':{'anoth':{'tf':1},'titl':{'tf':2}}}," +
- " 'uniqueKeyFieldName':'id'}"
+ " 'test_postv':{'anoth':{'tf':1},'titl':{'tf':2}}}}"
);
// re-use (multi-valued) fl, ignore things we can't handle
assertJQ(req("json.nl","map",
@@ -197,8 +192,7 @@ public class TermVectorComponentTest ext
TermVectorParams.TF, "true")
,"/termVectors=={'0':{'uniqueKey':'0'," +
" 'test_basictv':{'anoth':{'tf':1},'titl':{'tf':2}}," +
- " 'test_postv':{'anoth':{'tf':1},'titl':{'tf':2}}}," +
- " 'uniqueKeyFieldName':'id'}"
+ " 'test_postv':{'anoth':{'tf':1},'titl':{'tf':2}}}}"
);
}