[ntuple] Fix automatic schema evolution of record fields

tree/ntuple/inc/ROOT/RField/RFieldRecord.hxx

@@ -63,6 +63,7 @@ class RRecordField : public RFieldBase {
 
    /// If `emulatedFromType` is non-empty, this field was created as a replacement for a ClassField that we lack a
    /// dictionary for and reconstructed from the on-disk information.
+   /// Used by the public constructor and by Internal::CreateEmulatedRecordField().
    RRecordField(std::string_view fieldName, std::vector<std::unique_ptr<RFieldBase>> itemFields,
                 std::string_view emulatedFromType);
 
@@ -73,7 +74,7 @@ protected:
 
    std::size_t GetItemPadding(std::size_t baseOffset, std::size_t itemAlignment) const;
 
-   std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const final;
+   std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const override;
 
    void ConstructValue(void *where) const final;
    std::unique_ptr<RDeleter> GetDeleter() const final;
@@ -82,6 +83,8 @@ protected:
    void ReadGlobalImpl(ROOT::NTupleSize_t globalIndex, void *to) final;
    void ReadInClusterImpl(RNTupleLocalIndex localIndex, void *to) final;
 
+   /// Used by RPairField and RTupleField descendants. These descendants have their own logic to attach the subfields
+   /// that ensure that the resulting memory layout matches std::pair or std::tuple, resp.
    RRecordField(std::string_view fieldName, std::string_view typeName);
 
    void AttachItemFields(std::vector<std::unique_ptr<RFieldBase>> itemFields);
@@ -106,6 +109,8 @@ protected:
 public:
    /// Construct a RRecordField based on a vector of child fields. The ownership of the child fields is transferred
    /// to the RRecordField instance.
+   /// The resulting field uses a memory layout for its values as if there was a struct consisting of the passed
+   /// item fields.
    RRecordField(std::string_view fieldName, std::vector<std::unique_ptr<RFieldBase>> itemFields);
    RRecordField(RRecordField &&other) = default;
    RRecordField &operator=(RRecordField &&other) = default;
@@ -136,6 +141,10 @@ protected:
    RPairField(std::string_view fieldName, std::array<std::unique_ptr<RFieldBase>, 2> itemFields,
               const std::array<std::size_t, 2> &offsets);
 
+   std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const final;
+
+   void ReconcileOnDiskField(const RNTupleDescriptor &desc) final;
+
 public:
    RPairField(std::string_view fieldName, std::array<std::unique_ptr<RFieldBase>, 2> itemFields);
    RPairField(RPairField &&other) = default;
@@ -186,6 +195,10 @@ protected:
    RTupleField(std::string_view fieldName, std::vector<std::unique_ptr<RFieldBase>> itemFields,
                const std::vector<std::size_t> &offsets);
 
+   std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const final;
+
+   void ReconcileOnDiskField(const RNTupleDescriptor &desc) final;
+
 public:
    RTupleField(std::string_view fieldName, std::vector<std::unique_ptr<RFieldBase>> itemFields);
    RTupleField(RTupleField &&other) = default;

tree/ntuple/inc/ROOT/RFieldBase.hxx

@@ -85,7 +85,6 @@ This is and can only be partially enforced through C++.
 */
 // clang-format on
 class RFieldBase {
-   friend class ROOT::RClassField;                             // to mark members as artificial
    friend class ROOT::Experimental::Detail::RRawPtrWriteEntry; // to call Append()
    friend struct ROOT::Internal::RFieldCallbackInjector;       // used for unit tests
    friend struct ROOT::Internal::RFieldRepresentationModifier; // used for unit tests
@@ -421,6 +420,11 @@ protected:
    static void CallConstructValueOn(const RFieldBase &other, void *where) { other.ConstructValue(where); }
    static std::unique_ptr<RDeleter> GetDeleterOf(const RFieldBase &other) { return other.GetDeleter(); }
 
+   /// Allow parents to mark their childs as artificial fields (used in class and record fields)
+   static void CallSetArtificialOn(RFieldBase &other) { other.SetArtificial(); }
+   /// Allow class fields to adjust the type alias of their members
+   static void SetTypeAliasOf(RFieldBase &other, const std::string &alias) { other.fTypeAlias = alias; }
+
    /// Operations on values of complex types, e.g. ones that involve multiple columns or for which no direct
    /// column type exists.
    virtual std::size_t AppendImpl(const void *from);

tree/ntuple/src/RField.cxx

@@ -27,7 +27,9 @@
 #include <functional>
 #include <iostream>
 #include <memory>
+#include <string_view>
 #include <type_traits>
+#include <unordered_set>
 
 std::unique_ptr<ROOT::RFieldBase> ROOT::RFieldZero::CloneImpl(std::string_view /*newName*/) const
 {
@@ -605,7 +607,27 @@ void ROOT::RRecordField::ReadInClusterImpl(RNTupleLocalIndex localIndex, void *t
 
 void ROOT::RRecordField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
 {
-   EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName | kDiffTypeVersion);
+   if (fTraits & kTraitEmulatedField) {
+      // The field has been explicitly constructed following the on-disk information. No further reconcilation needed.
+      return;
+   }
+   // Note that the RPairField and RTupleField descendants have their own reconcilation logic
+   R__ASSERT(GetTypeName().empty());
+
+   const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
+   EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName | kDiffTypeVersion);
+
+   // The on-disk ID of subfields is matched by field name. So we inherently support reordering of fields
+   // and we will ignore extra on-disk fields.
+   // It remains to mark the extra in-memory fields as artificial.
+   std::unordered_set<std::string_view> onDiskSubfields;
+   for (const auto &subField : desc.GetFieldIterable(fieldDesc)) {
+      onDiskSubfields.insert(subField.GetFieldName());
+   }
+   for (auto &f : fSubfields) {
+      if (onDiskSubfields.count(f->GetFieldName()) == 0)
+         CallSetArtificialOn(*f);
+   }
 }
 
 void ROOT::RRecordField::ConstructValue(void *where) const

tree/ntuple/src/RFieldMeta.cxx

@@ -174,9 +174,9 @@ ROOT::RClassField::RClassField(std::string_view fieldName, TClass *classp)
 
       const auto normTypeName = ROOT::Internal::GetNormalizedUnresolvedTypeName(origTypeName);
       if (normTypeName == subField->GetTypeName()) {
-         subField->fTypeAlias = "";
+         SetTypeAliasOf(*subField, "");
       } else {
-         subField->fTypeAlias = normTypeName;
+         SetTypeAliasOf(*subField, normTypeName);
       }
 
       fTraits &= subField->GetTraits();
@@ -190,7 +190,7 @@ ROOT::RClassField::~RClassField()
    if (fStagingArea) {
       for (const auto &[_, si] : fStagingItems) {
          if (!(si.fField->GetTraits() & kTraitTriviallyDestructible)) {
-            auto deleter = si.fField->GetDeleter();
+            auto deleter = GetDeleterOf(*si.fField);
             deleter->operator()(fStagingArea.get() + si.fOffset, true /* dtorOnly */);
          }
       }
@@ -467,7 +467,7 @@ void ROOT::RClassField::BeforeConnectPageSource(ROOT::Internal::RPageSource &pag
    // Iterate over all sub fields in memory and mark those as missing that are not in the descriptor.
    for (auto &field : fSubfields) {
       if (regularSubfields.count(field->GetFieldName()) == 0) {
-         field->SetArtificial();
+         CallSetArtificialOn(*field);
       }
    }
 }
@@ -624,6 +624,29 @@ ROOT::RPairField::RPairField(std::string_view fieldName, std::array<std::unique_
    fOffsets.push_back(secondElem->GetThisOffset());
 }
 
+std::unique_ptr<ROOT::RFieldBase> ROOT::RPairField::CloneImpl(std::string_view newName) const
+{
+   std::array<std::size_t, 2> offsets = {fOffsets[0], fOffsets[1]};
+   std::array<std::unique_ptr<RFieldBase>, 2> itemClones = {fSubfields[0]->Clone(fSubfields[0]->GetFieldName()),
+                                                            fSubfields[1]->Clone(fSubfields[1]->GetFieldName())};
+   return std::unique_ptr<RPairField>(new RPairField(newName, std::move(itemClones), offsets));
+}
+
+void ROOT::RPairField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
+{
+   static const std::vector<std::string> prefixes = {"std::pair<", "std::tuple<"};
+
+   const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
+   EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName);
+   EnsureMatchingTypePrefix(fieldDesc, prefixes);
+
+   const auto nOnDiskSubfields = fieldDesc.GetLinkIds().size();
+   if (nOnDiskSubfields != 2) {
+      throw ROOT::RException(
+         R__FAIL("invalid number of on-disk subfields for std::pair " + std::to_string(nOnDiskSubfields)));
+   }
+}
+
 //------------------------------------------------------------------------------
 
 ROOT::RProxiedCollectionField::RCollectionIterableOnce::RIteratorFuncs
@@ -1151,6 +1174,32 @@ ROOT::RTupleField::RTupleField(std::string_view fieldName, std::vector<std::uniq
    }
 }
 
+std::unique_ptr<ROOT::RFieldBase> ROOT::RTupleField::CloneImpl(std::string_view newName) const
+{
+   std::vector<std::unique_ptr<RFieldBase>> itemClones;
+   itemClones.reserve(fSubfields.size());
+   for (const auto &f : fSubfields) {
+      itemClones.emplace_back(f->Clone(f->GetFieldName()));
+   }
+   return std::unique_ptr<RTupleField>(new RTupleField(newName, std::move(itemClones), fOffsets));
+}
+
+void ROOT::RTupleField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
+{
+   static const std::vector<std::string> prefixes = {"std::pair<", "std::tuple<"};
+
+   const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
+   EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName);
+   EnsureMatchingTypePrefix(fieldDesc, prefixes);
+
+   const auto nOnDiskSubfields = fieldDesc.GetLinkIds().size();
+   const auto nSubfields = fSubfields.size();
+   if (nOnDiskSubfields != nSubfields) {
+      throw ROOT::RException(R__FAIL("invalid number of on-disk subfields for std::tuple " +
+                                     std::to_string(nOnDiskSubfields) + " vs. " + std::to_string(nSubfields)));
+   }
+}
+
 //------------------------------------------------------------------------------
 
 namespace {

tree/ntuple/test/ntuple_evolution_shape.cxx

@@ -1012,3 +1012,48 @@ struct StreamerField {
    EXPECT_EVALUATE_EQ("ptrStreamerField->fInt", 2);
    EXPECT_EVALUATE_EQ("ptrStreamerField->fAdded", 137);
 }
+
+TEST(RNTupleEvolution, RecordField)
+{
+   FileRaii fileGuard("test_ntuple_evolution_record_field.root");
+   {
+      std::vector<std::unique_ptr<RFieldBase>> items;
+      items.emplace_back(std::make_unique<RField<char>>("f1"));
+      items.emplace_back(std::make_unique<RField<float>>("f2"));
+      items.emplace_back(std::make_unique<RField<std::string>>("f3"));
+
+      auto model = ROOT::RNTupleModel::Create();
+      model->AddField(std::make_unique<ROOT::RRecordField>("r", std::move(items)));
+
+      auto writer = ROOT::RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath());
+
+      auto r = writer->GetModel().GetDefaultEntry().GetPtr<void>("r");
+      const auto &rf = static_cast<const ROOT::RRecordField &>(writer->GetModel().GetConstField("r"));
+
+      auto f1 = reinterpret_cast<char *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[0]);
+      auto f3 = reinterpret_cast<std::string *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[2]);
+      *f1 = 'x';
+      *f3 = "ROOT";
+
+      writer->Fill();
+   }
+
+   std::vector<std::unique_ptr<RFieldBase>> items;
+   items.emplace_back(std::make_unique<RField<std::string>>("f3"));
+   items.emplace_back(std::make_unique<RField<std::vector<float>>>("f4"));
+   items.emplace_back(std::make_unique<RField<int>>("f1"));
+
+   auto model = ROOT::RNTupleModel::Create();
+   model->AddField(std::make_unique<ROOT::RRecordField>("r", std::move(items)));
+
+   auto reader = RNTupleReader::Open(std::move(model), "ntpl", fileGuard.GetPath());
+   auto r = reader->GetModel().GetDefaultEntry().GetPtr<void>("r");
+   const auto &rf = static_cast<const ROOT::RRecordField &>(reader->GetModel().GetConstField("r"));
+   auto f3 = reinterpret_cast<std::string *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[0]);
+   auto f4 = reinterpret_cast<std::vector<float> *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[1]);
+   auto f1 = reinterpret_cast<int *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[2]);
+   reader->LoadEntry(0);
+   EXPECT_EQ("ROOT", *f3);
+   EXPECT_TRUE(f4->empty());
+   EXPECT_EQ('x', *f1);
+}

tree/ntuple/test/ntuple_evolution_type.cxx

@@ -4,6 +4,7 @@
 #include <new>
 #include <string>
 #include <string_view>
+#include <tuple>
 #include <utility>
 #include <variant>
 
@@ -160,3 +161,49 @@ TEST(RNTupleEvolution, CheckVariant)
    EXPECT_EQ('O', std::get<ROOT::RVec<int>>(v(1))[2]);
    EXPECT_EQ('T', std::get<ROOT::RVec<int>>(v(1))[3]);
 }
+
+TEST(RNTupleEvolution, CheckPairTuple)
+{
+   FileRaii fileGuard("test_ntuple_evolution_check_pair_tuple.root");
+   {
+      auto model = ROOT::RNTupleModel::Create();
+      auto p = model->MakeField<std::pair<char, float>>("p");
+      auto t2 = model->MakeField<std::tuple<char, float>>("t2");
+      model->MakeField<std::tuple<char, float, float>>("t3");
+      auto writer = ROOT::RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath());
+
+      *p = {1, 2.0};
+      *t2 = {3, 4.0};
+      writer->Fill();
+   }
+
+   auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath());
+
+   try {
+      reader->GetView<std::tuple<char>>("p");
+      FAIL() << "non-matching number of subfields for pair/tuple should throw";
+   } catch (const ROOT::RException &err) {
+      EXPECT_THAT(err.what(), testing::HasSubstr("invalid number of on-disk subfields"));
+   }
+
+   try {
+      reader->GetView<std::tuple<char, float, float>>("p");
+      FAIL() << "non-matching number of subfields for pair/tuple should throw";
+   } catch (const ROOT::RException &err) {
+      EXPECT_THAT(err.what(), testing::HasSubstr("invalid number of on-disk subfields"));
+   }
+
+   try {
+      reader->GetView<std::pair<char, float>>("t3");
+      FAIL() << "non-matching number of subfields for pair/tuple should throw";
+   } catch (const ROOT::RException &err) {
+      EXPECT_THAT(err.what(), testing::HasSubstr("invalid number of on-disk subfields"));
+   }
+
+   auto t2 = reader->GetView<std::pair<char, double>>("t2");
+   auto p = reader->GetView<std::tuple<int, double>>("p");
+   EXPECT_EQ(3, t2(0).first);
+   EXPECT_DOUBLE_EQ(4.0, t2(0).second);
+   EXPECT_EQ(1, std::get<0>(p(0)));
+   EXPECT_DOUBLE_EQ(2.0, std::get<1>(p(0)));
+}