[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r272887687
##
File path: solr/core/src/java/org/apache/solr/schema/IndexSchema.java
##
@@ -1959,6 +1959,21 @@ public boolean isUsableForChildDocs() {
rootType.getTypeName().equals(uniqueKeyFieldType.getTypeName()));
}
+ /**
+ * Helper method that returns true if the {@link
#ROOT_FIELD_NAME} uses the exact
+ * same 'type' as the {@link #getUniqueKeyField()} and has {@link
#NEST_PATH_FIELD_NAME}
+ * defined as a {@link NestPathField}
+ * @lucene.internal
+ */
+ public boolean savesChildDocRelations() {
+//TODO make this boolean a field so it needn't be looked up each time?
+if (!isUsableForChildDocs()) {
+ return false;
+}
+FieldType nestPathType = getFieldTypeNoEx(NEST_PATH_FIELD_NAME);
+return null != nestPathType &&
nestPathType.getClass().equals(NestPathField.class);
Review comment:
if you do "instanceof NestPathField" it allows advanced users to subclass
which is nicer.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270683981
##
File path:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
##
@@ -663,118 +661,36 @@ boolean getUpdatedDocument(AddUpdateCommand cmd, long
versionOnUpdate) throws IO
final boolean isDeeplyNestedSchema =
req.getSchema().isUsableForChildDocs() &&
req.getSchema().hasExplicitField(IndexSchema.NEST_PATH_FIELD_NAME);
SolrInputDocument sdoc = cmd.getSolrInputDocument();
BytesRef id = cmd.getIndexedId();
-SolrInputDocument completeHierarchy =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id,
RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
+SolrInputDocument oldRootDocWithChildren =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id,
RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
-if (completeHierarchy == null) {
+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());
}
} else {
- completeHierarchy.remove(CommonParams.VERSION_FIELD);
+ oldRootDocWithChildren.remove(CommonParams.VERSION_FIELD);
}
SolrInputDocument mergedDoc;
-if(idField == null || completeHierarchy == null) {
+if(idField == null || oldRootDocWithChildren == null) {
// create a new doc by default if an old one wasn't found
mergedDoc = docMerger.merge(sdoc, new SolrInputDocument());
} else {
- if(isDeeplyNestedSchema && !isRootDoc(sdoc, completeHierarchy)) {
-// this is an update and the updated doc is not the root document
+ if(isDeeplyNestedSchema && !docMerger.isRootDoc(sdoc,
oldRootDocWithChildren)) {
+// 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);
-// get path of document to be updated
-String updatedDocPath = (String)
sdocWithChildren.getFieldValue(IndexSchema.NEST_PATH_FIELD_NAME);
-// get the SolrInputField containing the document which the
AddUpdateCommand updates
-SolrInputField sifToReplace = getFieldFromHierarchy(completeHierarchy,
updatedDocPath);
-// update SolrInputField, either appending or replacing the updated
document
-updateDocInSif(sifToReplace, sdocWithChildren, sdoc);
-mergedDoc = completeHierarchy;
+mergedDoc = docMerger.mergeNonRoot(sdoc, oldRootDocWithChildren,
sdocWithChildren);
Review comment:
`mergeChildDoc` would probably be clearer than `mergeNonRoot`? It's usually
clearer to avoid negatives when we can say clearly what it _is_ versus what it
_is not_.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270683816
##
File path:
solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
##
@@ -83,6 +86,17 @@ public static boolean isAtomicUpdate(final AddUpdateCommand
cmd) {
return false;
}
+
+ /**
+ *
+ * @param sdoc doc to be updated
+ * @param rootDoc the root document
+ * @return whether sdoc's id equals to rootDoc's id
+ */
+ public boolean isRootDoc(SolrInputDocument sdoc, SolrInputDocument rootDoc) {
Review comment:
This method's signature is so weird and its implementation is short enough
that I wonder if you could simply just inline it and we'd be happier?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270067277
##
File path:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
##
@@ -645,32 +660,126 @@ boolean getUpdatedDocument(AddUpdateCommand cmd, long
versionOnUpdate) throws IO
}
// full (non-inplace) atomic update
+final boolean isDeeplyNestedSchema =
req.getSchema().isUsableForChildDocs() &&
req.getSchema().hasExplicitField(IndexSchema.NEST_PATH_FIELD_NAME);
SolrInputDocument sdoc = cmd.getSolrInputDocument();
BytesRef id = cmd.getIndexedId();
-SolrInputDocument oldDoc =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id);
+SolrInputDocument completeHierarchy =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id,
RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
-if (oldDoc == null) {
- // create a new doc by default if an old one wasn't found
- if (versionOnUpdate <= 0) {
-oldDoc = new SolrInputDocument();
- } else {
+if (completeHierarchy == 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());
}
} else {
- oldDoc.remove(CommonParams.VERSION_FIELD);
+ completeHierarchy.remove(CommonParams.VERSION_FIELD);
}
-cmd.solrDoc = docMerger.merge(sdoc, oldDoc);
+SolrInputDocument mergedDoc;
Review comment:
I think much of the changes you did here in
DistributedUpdateProcessor.getUpdatedDocument (and new supporting methods
added), should move to AtomicUpdateDocumentMerger since it has to do with the
intricacies of merging documents. This code is distracting in
DistributedUpdateProcessor which has plenty of complicated responsibilities
otherwise. WDYT?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270096287
##
File path:
solr/core/src/test/org/apache/solr/update/processor/NestedAtomicUpdateTest.java
##
@@ -0,0 +1,659 @@
+/*
+ * 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.
+ */
+
+package org.apache.solr.update.processor;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import org.apache.lucene.util.BytesRef;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.handler.component.RealTimeGetComponent;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class NestedAtomicUpdateTest extends SolrTestCaseJ4 {
+
+ private final static String VERSION = "_version_";
+
+ @BeforeClass
+ public static void beforeTests() throws Exception {
+initCore("solrconfig-block-atomic-update.xml", "schema-nest.xml"); // use
"nest" schema
+ }
+
+ @Before
+ public void before() {
+clearIndex();
+assertU(commit());
+ }
+
+ @Test
+ public void testMergeChildDoc() throws Exception {
+SolrInputDocument newChildDoc = sdoc("id", "3", "cat_ss", "child");
+SolrInputDocument addedDoc = sdoc("id", "1",
+"cat_ss", Collections.singletonMap("add", "bbb"),
+"child", Collections.singletonMap("add", sdocs(newChildDoc)));
+
+SolrInputDocument dummyBlock = sdoc("id", "1",
+"cat_ss", new ArrayList<>(Arrays.asList("aaa", "ccc")),
+"_root_", "1", "child", new ArrayList<>(sdocs(addedDoc)));
+dummyBlock.removeField(VERSION);
+
+SolrInputDocument preMergeDoc = new SolrInputDocument(dummyBlock);
+AtomicUpdateDocumentMerger docMerger = new
AtomicUpdateDocumentMerger(req());
+docMerger.merge(addedDoc, dummyBlock);
+assertEquals("merged document should have the same id",
preMergeDoc.getFieldValue("id"), dummyBlock.getFieldValue("id"));
+assertDocContainsSubset(preMergeDoc, dummyBlock);
+assertDocContainsSubset(addedDoc, dummyBlock);
+assertDocContainsSubset(newChildDoc, (SolrInputDocument) ((List)
dummyBlock.getFieldValues("child")).get(1));
+assertEquals(dummyBlock.getFieldValue("id"),
dummyBlock.getFieldValue("id"));
+ }
+
+ @Test
+ public void testBlockAtomicInplaceUpdates() throws Exception {
+SolrInputDocument doc = sdoc("id", "1", "string_s", "root");
+addDoc(adoc(doc), "nested-rtg");
+
+assertU(commit());
+
+assertQ(req("q", "id:1", "fl", "*"),
+"//*[@numFound='1']",
+"//doc[1]/str[@name='id']=1"
+);
+
+List docs = IntStream.range(10, 20).mapToObj(x ->
sdoc("id", String.valueOf(x), "string_s",
+"child", "inplace_updatable_int", "0")).collect(Collectors.toList());
+doc = sdoc("id", "1", "children", Collections.singletonMap("add", docs));
+addAndGetVersion(doc, params("update.chain", "nested-rtg", "wt", "json",
"_route_", "1"));
+
+assertU(commit());
+
+
+assertQ(req("q", "_root_:1", "fl", "*", "rows", "11"),
+"//*[@numFound='11']"
+);
+
+assertQ(req("q", "string_s:child", "fl", "*"),
+"//*[@numFound='10']",
+"*[count(//str[@name='string_s'][.='child'])=10]"
+);
+
+for(int i = 10; i < 20; ++i) {
+ doc = sdoc("id", String.valueOf(i), "inplace_updatable_int",
Collections.singletonMap("inc", "1"));
+ addAndGetVersion(doc, params("update.chain", "nested-rtg", "wt", "json",
"_route_", "1"));
+ assertU(commit());
+}
+
+for(int i = 10; i < 20; ++i) {
+ doc = sdoc("id", String.valueOf(i), "inplace_updatable_int",
Collections.singletonMap("inc", "1"));
+ addAndGetVersion(doc, params("update.chain", "nested-rtg", "wt", "json",
"_route_", "1"));
+ assertU(commit());
+}
+
+// ensure updates work when block has more than 10 children
+for(int i = 10; i < 20; ++i) {
+ docs = IntStream.range(i * 10, (i * 10) + 5).mapToObj(x -> sdoc("id",
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270086512
##
File path:
solr/core/src/test/org/apache/solr/cloud/NestedShardedAtomicUpdateTest.java
##
@@ -0,0 +1,191 @@
+/*
+ * 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.
+ */
+
+package org.apache.solr.cloud;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.junit.Test;
+
+public class NestedShardedAtomicUpdateTest extends
AbstractFullDistribZkTestBase {
+
+ public NestedShardedAtomicUpdateTest() {
+stress = 0;
+sliceCount = 4;
+schemaString = "sharded-block-schema.xml";
+ }
+
+ @Override
+ protected String getCloudSolrConfig() {
+return "solrconfig-block-atomic-update.xml";
+ }
+
+ @Override
+ protected String getCloudSchemaFile() {
+return "sharded-block-schema.xml";
+ }
+
+ @Test
+ @ShardsFixed(num = 4)
+ public void test() throws Exception {
+boolean testFinished = false;
+try {
+ doNestedInplaceUpdateTest();
+ doRootShardRoutingTest();
+ testFinished = true;
+} finally {
+ if (!testFinished) {
+printLayoutOnTearDown = true;
+ }
+}
+ }
+
+ public void doRootShardRoutingTest() throws Exception {
+assertEquals(4,
cloudClient.getZkStateReader().getClusterState().getCollection(DEFAULT_COLLECTION).getSlices().size());
+final String[] ids = {"c", "d", "e", "f"};
+
+assertEquals("size of ids to index should be the same as the number of
clients", clients.size(), ids.length);
+// for now, we know how ranges will be distributed to shards.
+// may have to look it up in clusterstate if that assumption changes.
+
+SolrInputDocument doc = sdoc("id", "a", "level_s", "root");
+
+final SolrParams params = new ModifiableSolrParams().set("update.chain",
"nested-rtg").set("wt", "json").set("_route_", "a");
+
+int which = (params.get("_route_").hashCode() & 0x7fff) %
clients.size();
+SolrClient aClient = clients.get(which);
+
+indexDoc(aClient, params, doc);
+
+aClient.commit();
Review comment:
again; some of these commits are superfluous and would better be randomized
so exercise different internal code paths.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270085155
##
File path:
solr/core/src/test-files/solr/collection1/conf/solrconfig-block-atomic-update.xml
##
@@ -0,0 +1,64 @@
+
+
+
+
+
+
+ ${tests.luceneMatchVersion:LATEST}
+ http://www.w3.org/2001/XInclude"/>
+
+
+
+
+
+
+
+
+ ${solr.autoCommit.maxTime:-1}
+
+
+
+
+
+ ${solr.ulog.dir:}
+
+
+
+ ${solr.commitwithin.softcommit:true}
+
+
+
+
+
Review comment:
BTW your comment here is about the schema but this review thread is on
solrconfig.xml which I think could be a stock/basic one instead of yet another
test config file.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270092984
##
File path:
solr/core/src/test/org/apache/solr/cloud/NestedShardedAtomicUpdateTest.java
##
@@ -0,0 +1,191 @@
+/*
+ * 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.
+ */
+
+package org.apache.solr.cloud;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.junit.Test;
+
+public class NestedShardedAtomicUpdateTest extends
AbstractFullDistribZkTestBase {
+
+ public NestedShardedAtomicUpdateTest() {
+stress = 0;
+sliceCount = 4;
+schemaString = "sharded-block-schema.xml";
+ }
+
+ @Override
+ protected String getCloudSolrConfig() {
+return "solrconfig-block-atomic-update.xml";
+ }
+
+ @Override
+ protected String getCloudSchemaFile() {
+return "sharded-block-schema.xml";
+ }
+
+ @Test
+ @ShardsFixed(num = 4)
+ public void test() throws Exception {
+boolean testFinished = false;
+try {
+ doNestedInplaceUpdateTest();
+ doRootShardRoutingTest();
+ testFinished = true;
+} finally {
+ if (!testFinished) {
+printLayoutOnTearDown = true;
+ }
+}
+ }
+
+ public void doRootShardRoutingTest() throws Exception {
+assertEquals(4,
cloudClient.getZkStateReader().getClusterState().getCollection(DEFAULT_COLLECTION).getSlices().size());
+final String[] ids = {"c", "d", "e", "f"};
+
+assertEquals("size of ids to index should be the same as the number of
clients", clients.size(), ids.length);
+// for now, we know how ranges will be distributed to shards.
+// may have to look it up in clusterstate if that assumption changes.
+
+SolrInputDocument doc = sdoc("id", "a", "level_s", "root");
+
+final SolrParams params = new ModifiableSolrParams().set("update.chain",
"nested-rtg").set("wt", "json").set("_route_", "a");
+
+int which = (params.get("_route_").hashCode() & 0x7fff) %
clients.size();
+SolrClient aClient = clients.get(which);
+
+indexDoc(aClient, params, doc);
+
+aClient.commit();
+
+doc = sdoc("id", "a", "children", map("add", sdocs(sdoc("id", "b",
"level_s", "child";
+
+indexDoc(aClient, params, doc);
+aClient.commit();
+
+int i = 0;
+for (SolrClient client : clients) {
+
+ doc = sdoc("id", "b", "grandChildren", map("add", sdocs(sdoc("id",
ids[i], "level_s", "grand_child";
+
+ indexDocAndRandomlyCommit(client, params, doc);
+
+ doc = sdoc("id", "c", "inplace_updatable_int", map("inc", "1"));
+
+ indexDocAndRandomlyCommit(client, params, doc);
+
+ // assert RTG request respects _route_ param
+ QueryResponse routeRsp = client.query(params("qt","/get", "id","b",
"_route_", "a"));
+ SolrDocument results = (SolrDocument) routeRsp.getResponse().get("doc");
+ assertNotNull("RTG should find doc because _route_ was set to the root
documents' ID", results);
+ assertEquals("b", results.getFieldValue("id"));
+
+ // assert all docs are indexed under the same root
+ client.commit();
+ assertEquals(0, client.query(new ModifiableSolrParams().set("q",
"-_root_:a")).getResults().size());
+
+ // assert all docs are indexed inside the same block
+ QueryResponse rsp = client.query(params("qt","/get", "id","a", "fl", "*,
[child]"));
+ SolrDocument val = (SolrDocument) rsp.getResponse().get("doc");
+ assertEquals("a", val.getFieldValue("id"));
+ List children = (List) val.getFieldValues("children");
+ assertEquals(1, children.size());
+ SolrDocument childDoc = children.get(0);
+ assertEquals("b", childDoc.getFieldValue("id"));
+ List grandChildren = (List)
childDoc.getFieldValues("grandChildren");
+ assertEquals(++i, grandChildren.size());
+ SolrDocument grandChild =
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270092633
##
File path:
solr/core/src/test/org/apache/solr/cloud/NestedShardedAtomicUpdateTest.java
##
@@ -0,0 +1,191 @@
+/*
+ * 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.
+ */
+
+package org.apache.solr.cloud;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.junit.Test;
+
+public class NestedShardedAtomicUpdateTest extends
AbstractFullDistribZkTestBase {
+
+ public NestedShardedAtomicUpdateTest() {
+stress = 0;
+sliceCount = 4;
+schemaString = "sharded-block-schema.xml";
+ }
+
+ @Override
+ protected String getCloudSolrConfig() {
+return "solrconfig-block-atomic-update.xml";
+ }
+
+ @Override
+ protected String getCloudSchemaFile() {
+return "sharded-block-schema.xml";
+ }
+
+ @Test
+ @ShardsFixed(num = 4)
+ public void test() throws Exception {
+boolean testFinished = false;
+try {
+ doNestedInplaceUpdateTest();
+ doRootShardRoutingTest();
+ testFinished = true;
+} finally {
+ if (!testFinished) {
+printLayoutOnTearDown = true;
+ }
+}
+ }
+
+ public void doRootShardRoutingTest() throws Exception {
+assertEquals(4,
cloudClient.getZkStateReader().getClusterState().getCollection(DEFAULT_COLLECTION).getSlices().size());
+final String[] ids = {"c", "d", "e", "f"};
+
+assertEquals("size of ids to index should be the same as the number of
clients", clients.size(), ids.length);
+// for now, we know how ranges will be distributed to shards.
+// may have to look it up in clusterstate if that assumption changes.
+
+SolrInputDocument doc = sdoc("id", "a", "level_s", "root");
+
+final SolrParams params = new ModifiableSolrParams().set("update.chain",
"nested-rtg").set("wt", "json").set("_route_", "a");
+
+int which = (params.get("_route_").hashCode() & 0x7fff) %
clients.size();
+SolrClient aClient = clients.get(which);
+
+indexDoc(aClient, params, doc);
+
+aClient.commit();
+
+doc = sdoc("id", "a", "children", map("add", sdocs(sdoc("id", "b",
"level_s", "child";
+
+indexDoc(aClient, params, doc);
+aClient.commit();
+
+int i = 0;
+for (SolrClient client : clients) {
+
+ doc = sdoc("id", "b", "grandChildren", map("add", sdocs(sdoc("id",
ids[i], "level_s", "grand_child";
+
+ indexDocAndRandomlyCommit(client, params, doc);
+
+ doc = sdoc("id", "c", "inplace_updatable_int", map("inc", "1"));
+
+ indexDocAndRandomlyCommit(client, params, doc);
+
+ // assert RTG request respects _route_ param
+ QueryResponse routeRsp = client.query(params("qt","/get", "id","b",
"_route_", "a"));
+ SolrDocument results = (SolrDocument) routeRsp.getResponse().get("doc");
+ assertNotNull("RTG should find doc because _route_ was set to the root
documents' ID", results);
+ assertEquals("b", results.getFieldValue("id"));
+
+ // assert all docs are indexed under the same root
+ client.commit();
+ assertEquals(0, client.query(new ModifiableSolrParams().set("q",
"-_root_:a")).getResults().size());
+
+ // assert all docs are indexed inside the same block
+ QueryResponse rsp = client.query(params("qt","/get", "id","a", "fl", "*,
[child]"));
+ SolrDocument val = (SolrDocument) rsp.getResponse().get("doc");
+ assertEquals("a", val.getFieldValue("id"));
+ List children = (List) val.getFieldValues("children");
+ assertEquals(1, children.size());
+ SolrDocument childDoc = children.get(0);
+ assertEquals("b", childDoc.getFieldValue("id"));
+ List grandChildren = (List)
childDoc.getFieldValues("grandChildren");
+ assertEquals(++i, grandChildren.size());
Review comment:
aha; I see now. This is
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638 URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270078602 ## File path: solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java ## @@ -645,32 +660,126 @@ boolean getUpdatedDocument(AddUpdateCommand cmd, long versionOnUpdate) throws IO } // full (non-inplace) atomic update +final boolean isDeeplyNestedSchema = req.getSchema().isUsableForChildDocs() && req.getSchema().hasExplicitField(IndexSchema.NEST_PATH_FIELD_NAME); SolrInputDocument sdoc = cmd.getSolrInputDocument(); BytesRef id = cmd.getIndexedId(); -SolrInputDocument oldDoc = RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id); +SolrInputDocument completeHierarchy = RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id, RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN); Review comment: I think "oldDocWithChildren" would be a better name. The semantics that it's the old document is important. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638 URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270097963 ## File path: solr/solr-ref-guide/src/the-terms-component.adoc ## @@ -301,3 +301,5 @@ See the section <
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270075968
##
File path:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
##
@@ -645,32 +660,126 @@ boolean getUpdatedDocument(AddUpdateCommand cmd, long
versionOnUpdate) throws IO
}
// full (non-inplace) atomic update
+final boolean isDeeplyNestedSchema =
req.getSchema().isUsableForChildDocs() &&
req.getSchema().hasExplicitField(IndexSchema.NEST_PATH_FIELD_NAME);
SolrInputDocument sdoc = cmd.getSolrInputDocument();
BytesRef id = cmd.getIndexedId();
-SolrInputDocument oldDoc =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id);
+SolrInputDocument completeHierarchy =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id,
RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
-if (oldDoc == null) {
- // create a new doc by default if an old one wasn't found
- if (versionOnUpdate <= 0) {
-oldDoc = new SolrInputDocument();
- } else {
+if (completeHierarchy == 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());
}
} else {
- oldDoc.remove(CommonParams.VERSION_FIELD);
+ completeHierarchy.remove(CommonParams.VERSION_FIELD);
}
-cmd.solrDoc = docMerger.merge(sdoc, oldDoc);
+SolrInputDocument mergedDoc;
+if(idField == null || completeHierarchy == null) {
+ // create a new doc by default if an old one wasn't found
+ mergedDoc = docMerger.merge(sdoc, new SolrInputDocument());
+} else {
+ if(isDeeplyNestedSchema && !isRootDoc(sdoc, completeHierarchy)) {
Review comment:
I suggest commenting: "non-root docs to update must be provided with a
_root_ field for this to work" (assuming you agree this is true; logic seems
to show this)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r270084153
##
File path:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
##
@@ -1032,6 +1151,24 @@ protected void assertNotFinished() {
finished = true;
}
+ /**
+ *
+ * @return whether this update changes a value of a block
+ */
+ public static boolean shouldRefreshUlogCaches(AddUpdateCommand cmd) {
Review comment:
Can we make this logic more efficient by adding a field to AddUpdateCommand
that declares (a) is this update an atomic update, and furthermore, did it
involve nested documents? Perhaps two separate `Boolean`s, null meaning not
yet calculated. By this juncture in the code I think we've already determined
these things and I'd like to avoid potential overhead per document going into
Solr. Lambda's can add up.
typo "update.q" => "update"
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r269519745
##
File path:
solr/core/src/test-files/solr/collection1/conf/solrconfig-block-atomic-update.xml
##
@@ -0,0 +1,64 @@
+
+
+
+
+
+
+ ${tests.luceneMatchVersion:LATEST}
+ http://www.w3.org/2001/XInclude"/>
+
+
+
+
+
+
+
+
+ ${solr.autoCommit.maxTime:-1}
+
+
+
+
+
+ ${solr.ulog.dir:}
+
+
+
+ ${solr.commitwithin.softcommit:true}
+
+
+
+
+
Review comment:
Could that be avoided then? Less test files is a good thing. If can’t be
then this distinction should be documented
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638 URL: https://github.com/apache/lucene-solr/pull/455#discussion_r269404589 ## File path: solr/core/src/test-files/solr/collection1/conf/sharded-block-schema.xml ## @@ -0,0 +1,65 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Review comment: This is obsoleted by 8.0. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r269404669
##
File path:
solr/core/src/test-files/solr/collection1/conf/solrconfig-block-atomic-update.xml
##
@@ -0,0 +1,64 @@
+
+
+
+
+
+
+ ${tests.luceneMatchVersion:LATEST}
+ http://www.w3.org/2001/XInclude"/>
+
+
+
+
+
+
+
+
+ ${solr.autoCommit.maxTime:-1}
+
+
+
+
+
+ ${solr.ulog.dir:}
+
+
+
+ ${solr.commitwithin.softcommit:true}
+
+
+
+
+
Review comment:
Obsoleted by 8.0? If so the whole file can be removed (I think)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638 URL: https://github.com/apache/lucene-solr/pull/455#discussion_r269402083 ## File path: solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java ## @@ -193,6 +194,10 @@ public static boolean isSupportedFieldForInPlaceUpdate(SchemaField schemaField) // not a supported in-place update op return Collections.emptySet(); } +// fail fast if child doc Review comment: Minor: the word "fail" is suggestive of an error. I think you can simply say we don't support in-place updates of a nested relation. BTW maybe this loop would be more efficient to loop on the Map.Entry so that we could eliminate the "get". This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r269405154
##
File path:
solr/core/src/test/org/apache/solr/cloud/AtomicUpdateBlockShardedTest.java
##
@@ -0,0 +1,187 @@
+/*
+ * 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.
+ */
+
+package org.apache.solr.cloud;
+
+import java.util.List;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.junit.Test;
+
+public class AtomicUpdateBlockShardedTest extends
AbstractFullDistribZkTestBase {
+
+ public AtomicUpdateBlockShardedTest() {
+stress = 0;
+sliceCount = 4;
+schemaString = "sharded-block-schema.xml";
+ }
+
+ @Override
+ protected String getCloudSolrConfig() {
+return "solrconfig-block-atomic-update.xml";
+ }
+
+ @Override
+ protected String getCloudSchemaFile() {
+return "sharded-block-schema.xml";
+ }
+
+ @Test
+ @ShardsFixed(num = 4)
+ public void test() throws Exception {
+boolean testFinished = false;
+try {
+ doNestedInplaceUpdateTest();
+ doRootShardRoutingTest();
+ testFinished = true;
+} finally {
+ if (!testFinished) {
+printLayoutOnTearDown = true;
+ }
+}
+ }
+
+ public void doRootShardRoutingTest() throws Exception {
+assertEquals(4,
cloudClient.getZkStateReader().getClusterState().getCollection(DEFAULT_COLLECTION).getSlices().size());
+final String[] ids = {"c", "d", "e", "f"};
+
+assertEquals("size of ids to index should be the same as the number of
clients", clients.size(), ids.length);
+// for now, we know how ranges will be distributed to shards.
+// may have to look it up in clusterstate if that assumption changes.
+
+SolrInputDocument doc = sdoc("id", "a", "level_s", "root");
+
+final SolrParams params = new ModifiableSolrParams().set("update.chain",
"nested-rtg").set("wt", "json").set("_route_", "a");
+
+int which = (params.get("_route_").hashCode() & 0x7fff) %
clients.size();
+SolrClient aClient = clients.get(which);
+
+indexDoc(aClient, params, doc);
+
+aClient.commit();
+
+doc = sdoc("id", "a", "children", map("add", sdocs(sdoc("id", "b",
"level_s", "child";
+
+indexDoc(aClient, params, doc);
+aClient.commit();
+
+int i = 0;
+for (SolrClient client : clients) {
+
+ doc = sdoc("id", "b", "grandChildren", map("add", sdocs(sdoc("id",
ids[i], "level_s", "grand_child";
+
+ indexDoc(client, params, doc);
+ client.commit();
Review comment:
The commits here could be made to occur randomly; I don't think semantically
they need to be there. Only truly needed before a query.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r269405690
##
File path:
solr/core/src/test/org/apache/solr/cloud/AtomicUpdateBlockShardedTest.java
##
@@ -0,0 +1,187 @@
+/*
+ * 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.
+ */
+
+package org.apache.solr.cloud;
+
+import java.util.List;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.junit.Test;
+
+public class AtomicUpdateBlockShardedTest extends
AbstractFullDistribZkTestBase {
+
+ public AtomicUpdateBlockShardedTest() {
+stress = 0;
+sliceCount = 4;
+schemaString = "sharded-block-schema.xml";
+ }
+
+ @Override
+ protected String getCloudSolrConfig() {
+return "solrconfig-block-atomic-update.xml";
+ }
+
+ @Override
+ protected String getCloudSchemaFile() {
+return "sharded-block-schema.xml";
+ }
+
+ @Test
+ @ShardsFixed(num = 4)
+ public void test() throws Exception {
+boolean testFinished = false;
+try {
+ doNestedInplaceUpdateTest();
+ doRootShardRoutingTest();
+ testFinished = true;
+} finally {
+ if (!testFinished) {
+printLayoutOnTearDown = true;
+ }
+}
+ }
+
+ public void doRootShardRoutingTest() throws Exception {
+assertEquals(4,
cloudClient.getZkStateReader().getClusterState().getCollection(DEFAULT_COLLECTION).getSlices().size());
+final String[] ids = {"c", "d", "e", "f"};
+
+assertEquals("size of ids to index should be the same as the number of
clients", clients.size(), ids.length);
+// for now, we know how ranges will be distributed to shards.
+// may have to look it up in clusterstate if that assumption changes.
+
+SolrInputDocument doc = sdoc("id", "a", "level_s", "root");
+
+final SolrParams params = new ModifiableSolrParams().set("update.chain",
"nested-rtg").set("wt", "json").set("_route_", "a");
+
+int which = (params.get("_route_").hashCode() & 0x7fff) %
clients.size();
+SolrClient aClient = clients.get(which);
+
+indexDoc(aClient, params, doc);
+
+aClient.commit();
+
+doc = sdoc("id", "a", "children", map("add", sdocs(sdoc("id", "b",
"level_s", "child";
+
+indexDoc(aClient, params, doc);
+aClient.commit();
+
+int i = 0;
+for (SolrClient client : clients) {
+
+ doc = sdoc("id", "b", "grandChildren", map("add", sdocs(sdoc("id",
ids[i], "level_s", "grand_child";
+
+ indexDoc(client, params, doc);
+ client.commit();
+
+ doc = sdoc("id", "c", "inplace_updatable_int", map("inc", "1"));
+
+ indexDoc(client, params, doc);
+ client.commit();
+
+ // assert RTG request respects _route_ param
+ QueryResponse routeRsp = client.query(params("qt","/get", "id","b",
"_route_", "a"));
+ SolrDocument results = (SolrDocument) routeRsp.getResponse().get("doc");
+ assertNotNull("RTG should find doc because _route_ was set to the root
documents' ID", results);
+ assertEquals("b", results.getFieldValue("id"));
+
+ // assert all docs are indexed under the same root
+ assertEquals(0, client.query(new ModifiableSolrParams().set("q",
"-_root_:a")).getResults().size());
+
+ // assert all docs are indexed inside the same block
+ QueryResponse rsp = client.query(params("qt","/get", "id","a", "fl", "*,
[child]"));
+ SolrDocument val = (SolrDocument) rsp.getResponse().get("doc");
+ assertEquals("a", val.getFieldValue("id"));
+ List children = (List) val.getFieldValues("children");
+ assertEquals(1, children.size());
+ SolrDocument childDoc = children.get(0);
+ assertEquals("b", childDoc.getFieldValue("id"));
+ List grandChildren = (List)
childDoc.getFieldValues("grandChildren");
+ assertEquals(++i, grandChildren.size());
+ SolrDocument grandChild = grandChildren.get(0);
+ assertEquals(i, grandChild.getFirstValue("inplace_updatable_int"));
+
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r269404266
##
File path:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
##
@@ -815,31 +871,7 @@ protected void versionDeleteByQuery(DeleteUpdateCommand
cmd) throws IOException
vinfo.blockUpdates();
try {
- if (versionsStored) {
Review comment:
Was this simply a refactoring or did you modify the logic?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #455: SOLR-12638
dsmiley commented on a change in pull request #455: SOLR-12638
URL: https://github.com/apache/lucene-solr/pull/455#discussion_r269403798
##
File path:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
##
@@ -645,32 +660,78 @@ boolean getUpdatedDocument(AddUpdateCommand cmd, long
versionOnUpdate) throws IO
}
// full (non-inplace) atomic update
+final boolean isDeeplyNestedSchema =
req.getSchema().isUsableForChildDocs() &&
req.getSchema().hasExplicitField(IndexSchema.NEST_PATH_FIELD_NAME);
SolrInputDocument sdoc = cmd.getSolrInputDocument();
BytesRef id = cmd.getIndexedId();
-SolrInputDocument oldDoc =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id);
+SolrInputDocument nestedDoc =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id,
RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
-if (oldDoc == null) {
- // create a new doc by default if an old one wasn't found
- if (versionOnUpdate <= 0) {
-oldDoc = new SolrInputDocument();
- } else {
+if (nestedDoc == 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());
}
} else {
- oldDoc.remove(CommonParams.VERSION_FIELD);
+ nestedDoc.remove(CommonParams.VERSION_FIELD);
}
-cmd.solrDoc = docMerger.merge(sdoc, oldDoc);
+SolrInputDocument mergedDoc;
+if(idField == null || nestedDoc == null) {
+ // create a new doc by default if an old one wasn't found
+ mergedDoc = docMerger.merge(sdoc, new SolrInputDocument());
+} else {
+ if(isDeeplyNestedSchema &&
nestedDoc.containsKey(IndexSchema.ROOT_FIELD_NAME) &&
Review comment:
this part section is fairly complex. I feel it needs its own method and
extra comments on what's happening.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
