You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by is...@apache.org on 2019/04/24 10:52:51 UTC
[lucene-solr] branch branch_8x updated: SOLR-12167: Throw an
exception, instead of just a warning, upon unknown atomic update
This is an automated email from the ASF dual-hosted git repository.
ishan 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 a2d499e SOLR-12167: Throw an exception, instead of just a warning, upon unknown atomic update
a2d499e is described below
commit a2d499e32a471f5d22f9101125543e163c0db293
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Wed Apr 24 16:20:10 2019 +0530
SOLR-12167: Throw an exception, instead of just a warning, upon unknown atomic update
---
solr/CHANGES.txt | 3 ++
.../processor/AtomicUpdateDocumentMerger.java | 19 ++++-----
.../solr/update/processor/AtomicUpdatesTest.java | 46 +++++++++++++++++++++-
3 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 173a3b2..ab49a4d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -215,6 +215,9 @@ Improvements
* SOLR-13337: Only request the minimum required number of terms from each shard when using terms.sort=index and none
are discarded due to terms.min/maxcount (Morten Bøgeskov,Munendra S N via Mikhail Khludnev)
+* SOLR-12167: Throw an exception, instead of just a warning, when unknown atomic update operation is
+ encountered (Munendra S N via 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 08b249c..2d0c923 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
@@ -101,39 +101,36 @@ public class AtomicUpdateDocumentMerger {
for (Entry<String,Object> entry : ((Map<String,Object>) val).entrySet()) {
String key = entry.getKey();
Object fieldVal = entry.getValue();
- boolean updateField = false;
switch (key) {
case "add":
- updateField = true;
doAdd(toDoc, sif, fieldVal);
break;
case "set":
- updateField = true;
doSet(toDoc, sif, fieldVal);
break;
case "remove":
- updateField = true;
doRemove(toDoc, sif, fieldVal);
break;
case "removeregex":
- updateField = true;
doRemoveRegex(toDoc, sif, fieldVal);
break;
case "inc":
- updateField = true;
doInc(toDoc, sif, fieldVal);
break;
case "add-distinct":
- updateField = true;
doAddDistinct(toDoc, sif, fieldVal);
break;
default:
- //Perhaps throw an error here instead?
- log.warn("Unknown operation for the an atomic update, operation ignored: " + key);
- break;
+ Object id = toDoc.containsKey(idField.getName())? toDoc.getFieldValue(idField.getName()):
+ fromDoc.getFieldValue(idField.getName());
+ String err = "Unknown operation for the an atomic update, operation ignored: " + key;
+ if (id != null) {
+ err = err + " for id:" + id;
+ }
+ throw new SolrException(ErrorCode.BAD_REQUEST, err);
}
// validate that the field being modified is not the id field.
- if (updateField && idField.getName().equals(sif.getName())) {
+ if (idField.getName().equals(sif.getName())) {
throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid update of id field: " + sif);
}
diff --git a/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java b/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java
index f72fd67..53798a1 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java
@@ -1006,6 +1006,11 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '1']");
assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']");
assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '1']");
+
+ // update on id
+ doc = new SolrInputDocument();
+ doc.setField("id", ImmutableMap.of("set", "1001"));
+ assertFailedU(adoc(doc));
}
public void testAtomicUpdatesOnDateFields() throws Exception {
@@ -1190,7 +1195,7 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
doc = new SolrInputDocument();
doc.setField("id", "7");
doc.setField("cat", ImmutableMap.of("whatever", "bbb"));
- assertU(adoc(doc));
+ assertFailedU( adoc(doc));
assertU(commit());
assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '1']");
@@ -1198,6 +1203,45 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '0']");
assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '1']");
+ // add a nested document;
+ doc = new SolrInputDocument();
+ doc.setField("id", "123");
+ doc.setField("cat", ImmutableMap.of("whatever", "ddd"));
+
+ SolrInputDocument childDoc = new SolrInputDocument();
+ childDoc.setField("id", "1231");
+ childDoc.setField("title", "title_nested");
+ doc.addChildDocument(childDoc);
+ assertFailedU(adoc(doc));
+ assertU(commit());
+
+ assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '1']");
+ assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '1']");
+ assertQ(req("q", "cat:ddd", "indent", "true"), "//result[@numFound = '0']");
+ assertQ(req("q", "title:title_nested", "indent", "true"), "//result[@numFound = '0']");
+ assertQ(req("q", "id:123", "indent", "true"), "//result[@numFound = '0']");
+ assertQ(req("q", "id:1231", "indent", "true"), "//result[@numFound = '0']");
+ assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '1']");
+
+ doc = new SolrInputDocument();
+ doc.setField("id", "123");
+ doc.setField("title", "title_parent");
+
+ childDoc = new SolrInputDocument();
+ childDoc.setField("id", "12311");
+ childDoc.setField("cat", "ddd");
+ childDoc.setField("title", "title_nested");
+ doc.addChildDocument(childDoc);
+ assertU(adoc(doc));
+ assertU(commit());
+
+ assertQ(req("q", "*:*", "indent", "true"), "//result[@numFound = '3']");
+ assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '1']");
+ assertQ(req("q", "cat:ddd", "indent", "true"), "//result[@numFound = '1']");
+ assertQ(req("q", "title:title_nested", "indent", "true"), "//result[@numFound = '1']");
+ assertQ(req("q", "id:123", "indent", "true"), "//result[@numFound = '1']");
+ assertQ(req("q", "id:12311", "indent", "true"), "//result[@numFound = '1']");
+ assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '1']");
}
public void testFieldsWithDefaultValuesWhenAtomicUpdatesAgainstTlog() {