You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by yo...@apache.org on 2012/10/27 07:29:53 UTC

svn commit: r1402742 - in /lucene/dev/trunk/solr: CHANGES.txt core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java core/src/test/org/apache/solr/update/TestUpdate.java

Author: yonik
Date: Sat Oct 27 05:29:53 2012
New Revision: 1402742

URL: http://svn.apache.org/viewvc?rev=1402742&view=rev
Log:
SOLR-3998: Atomic update on uniqueKey field itself causes duplicate document

Modified:
    lucene/dev/trunk/solr/CHANGES.txt
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
    lucene/dev/trunk/solr/core/src/test/org/apache/solr/update/TestUpdate.java

Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1402742&r1=1402741&r2=1402742&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Sat Oct 27 05:29:53 2012
@@ -122,6 +122,9 @@ Bug Fixes
 * SOLR-3995: Recovery may never finish on SolrCore shutdown if the last reference to 
   a SolrCore is closed by the recovery process. (Mark Miller)
 
+* SOLR-3998: Atomic update on uniqueKey field itself causes duplicate document.
+  (Eric Spencer, yonik)
+
 Other Changes
 ----------------------
 

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java?rev=1402742&r1=1402741&r2=1402742&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java Sat Oct 27 05:29:53 2012
@@ -603,11 +603,15 @@ public class DistributedUpdateProcessor 
         for (Entry<String,Object> entry : ((Map<String,Object>) val).entrySet()) {
           String key = entry.getKey();
           Object fieldVal = entry.getValue();
+          boolean updateField = false;
           if ("add".equals(key)) {
+            updateField = true;
             oldDoc.addField( sif.getName(), fieldVal, sif.getBoost());
           } else if ("set".equals(key)) {
+            updateField = true;
             oldDoc.setField(sif.getName(),  fieldVal, sif.getBoost());
           } else if ("inc".equals(key)) {
+            updateField = true;
             SolrInputField numericField = oldDoc.get(sif.getName());
             if (numericField == null) {
               oldDoc.setField(sif.getName(),  fieldVal, sif.getBoost());
@@ -636,6 +640,12 @@ public class DistributedUpdateProcessor 
             }
 
           }
+
+          // validate that the field being modified is not the id field.
+          if (updateField && idField.getName().equals(sif.getName())) {
+            throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid update of id field: " + sif);
+          }
+
         }
       } else {
         // normal fields are treated as a "set"

Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/update/TestUpdate.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/update/TestUpdate.java?rev=1402742&r1=1402741&r2=1402742&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/update/TestUpdate.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/update/TestUpdate.java Sat Oct 27 05:29:53 2012
@@ -16,34 +16,12 @@
  */
 package org.apache.solr.update;
 
-
-import org.apache.lucene.analysis.MockAnalyzer;
-import org.apache.lucene.document.Document;
-import org.apache.lucene.document.Field;
-import org.apache.lucene.document.FieldType;
-import org.apache.lucene.index.*;
-import org.apache.lucene.search.DocIdSetIterator;
-import org.apache.lucene.search.TermQuery;
-import org.apache.lucene.store.Directory;
-import org.apache.lucene.util.BytesRef;
-import org.apache.noggit.ObjectBuilder;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.SolrInputDocument;
-import org.apache.solr.request.SolrQueryRequest;
-import org.apache.solr.util.TestHarness;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-import java.io.IOException;
-import java.util.*;
-import java.util.concurrent.*;
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
-
-import static org.apache.solr.core.SolrCore.verbose;
+import java.util.concurrent.Callable;
 
 public class TestUpdate extends SolrTestCaseJ4 {
   @BeforeClass
@@ -220,6 +198,49 @@ public class TestUpdate extends SolrTest
         ,"=={'doc':{'id':'1', 'val_i':5, 'val_is':[1999999996], 'val2_i':-2000000004, 'val2_f':1.0E20, 'val2_d':-1.2345678901e+100, 'val2_l':4999999996}}"
     );
 
+
+    // remove some fields
+    version = addAndGetVersion(sdoc(
+        "id", "1",
+        "val_is", map("set",null),
+        "val2_f", map("set",null)
+    ),
+        null);
+
+    afterUpdate.call();
+
+    assertJQ(req("qt","/get", "id","1", "fl","id,val*")
+        ,"=={'doc':{'id':'1', 'val_i':5, 'val2_i':-2000000004, 'val2_d':-1.2345678901e+100, 'val2_l':4999999996}}"
+    );
+
+    // test that updating a unique id results in failure.
+    try {
+      ignoreException("Invalid update of id field");
+      version = addAndGetVersion(sdoc(
+          "id", map("set","1"),
+          "val_is", map("inc","2000000000")
+      ),
+          null);
+
+      fail();
+    } catch (SolrException se) {
+      resetExceptionIgnores();
+      assertEquals(400, se.code());
+      assertTrue(se.getMessage().indexOf("Invalid update of id field") >= 0);
+    }
+
+    afterUpdate.call();
+
+    assertJQ(req("qt","/get", "id","1", "fl","id,val*")
+        ,"=={'doc':{'id':'1', 'val_i':5, 'val2_i':-2000000004, 'val2_d':-1.2345678901e+100, 'val2_l':4999999996}}"
+    );
+
+   // nothing should have changed - check with a normal query that we didn't create a duplicate
+    assertU(commit("softCommit","false"));
+    assertJQ(req("q","id:1", "fl","id")
+        ,"/response/numFound==1"
+    );
+
   }
 
 }