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}}}}"
     );
 
   }