You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mu...@apache.org on 2019/06/24 09:42:17 UTC
[lucene-solr] branch branch_8x updated: SOLR-12127: set op with
null or empty list val should be atomic update
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new cf6c81c SOLR-12127: set op with null or empty list val should be atomic update
cf6c81c is described below
commit cf6c81c26be110acb4c3b1225d79adbb15d491c4
Author: Munendra S N <mu...@apache.org>
AuthorDate: Mon Jun 24 14:41:02 2019 +0530
SOLR-12127: set op with null or empty list val should be atomic update
* Inplace update supports set and inc operation but when null or
empty list is specified with set op, then it should always be treated
as atomic update since this case is equivalent to removing field
from the document
---
solr/CHANGES.txt | 3 ++
.../processor/AtomicUpdateDocumentMerger.java | 12 +++++--
.../solr/update/TestInPlaceUpdatesDistrib.java | 41 ++++++++++++++++++++++
.../solr/update/TestInPlaceUpdatesStandalone.java | 21 +++++++++++
4 files changed, 74 insertions(+), 3 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index f18ac12..6496beb 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -136,6 +136,9 @@ Bug Fixes
* SOLR-13545: ContentStreamUpdateRequest refused to close file (Colvin Cowie, Mikhail Khludnev)
+* SOLR-12127: Updates containing 'set' operation with value null or empty list should be considered as Atomic Update
+ (Oliver Kuldmäe, Munendra S N, Ishan Chattopadhyaya)
+
Other Changes
----------------------
diff --git a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
index 5b190c1..4329ae9 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
@@ -16,8 +16,6 @@
*/
package org.apache.solr.update.processor;
-import static org.apache.solr.common.params.CommonParams.ID;
-
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
@@ -59,6 +57,8 @@ import org.apache.solr.util.RefCounted;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.solr.common.params.CommonParams.ID;
+
/**
* @lucene.experimental
*/
@@ -194,10 +194,16 @@ public class AtomicUpdateDocumentMerger {
return Collections.emptySet();
}
// else it's a atomic update map...
- for (String op : ((Map<String, Object>)fieldValue).keySet()) {
+ Map<String, Object> fieldValueMap = (Map<String, Object>)fieldValue;
+ for (String op : fieldValueMap.keySet()) {
+ Object obj = fieldValueMap.get(op);
if (!op.equals("set") && !op.equals("inc")) {
// not a supported in-place update op
return Collections.emptySet();
+ } else if (op.equals("set") && (obj == null || (obj instanceof Collection && ((Collection) obj).isEmpty()))) {
+ // when operation is set and value is either null or empty list
+ // treat the update as atomic instead of inplace
+ return Collections.emptySet();
}
// fail fast if child doc
if(isChildDoc(((Map<String, Object>) fieldValue).get(op))) {
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
index c40879c..5061805 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
@@ -178,6 +178,8 @@ public class TestInPlaceUpdatesDistrib extends AbstractFullDistribZkTestBase {
resetDelays();
reorderedDBQsResurrectionTest();
resetDelays();
+ setNullForDVEnabledField();
+ resetDelays();
// AwaitsFix this test fails easily
// reorderedDBQsUsingUpdatedValueFromADroppedUpdate();
@@ -219,6 +221,45 @@ public class TestInPlaceUpdatesDistrib extends AbstractFullDistribZkTestBase {
assertEquals(2, NONLEADERS.size());
}
+ private void setNullForDVEnabledField() throws Exception {
+ // to test set=null
+ // should this test be here? As set null would be an atomic update
+ clearIndex();
+ commit();
+
+ buildRandomIndex(0);
+ float inplace_updatable_float = 1;
+
+ // update doc, set
+ index("id", 0, "inplace_updatable_float", map("set", inplace_updatable_float));
+
+ LEADER.commit();
+ SolrDocument sdoc = LEADER.getById("0"); // RTG straight from the index
+ assertEquals(inplace_updatable_float, sdoc.get("inplace_updatable_float"));
+ assertEquals("title0", sdoc.get("title_s"));
+ long version0 = (long) sdoc.get("_version_");
+
+ for (SolrClient client : NONLEADERS) {
+ SolrDocument doc = client.getById(String.valueOf(0), params("distrib", "false"));
+ assertEquals(inplace_updatable_float, doc.get("inplace_updatable_float"));
+ assertEquals(version0, doc.get("_version_"));
+ }
+
+ index("id", 0, "inplace_updatable_float", map("set", null));
+ LEADER.commit();
+
+ sdoc = LEADER.getById("0"); // RTG straight from the index
+ assertNull(sdoc.get("inplace_updatable_float"));
+ assertEquals("title0", sdoc.get("title_s"));
+ long version1 = (long) sdoc.get("_version_");
+
+ for (SolrClient client : NONLEADERS) {
+ SolrDocument doc = client.getById(String.valueOf(0), params("distrib", "false"));
+ assertNull(doc.get("inplace_updatable_float"));
+ assertEquals(version1, doc.get("_version_"));
+ }
+ }
+
final int NUM_RETRIES = 100, WAIT_TIME = 50;
// The following should work: full update to doc 0, in-place update for doc 0, delete doc 0
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
index 02452a1..0f9d2b7 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
@@ -336,6 +336,27 @@ public class TestInPlaceUpdatesStandalone extends SolrTestCaseJ4 {
}
@Test
+ public void testUpdateWithValueNull() throws Exception {
+ long doc = addAndGetVersion(sdoc("id", "1", "title_s", "first", "inplace_updatable_float", 42), null);
+ assertU(commit("softCommit", "false"));
+
+ assertQ(req("q", "*:*", "fq", "inplace_updatable_float:[* TO *]"), "//*[@numFound='1']");
+ // RTG before update
+ assertJQ(req("qt","/get", "id","1", "fl","id,inplace_updatable_float,title_s"),
+ "=={'doc':{'id':'1', 'inplace_updatable_float':" + 42.0 + ",'title_s':" + "first" + "}}");
+
+ // set the value to null
+ doc = addAndAssertVersion(doc, "id", "1", "inplace_updatable_float", map("set", null));
+ assertU(commit("softCommit", "false"));
+
+ // numProducts should be 0
+ assertQ(req("q", "*:*", "fq", "inplace_updatable_float:[* TO *]"), "//*[@numFound='0']");
+ // after update
+ assertJQ(req("qt","/get", "id","1", "fl","id,inplace_updatable_float,title_s"),
+ "=={'doc':{'id':'1','title_s':first}}");
+ }
+
+ @Test
public void testDVUpdatesWithDBQofUpdatedValue() throws Exception {
long version1 = addAndGetVersion(sdoc("id", "1", "title_s", "first", "inplace_updatable_float", "0"), null);
assertU(commit());