mirror of https://github.com/apache/lucene.git
SOLR-13523: Fix Atomic Updates when _nest_path_ is declared.
Change the most common test schema to include this field so we better test our code paths.
This commit is contained in:
David Smiley
parent
5d47875184
commit
54c5b8a7f1
|
|
@ -209,7 +209,11 @@ Jetty 9.4.14.v20181114
|
|||
Bug Fixes
|
||||
----------------------
|
||||
|
||||
SOLR-13510: Intermittent 401's for internode requests with basicauth enabled (Cao Manh Dat, Colvin Cowie)
|
||||
* SOLR-13510: Intermittent 401's for internode requests with basicauth enabled (Cao Manh Dat, Colvin Cowie)
|
||||
|
||||
* SOLR-13523: In 8.1, Atomic Updates were broken (NPE) when the schema declared the new _nest_path_ field even if you
|
||||
weren't using nested docs. In-place updates were not affected (worked).
|
||||
Reminder: if you don't need nested docs then remove both _root_ and _nest_path_ ! (David Smiley)
|
||||
|
||||
================== 8.1.1 ==================
|
||||
|
||||
|
|
|
|||
|
|
@ -668,18 +668,19 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
|
|||
|
||||
// full (non-inplace) atomic update
|
||||
SolrInputDocument sdoc = cmd.getSolrInputDocument();
|
||||
BytesRef id = cmd.getIndexedId();
|
||||
SolrInputDocument oldRootDocWithChildren = RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id, RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
|
||||
BytesRef idBytes = cmd.getIndexedId();
|
||||
String idString = cmd.getPrintableId();
|
||||
SolrInputDocument oldRootDocWithChildren = RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), idBytes, RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
|
||||
|
||||
if (oldRootDocWithChildren == null) {
|
||||
if (versionOnUpdate > 0) {
|
||||
// could just let the optimistic locking throw the error
|
||||
throw new SolrException(ErrorCode.CONFLICT, "Document not found for update. id=" + cmd.getPrintableId());
|
||||
throw new SolrException(ErrorCode.CONFLICT, "Document not found for update. id=" + idString);
|
||||
} else if (req.getParams().get(ShardParams._ROUTE_) != null) {
|
||||
// the specified document could not be found in this shard
|
||||
// and was explicitly routed using _route_
|
||||
throw new SolrException(ErrorCode.BAD_REQUEST,
|
||||
"Could not find document " + idField.getName() + ":" + id +
|
||||
"Could not find document id=" + idString +
|
||||
", perhaps the wrong \"_route_\" param was supplied");
|
||||
}
|
||||
} else {
|
||||
|
|
@ -692,12 +693,12 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
|
|||
// create a new doc by default if an old one wasn't found
|
||||
mergedDoc = docMerger.merge(sdoc, new SolrInputDocument());
|
||||
} else {
|
||||
if(req.getSchema().savesChildDocRelations() &&
|
||||
!sdoc.getField(idField.getName()).getFirstValue().toString()
|
||||
.equals((String) oldRootDocWithChildren.getFieldValue(IndexSchema.ROOT_FIELD_NAME))) {
|
||||
String oldRootDocRootFieldVal = (String) oldRootDocWithChildren.getFieldValue(IndexSchema.ROOT_FIELD_NAME);
|
||||
if(req.getSchema().savesChildDocRelations() && oldRootDocRootFieldVal != null &&
|
||||
!idString.equals(oldRootDocRootFieldVal)) {
|
||||
// this is an update where the updated doc is not the root document
|
||||
SolrInputDocument sdocWithChildren = RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(),
|
||||
id, RealTimeGetComponent.Resolution.DOC_WITH_CHILDREN);
|
||||
idBytes, RealTimeGetComponent.Resolution.DOC_WITH_CHILDREN);
|
||||
mergedDoc = docMerger.mergeChildDoc(sdoc, oldRootDocWithChildren, sdocWithChildren);
|
||||
} else {
|
||||
mergedDoc = docMerger.merge(sdoc, oldRootDocWithChildren);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,34 @@
|
|||
<?xml version="1.0" encoding="UTF-8" ?>
|
||||
<!--
|
||||
Licensed to the Apache Software Foundation (ASF) under one or more
|
||||
contributor license agreements. See the NOTICE file distributed with
|
||||
this work for additional information regarding copyright ownership.
|
||||
The ASF licenses this file to You under the Apache License, Version 2.0
|
||||
(the "License"); you may not use this file except in compliance with
|
||||
the License. You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing, software
|
||||
distributed under the License is distributed on an "AS IS" BASIS,
|
||||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
See the License for the specific language governing permissions and
|
||||
limitations under the License.
|
||||
-->
|
||||
<schema name="root" version="1.6">
|
||||
<field name="id" type="string" indexed="true" stored="true"/>
|
||||
|
||||
<!-- points to the root document of a block of nested documents -->
|
||||
<field name="_root_" type="string" indexed="true" stored="false"/>
|
||||
|
||||
<field name="intDefault" type="int" indexed="true" stored="true" default="42" multiValued="false"/>
|
||||
<field name="intDvoDefault" type="int" indexed="false" stored="false" multiValued="false"
|
||||
useDocValuesAsStored="true" docValues="true" default="42" />
|
||||
|
||||
<dynamicField name="*" type="string" indexed="true" stored="true"/>
|
||||
|
||||
<fieldType name="string" class="solr.StrField"/>
|
||||
<fieldType name="int" class="${solr.tests.IntegerFieldType}" docValues="${solr.tests.numeric.dv}" precisionStep="0" omitNorms="true" positionIncrementGap="0"/>
|
||||
|
||||
<uniqueKey>id</uniqueKey>
|
||||
</schema>
|
||||
|
|
@ -507,7 +507,8 @@
|
|||
|
||||
|
||||
<field name="id" type="string" indexed="true" stored="${solr.tests.id.stored:true}" multiValued="false" required="false" docValues="${solr.tests.id.docValues:false}"/>
|
||||
<field name="_root_" type="string" indexed="true" stored="true" multiValued="false" required="false"/>
|
||||
<field name="_root_" type="string" indexed="true" stored="false"/>
|
||||
<field name="_nest_path_" type="_nest_path_" /><fieldType name="_nest_path_" class="solr.NestPathField"/>
|
||||
|
||||
<field name="signatureField" type="string" indexed="true" stored="false"/>
|
||||
<field name="uuid" type="uuid" stored="true"/>
|
||||
|
|
|
|||
|
|
@ -36,7 +36,7 @@ public class TestChildDocTransformer extends SolrTestCaseJ4 {
|
|||
|
||||
@BeforeClass
|
||||
public static void beforeClass() throws Exception {
|
||||
initCore("solrconfig.xml","schema.xml"); // *not* the "nest" schema version
|
||||
initCore("solrconfig.xml","schema-root.xml"); // *not* the "nest" schema
|
||||
}
|
||||
|
||||
@After
|
||||
|
|
@ -356,12 +356,12 @@ public class TestChildDocTransformer extends SolrTestCaseJ4 {
|
|||
String[] tests = new String[] {
|
||||
"/response/docs/[0]/id=='1'",
|
||||
"/response/docs/[0]/_childDocuments_/[0]/id=='2'",
|
||||
"/response/docs/[0]/_childDocuments_/[0]/cat/[0]/=='childDocument'",
|
||||
"/response/docs/[0]/_childDocuments_/[0]/title/[0]/=='" + titleVals[0] + "'",
|
||||
"/response/docs/[0]/_childDocuments_/[0]/cat=='childDocument'",
|
||||
"/response/docs/[0]/_childDocuments_/[0]/title=='" + titleVals[0] + "'",
|
||||
"/response/docs/[1]/id=='4'",
|
||||
"/response/docs/[1]/_childDocuments_/[0]/id=='5'",
|
||||
"/response/docs/[1]/_childDocuments_/[0]/cat/[0]/=='childDocument'",
|
||||
"/response/docs/[1]/_childDocuments_/[0]/title/[0]/=='" + titleVals[1] + "'"
|
||||
"/response/docs/[1]/_childDocuments_/[0]/cat=='childDocument'",
|
||||
"/response/docs/[1]/_childDocuments_/[0]/title=='" + titleVals[1] + "'"
|
||||
};
|
||||
|
||||
|
||||
|
|
@ -384,12 +384,12 @@ public class TestChildDocTransformer extends SolrTestCaseJ4 {
|
|||
String[] tests = new String[] {
|
||||
"/response/docs/[0]/id=='1'",
|
||||
"/response/docs/[0]/children/docs/[0]/id=='2'",
|
||||
"/response/docs/[0]/children/docs/[0]/cat/[0]/=='childDocument'",
|
||||
"/response/docs/[0]/children/docs/[0]/title/[0]/=='" + titleVals[0] + "'",
|
||||
"/response/docs/[0]/children/docs/[0]/cat=='childDocument'",
|
||||
"/response/docs/[0]/children/docs/[0]/title=='" + titleVals[0] + "'",
|
||||
"/response/docs/[1]/id=='4'",
|
||||
"/response/docs/[1]/children/docs/[0]/id=='5'",
|
||||
"/response/docs/[1]/children/docs/[0]/cat/[0]/=='childDocument'",
|
||||
"/response/docs/[1]/children/docs/[0]/title/[0]/=='" + titleVals[1] + "'"
|
||||
"/response/docs/[1]/children/docs/[0]/cat=='childDocument'",
|
||||
"/response/docs/[1]/children/docs/[0]/title=='" + titleVals[1] + "'"
|
||||
};
|
||||
|
||||
|
||||
|
|
@ -417,12 +417,12 @@ public class TestChildDocTransformer extends SolrTestCaseJ4 {
|
|||
"//*[@numFound='2']",
|
||||
"/response/result/doc[1]/str[@name='id']='1'" ,
|
||||
"/response/result/doc[1]/doc[1]/str[@name='id']='2'" ,
|
||||
"/response/result/doc[1]/doc[1]/arr[@name='cat']/str[1]='childDocument'" ,
|
||||
"/response/result/doc[1]/doc[1]/arr[@name='title']/str[1]='" + titleVals[0] + "'" ,
|
||||
"/response/result/doc[1]/doc[1]/str[@name='cat']='childDocument'" ,
|
||||
"/response/result/doc[1]/doc[1]/str[@name='title']='" + titleVals[0] + "'" ,
|
||||
"/response/result/doc[2]/str[@name='id']='4'" ,
|
||||
"/response/result/doc[2]/doc[1]/str[@name='id']='5'",
|
||||
"/response/result/doc[2]/doc[1]/arr[@name='cat']/str[1]='childDocument'",
|
||||
"/response/result/doc[2]/doc[1]/arr[@name='title']/str[1]='" + titleVals[1] + "'"};
|
||||
"/response/result/doc[2]/doc[1]/str[@name='cat']='childDocument'",
|
||||
"/response/result/doc[2]/doc[1]/str[@name='title']='" + titleVals[1] + "'"};
|
||||
|
||||
assertQ(req("q", "*:*",
|
||||
"sort", "id asc",
|
||||
|
|
@ -443,12 +443,12 @@ public class TestChildDocTransformer extends SolrTestCaseJ4 {
|
|||
"//*[@numFound='2']",
|
||||
"/response/result/doc[1]/str[@name='id']='1'" ,
|
||||
"/response/result/doc[1]/result[@name='children'][@numFound=1]/doc[1]/str[@name='id']='2'" ,
|
||||
"/response/result/doc[1]/result[@name='children'][@numFound=1]/doc[1]/arr[@name='cat']/str[1]='childDocument'" ,
|
||||
"/response/result/doc[1]/result[@name='children'][@numFound=1]/doc[1]/arr[@name='title']/str[1]='" + titleVals[0] + "'" ,
|
||||
"/response/result/doc[1]/result[@name='children'][@numFound=1]/doc[1]/str[@name='cat']='childDocument'" ,
|
||||
"/response/result/doc[1]/result[@name='children'][@numFound=1]/doc[1]/str[@name='title']='" + titleVals[0] + "'" ,
|
||||
"/response/result/doc[2]/str[@name='id']='4'" ,
|
||||
"/response/result/doc[2]/result[@name='children'][@numFound=1]/doc[1]/str[@name='id']='5'",
|
||||
"/response/result/doc[2]/result[@name='children'][@numFound=1]/doc[1]/arr[@name='cat']/str[1]='childDocument'",
|
||||
"/response/result/doc[2]/result[@name='children'][@numFound=1]/doc[1]/arr[@name='title']/str[1]='" + titleVals[1] + "'"};
|
||||
"/response/result/doc[2]/result[@name='children'][@numFound=1]/doc[1]/str[@name='cat']='childDocument'",
|
||||
"/response/result/doc[2]/result[@name='children'][@numFound=1]/doc[1]/str[@name='title']='" + titleVals[1] + "'"};
|
||||
|
||||
assertQ(req(
|
||||
"q", "*:*", "fq", "subject:\"parentDocument\" ",
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ import org.apache.solr.common.SolrException;
|
|||
import org.apache.solr.common.SolrInputDocument;
|
||||
import org.apache.solr.common.params.ModifiableSolrParams;
|
||||
import org.apache.solr.request.LocalSolrQueryRequest;
|
||||
import org.apache.solr.request.SolrQueryRequest;
|
||||
import org.apache.solr.response.SolrQueryResponse;
|
||||
import org.apache.solr.update.AddUpdateCommand;
|
||||
import org.junit.BeforeClass;
|
||||
|
|
@ -231,9 +232,10 @@ public class AtomicUpdateProcessorFactoryTest extends SolrTestCaseJ4 {
|
|||
cmd.solrDoc.addField("int_i", index);
|
||||
|
||||
try {
|
||||
SolrQueryResponse rsp = new SolrQueryResponse();
|
||||
factory.getInstance(cmd.getReq(), new SolrQueryResponse(),
|
||||
createDistributedUpdateProcessor(cmd.getReq(), new SolrQueryResponse(),
|
||||
new RunUpdateProcessor(cmd.getReq(), null))).processAdd(cmd);
|
||||
createDistributedUpdateProcessor(cmd.getReq(), rsp,
|
||||
createRunUpdateProcessor(cmd.getReq(), rsp, null))).processAdd(cmd);
|
||||
} catch (IOException e) {
|
||||
}
|
||||
}
|
||||
|
|
@ -266,6 +268,10 @@ public class AtomicUpdateProcessorFactoryTest extends SolrTestCaseJ4 {
|
|||
|
||||
}
|
||||
|
||||
private UpdateRequestProcessor createRunUpdateProcessor(SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor next) {
|
||||
return new RunUpdateProcessorFactory().getInstance(req, rsp, next);
|
||||
}
|
||||
|
||||
private String generateRandomString() {
|
||||
char[] chars = "abcdefghijklmnopqrstuvwxyz0123456789".toCharArray();
|
||||
StringBuilder sb = new StringBuilder();
|
||||
|
|
|
|||
|
|
@ -1259,7 +1259,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=8"
|
||||
);
|
||||
|
||||
// do atomic update
|
||||
|
|
@ -1274,7 +1273,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=8"
|
||||
);
|
||||
|
||||
assertU(commit());
|
||||
|
|
@ -1288,7 +1286,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=8"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -1311,7 +1308,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=7"
|
||||
);
|
||||
// do atomic update
|
||||
assertU(adoc(sdoc("id", "7", fieldToUpdate, ImmutableMap.of("inc", -555))));
|
||||
|
|
@ -1325,7 +1321,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=7"
|
||||
);
|
||||
|
||||
// diff doc where we check that we can overwrite the default value
|
||||
|
|
@ -1340,7 +1335,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=7"
|
||||
);
|
||||
// do atomic update
|
||||
assertU(adoc(sdoc("id", "8", fieldToUpdate, ImmutableMap.of("inc", -555))));
|
||||
|
|
@ -1354,7 +1348,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=7"
|
||||
);
|
||||
|
||||
assertU(commit());
|
||||
|
|
@ -1369,7 +1362,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=7"
|
||||
);
|
||||
assertQ(fieldToUpdate + ": doc8 post commit RTG"
|
||||
, req("qt", "/get", "id", "8")
|
||||
|
|
@ -1381,7 +1373,6 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
|
|||
, "//doc/long[@name='_version_']"
|
||||
, "//doc/date[@name='timestamp']"
|
||||
, "//doc/arr[@name='multiDefault']/str[.='muLti-Default']"
|
||||
, "count(//doc/*)=7"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -114,16 +114,22 @@ public class UpdateRequestProcessorFactoryTest extends SolrTestCaseJ4 {
|
|||
assertNotNull(name, chain);
|
||||
|
||||
// either explicitly, or because of injection
|
||||
assertEquals(name + " chain length: " + chain.toString(), EXPECTED_CHAIN_LENGTH,
|
||||
assertEquals(name + " factory chain length: " + chain.toString(), EXPECTED_CHAIN_LENGTH,
|
||||
chain.getProcessors().size());
|
||||
|
||||
// test a basic (non distrib) chain
|
||||
proc = chain.createProcessor(req(), new SolrQueryResponse());
|
||||
procs = procToList(proc);
|
||||
assertEquals(name + " procs size: " + procs.toString(),
|
||||
// -1 = NoOpDistributingUpdateProcessorFactory produces no processor
|
||||
EXPECTED_CHAIN_LENGTH - ("distrib-chain-noop".equals(name) ? 1 : 0),
|
||||
procs.size());
|
||||
|
||||
int expectedProcLen = EXPECTED_CHAIN_LENGTH;
|
||||
if ("distrib-chain-noop".equals(name)) { // NoOpDistributingUpdateProcessorFactory produces no processor
|
||||
expectedProcLen--;
|
||||
}
|
||||
if (procs.stream().anyMatch(p -> p.getClass().getSimpleName().equals("NestedUpdateProcessor"))) {
|
||||
expectedProcLen++; // NestedUpdate sneaks in via RunUpdate's Factory.
|
||||
}
|
||||
|
||||
assertEquals(name + " procs size: " + procs.toString(), expectedProcLen, procs.size());
|
||||
|
||||
// Custom comes first in all three of our chains
|
||||
assertTrue(name + " first processor isn't a CustomUpdateRequestProcessor: " + procs.toString(),
|
||||
|
|
@ -159,12 +165,19 @@ public class UpdateRequestProcessorFactoryTest extends SolrTestCaseJ4 {
|
|||
procs.get(procs.size()-1) instanceof RunUpdateProcessor );
|
||||
|
||||
// either 1 proc was droped in distrib mode, or 1 for the "implicit" chain
|
||||
|
||||
expectedProcLen = EXPECTED_CHAIN_LENGTH;
|
||||
expectedProcLen--; // -1 = all chains lose CustomUpdateRequestProcessorFactory
|
||||
if ("distrib-chain-explicit".equals(name) == false) {
|
||||
// -1 = distrib-chain-noop: NoOpDistributingUpdateProcessorFactory produces no processor
|
||||
// -1 = distrib-chain-implicit: does RemoveBlank before distrib
|
||||
expectedProcLen--;
|
||||
}
|
||||
if (procs.stream().anyMatch(p -> p.getClass().getSimpleName().equals("NestedUpdateProcessor"))) {
|
||||
expectedProcLen++; // NestedUpdate sneaks in via RunUpdate's Factory.
|
||||
}
|
||||
assertEquals(name + " (distrib) chain has wrong length: " + procs.toString(),
|
||||
// -1 = all chains lose CustomUpdateRequestProcessorFactory
|
||||
// -1 = distrib-chain-noop: NoOpDistributingUpdateProcessorFactory produces no processor
|
||||
// -1 = distrib-chain-implicit: does RemoveBlank before distrib
|
||||
EXPECTED_CHAIN_LENGTH - ( "distrib-chain-explicit".equals(name) ? 1 : 2),
|
||||
procs.size());
|
||||
expectedProcLen, procs.size());
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue