Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/455#discussion_r224307768
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
---
@@ -1360,24 +1385,47 @@ boolean getUpdatedDocument(AddUpdateCommand cmd,
long versionOnUpdate) throws IO
}
// full (non-inplace) atomic update
+ final boolean isNestedSchema = req.getSchema().isUsableForChildDocs();
SolrInputDocument sdoc = cmd.getSolrInputDocument();
BytesRef id = cmd.getIndexedId();
- SolrInputDocument oldDoc =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id);
+ SolrInputDocument blockDoc =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id, null,
+ false, null, true, true);
- if (oldDoc == null) {
- // create a new doc by default if an old one wasn't found
- if (versionOnUpdate <= 0) {
- oldDoc = new SolrInputDocument();
- } else {
+ if (blockDoc == 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);
+ blockDoc.remove(CommonParams.VERSION_FIELD);
}
- cmd.solrDoc = docMerger.merge(sdoc, oldDoc);
+ SolrInputDocument mergedDoc;
+ if(idField == null || blockDoc == null) {
+ // create a new doc by default if an old one wasn't found
+ mergedDoc = docMerger.merge(sdoc, new SolrInputDocument());
+ } else {
+ if(isNestedSchema &&
req.getSchema().hasExplicitField(IndexSchema.NEST_PATH_FIELD_NAME) &&
+ blockDoc.containsKey(IndexSchema.ROOT_FIELD_NAME) &&
!id.utf8ToString().equals(blockDoc.getFieldValue(IndexSchema.ROOT_FIELD_NAME)))
{
+ SolrInputDocument oldDoc =
RealTimeGetComponent.getInputDocument(cmd.getReq().getCore(), id, null,
+ false, null, true, false);
+ mergedDoc = docMerger.merge(sdoc, oldDoc);
+ String docPath = (String)
mergedDoc.getFieldValue(IndexSchema.NEST_PATH_FIELD_NAME);
+ List<String> docPaths = StrUtils.splitSmart(docPath,
PATH_SEP_CHAR);
+ SolrInputField replaceDoc =
blockDoc.getField(docPaths.remove(0).replaceAll(PATH_SEP_CHAR + "|" +
NUM_SEP_CHAR, ""));
--- End diff --
The logic here (not just this line) is non-obvious to me. There are no
comments. Please add comments and maybe refactor out a method. The use of
replaceAll with a regexp is suspicious to me. None of the tests you added
triggered a breakpoint within the docPaths loop below. Needs more testing or
possible error.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
