SOLR-11876: In-place updates fail during resolution if required fields are present

solr/CHANGES.txt

@@ -61,6 +61,9 @@ Bug Fixes
 
 * SOLR-13229: Cleanup replicasMetTragicEvent after all types of exception (Tomás Fernández Löbbe)
 
+* SOLR-11876: In-place update fails when resolving from Tlog if schema has a required field (Justin Deoliveira, janhoy,
+  Ishan Chattopadhyaya)
+
 Improvements
 ----------------------
 * SOLR-12999: Index replication could delete segments before downloading segments from master if there is not enough

solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java

@@ -82,6 +82,7 @@ import org.apache.solr.update.IndexFingerprint;
 import org.apache.solr.update.PeerSync;
 import org.apache.solr.update.PeerSyncWithLeader;
 import org.apache.solr.update.UpdateLog;
+import org.apache.solr.update.processor.AtomicUpdateDocumentMerger;
 import org.apache.solr.util.RefCounted;
 import org.apache.solr.util.TestInjection;
 import org.slf4j.Logger;
@@ -426,7 +427,15 @@ public class RealTimeGetComponent extends SearchComponent
     } else { // i.e. lastPrevPointer==0
       assert lastPrevPointer == 0;
       // We have successfully resolved the document based off the tlogs
-      return toSolrDoc(partialDoc, core.getLatestSchema());
+
+      // determine whether we can use the in place document, if the caller specified onlyTheseFields
+      // and those fields are all supported for in-place updates
+      IndexSchema schema = core.getLatestSchema();
+      boolean forInPlaceUpdate = onlyTheseFields != null
+          && onlyTheseFields.stream().map(schema::getField)
+          .allMatch(f -> null!=f && AtomicUpdateDocumentMerger.isSupportedFieldForInPlaceUpdate(f));
+
+      return toSolrDoc(partialDoc, schema, forInPlaceUpdate);
     }
   }
 
@@ -773,9 +782,21 @@ public class RealTimeGetComponent extends SearchComponent
    * @lucene.experimental
    */
   public static SolrDocument toSolrDoc(SolrInputDocument sdoc, IndexSchema schema) {
+    return toSolrDoc(sdoc, schema, false);
+  }
+
+  /**
+   * Converts a SolrInputDocument to SolrDocument, using an IndexSchema instance.
+   *
+   * @param sdoc The input document.
+   * @param schema The index schema.
+   * @param forInPlaceUpdate Whether the document is being used for an in place update,
+   *                         see {@link DocumentBuilder#toDocument(SolrInputDocument, IndexSchema, boolean, boolean)}
+   */
+  public static SolrDocument toSolrDoc(SolrInputDocument sdoc, IndexSchema schema, boolean forInPlaceUpdate) {
     // TODO what about child / nested docs?
     // TODO: do something more performant than this double conversion
-    Document doc = DocumentBuilder.toDocument(sdoc, schema);
+    Document doc = DocumentBuilder.toDocument(sdoc, schema, forInPlaceUpdate, true);
 
     // copy the stored fields only
     Document out = new Document();

solr/core/src/java/org/apache/solr/update/DocumentBuilder.java

@@ -229,8 +229,8 @@ public class DocumentBuilder {
     // Now validate required fields or add default values
     // fields with default values are defacto 'required'
 
-    // Note: We don't need to add default fields if this document is to be used for
-    // in-place updates, since this validation and population of default fields would've happened
+    // Note: We don't need to add required fields if this document is to be used for
+    // in-place updates, since this validation and population of required fields would've happened
     // during the full indexing initially.
     if (!forInPlaceUpdate) {
       for (SchemaField field : schema.getRequiredFields()) {

solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java

@@ -148,7 +148,7 @@ public class AtomicUpdateDocumentMerger {
    * Note: If an update command has updates to only supported fields (and _version_ is also supported),
    * only then is such an update command executed as an in-place update.
    */
-  private static boolean isSupportedFieldForInPlaceUpdate(SchemaField schemaField) {
+  public static boolean isSupportedFieldForInPlaceUpdate(SchemaField schemaField) {
     return !(schemaField.indexed() || schemaField.stored() || !schemaField.hasDocValues() || 
         schemaField.multiValued() || !(schemaField.getType() instanceof NumericValueFieldType));
   }

solr/core/src/test-files/solr/collection1/conf/schema-inplace-required-field.xml

@@ -0,0 +1,35 @@
+<?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="inplace-updates" version="1.6">
+
+  <uniqueKey>id</uniqueKey>
+  <field name="id" type="string" indexed="true" stored="true" docValues="true"/>
+  <field name="_version_" type="long" indexed="false" stored="false"  docValues="true" />
+
+  <field name="name" type="string" indexed="true" stored="true" required="true"/>
+  <field name="inplace_updatable_int"   type="int"   indexed="false" stored="false" docValues="true" />
+
+  <fieldType name="string" class="solr.StrField" multiValued="false" indexed="false" stored="false" docValues="false" />
+  <fieldType name="long" class="${solr.tests.LongFieldType}" multiValued="false" indexed="false" stored="false" docValues="false"/>
+  <fieldType name="int" class="${solr.tests.IntegerFieldType}" multiValued="false" indexed="false" stored="false" docValues="false"/>
+
+  <!-- cruft needed by the solrconfig used in our tests for startup, but not used in the tests -->
+  <field name="signatureField" type="string" indexed="true" stored="false"/>
+  <dynamicField name="*_sS" type="string" indexed="true" stored="true"/>
+
+</schema>

solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesRequiredField.java

@@ -0,0 +1,48 @@
+/*
+ * 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;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.update.TestInPlaceUpdatesStandalone.addAndAssertVersion;
+
+public class TestInPlaceUpdatesRequiredField extends SolrTestCaseJ4  {
+
+  @BeforeClass
+  public static void beforeClass() throws Exception {
+    initCore("solrconfig-tlog.xml", "schema-inplace-required-field.xml");
+  }
+
+  @Test
+  public void testUpdateFromTlog() throws Exception {
+    long version1 = addAndGetVersion(sdoc("id", "1", "name", "first", "inplace_updatable_int", 1), null);
+    assertU(commit("softCommit", "false"));
+    assertQ(req("q", "*:*"), "//*[@numFound='1']");
+
+    // do an in place update that hits off the tlog
+    version1 = addAndAssertVersion(version1, "id", "1", "inplace_updatable_int", map("inc", 1));
+    version1 = addAndAssertVersion(version1, "id", "1", "inplace_updatable_int", map("inc", 1));
+    version1 = addAndAssertVersion(version1, "id", "1", "inplace_updatable_int", map("inc", 1)); // new value should be 4
+    assertU(commit("softCommit", "false"));
+    assertQ(req("q", "id:1", "fl", "*,[docid]"),
+        "//result/doc[1]/int[@name='inplace_updatable_int'][.='4']",
+        "//result/doc[1]/long[@name='_version_'][.='"+version1+"']");
+  }
+}