[6.40] [ntuple] Fix late model extension of untyped records

tree/ntuple/src/RField.cxx

@@ -644,6 +644,26 @@ void ROOT::Internal::AddItemToRecord(RRecordField &record, std::unique_ptr<RFiel
    // Only supported for untyped records
    assert(record.GetTypeName().empty());
    record.AddItem(std::move(newItem));
+
+   // For nested record fields, we have to reattach all fields of all parents because the modification to the
+   // current field will change its size and may change its alignment and hence the layout of all parents.
+   auto parent = record.fParent;
+   while (parent && typeid(*parent) != typeid(RFieldZero)) {
+      // untyped records can only have an untyped record as parent
+      assert(dynamic_cast<RRecordField *>(parent) && parent->GetTypeName().empty());
+
+      auto precord = static_cast<RRecordField *>(parent);
+      std::vector<std::unique_ptr<ROOT::RFieldBase>> subfields;
+      std::swap(precord->fSubfields, subfields);
+      precord->fTraits = ROOT::RFieldBase::kTraitExtensible;
+      precord->fSubfieldNames.clear();
+      precord->fMaxAlignment = 1;
+      precord->fSize = 0;
+      precord->fOffsets.clear();
+      precord->AttachItemFields(std::move(subfields));
+
+      parent = precord->fParent;
+   }
 }
 
 std::size_t ROOT::RRecordField::GetItemPadding(std::size_t baseOffset, std::size_t itemAlignment) const

tree/ntuple/src/RPageSinkBuf.cxx

@@ -109,7 +109,16 @@ void ROOT::Internal::RPageSinkBuf::UpdateSchema(const ROOT::Internal::RNTupleMod
    auto cloneAddField = [&](const ROOT::RFieldBase *field) {
       auto cloned = field->Clone(field->GetFieldName());
       auto p = &(*cloned);
-      fInnerModel->AddField(std::move(cloned));
+
+      auto parent = field->GetParent();
+      assert(parent);
+      if (typeid(*parent) != typeid(RFieldZero)) {
+         auto &innerParent = fInnerModel->GetMutableField(parent->GetQualifiedFieldName());
+         assert(dynamic_cast<RRecordField *>(&innerParent));
+         AddItemToRecord(static_cast<RRecordField &>(innerParent), std::move(cloned));
+      } else {
+         fInnerModel->AddField(std::move(cloned));
+      }
       return p;
    };
    auto cloneAddProjectedField = [&](ROOT::RFieldBase *field) {

tree/ntuple/test/ntuple_modelext.cxx

@@ -737,33 +737,33 @@ TEST(RNTuple, ModelExtensionRecordSimple)
    std::string expect{
 R"({
   "r": {
-    "pt": 1
-  },
-  "vec": []
+    "pt": 1,
+    "vec": []
+  }
 }
 {
   "r": {
-    "pt": 2
-  },
-  "vec": []
+    "pt": 2,
+    "vec": []
+  }
 }
 {
   "r": {
-    "pt": 3
-  },
-  "vec": [10, 11]
+    "pt": 3,
+    "vec": [10, 11]
+  }
 }
 {
   "r": {
-    "pt": 4
-  },
-  "vec": []
+    "pt": 4,
+    "vec": []
+  }
 }
 {
   "r": {
-    "pt": 5
-  },
-  "vec": [12]
+    "pt": 5,
+    "vec": [12]
+  }
 }
 )" };
    // clang-format on
@@ -782,9 +782,9 @@ TEST(RNTuple, ModelExtensionRecordNested)
       model->AddField(std::move(recordField));
 
       RNTupleWriteOptions opts;
-      opts.SetUseBufferedWrite(false);
       auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath(), opts);
 
+      EXPECT_EQ(1u, writer->GetModel().GetConstField("r1").GetValueSize());
       auto entry = writer->CreateEntry();
       writer->Fill(*entry);
       writer->CommitCluster();
@@ -800,27 +800,58 @@ TEST(RNTuple, ModelExtensionRecordNested)
       modelUpdater->AddField(std::move(recordField), "r1.r2");
       modelUpdater->CommitUpdate();
 
+      EXPECT_EQ(sizeof(float), writer->GetModel().GetConstField("r1").GetValueSize());
       entry = writer->CreateEntry();
       auto ptrFloat = static_cast<float *>(entry->GetPtr<void>("r1").get());
       *ptrFloat = 1.0;
       writer->Fill(*entry);
+
+      modelUpdater->BeginUpdate();
+      modelUpdater->AddField(std::make_unique<RField<double>>("ptHP"), "r1.r2.r3.r4");
+      modelUpdater->CommitUpdate();
+
+      EXPECT_EQ(2 * sizeof(double), writer->GetModel().GetConstField("r1").GetValueSize());
+      entry = writer->CreateEntry();
+      struct FloatAndDouble {
+         float pt;
+         double ptHP;
+      };
+
+      auto ptrFloatAndDouble = static_cast<FloatAndDouble *>(entry->GetPtr<void>("r1").get());
+      ptrFloatAndDouble->pt = 2.0;
+      ptrFloatAndDouble->ptHP = 3.0;
+      writer->Fill(*entry);
    }
 
    auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath());
-   EXPECT_EQ(3u, reader->GetNEntries());
+   EXPECT_EQ(4u, reader->GetNEntries());
 
    std::ostringstream os;
    reader->Show(0, os);
    reader->Show(1, os);
    reader->Show(2, os);
+   reader->Show(3, os);
    // clang-format off
    std::string expect{
 R"({
   "r1": {
     "r2": {
       "r3": {
         "r4": {
-          "pt": 0
+          "pt": 0,
+          "ptHP": 0
+        }
+      }
+    }
+  }
+}
+{
+  "r1": {
+    "r2": {
+      "r3": {
+        "r4": {
+          "pt": 0,
+          "ptHP": 0
         }
       }
     }
@@ -831,7 +862,8 @@ R"({
     "r2": {
       "r3": {
         "r4": {
-          "pt": 0
+          "pt": 1,
+          "ptHP": 0
         }
       }
     }
@@ -842,7 +874,8 @@ R"({
     "r2": {
       "r3": {
         "r4": {
-          "pt": 1
+          "pt": 2,
+          "ptHP": 3
         }
       }
     }