[ntuple] Enable subfield access in REntry

tree/ntuple/v7/inc/ROOT/REntry.hxx

@@ -87,15 +87,15 @@ private:
 
    void AddValue(RFieldBase::RValue &&value)
    {
-      fFieldName2Token[value.GetField().GetFieldName()] = fValues.size();
+      fFieldName2Token[value.GetField().GetQualifiedFieldName()] = fValues.size();
       fValues.emplace_back(std::move(value));
    }
 
    /// While building the entry, adds a new value to the list and return the value's shared pointer
    template <typename T, typename... ArgsT>
    std::shared_ptr<T> AddValue(RField<T> &field, ArgsT &&...args)
    {
-      fFieldName2Token[field.GetFieldName()] = fValues.size();
+      fFieldName2Token[field.GetQualifiedFieldName()] = fValues.size();
       auto ptr = std::make_shared<T>(std::forward<ArgsT>(args)...);
       fValues.emplace_back(field.BindValue(ptr));
       return ptr;
@@ -136,7 +136,7 @@ private:
       if constexpr (!std::is_void_v<T>) {
          const auto &v = fValues[token.fIndex];
          if (v.GetField().GetTypeName() != RField<T>::TypeName()) {
-            throw RException(R__FAIL("type mismatch for field " + v.GetField().GetFieldName() + ": " +
+            throw RException(R__FAIL("type mismatch for field " + v.GetField().GetQualifiedFieldName() + ": " +
                                      v.GetField().GetTypeName() + " vs. " + RField<T>::TypeName()));
          }
       }

tree/ntuple/v7/inc/ROOT/RFieldBase.hxx

@@ -32,6 +32,7 @@ namespace ROOT {
 namespace Experimental {
 
 class RFieldBase;
+class RNTupleModel;
 
 namespace Internal {
 struct RFieldCallbackInjector;
@@ -65,6 +66,7 @@ This is and can only be partially enforced through C++.
 */
 // clang-format on
 class RFieldBase {
+   friend class RNTupleModel;
    friend struct ROOT::Experimental::Internal::RFieldCallbackInjector;       // used for unit tests
    friend struct ROOT::Experimental::Internal::RFieldRepresentationModifier; // used for unit tests
    friend void Internal::CallFlushColumnsOnField(RFieldBase &);
@@ -406,6 +408,13 @@ protected:
    /// Fields may need direct access to the principal column of their sub fields, e.g. in RRVecField::ReadBulk
    static Internal::RColumn *GetPrincipalColumnOf(const RFieldBase &other) { return other.fPrincipalColumn; }
 
+   /// Set the field's parent
+   void SetParent(RFieldBase *parent) { fParent = parent; }
+
+   /// Set a field's subfield pointer to the one provided. This subfield must already be present in fSubFields, i.e., it
+   /// is not possible to add brand new subfields and change the structure of the field this way.
+   void SetSubField(std::unique_ptr<RFieldBase> subField);
+
    /// Set a user-defined function to be called after reading a value, giving a chance to inspect and/or modify the
    /// value object.
    /// Returns an index that can be used to remove the callback.

tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx

@@ -293,6 +293,12 @@ public:
    /// Throws an exception if the field is null.
    void AddField(std::unique_ptr<RFieldBase> field);
 
+   /// Adds a subfield belonging to the provided parent, whose type is not known at compile time. `field` must be a
+   /// valid, existing subfield of `parent` (i.e., it is not possible to alter the parent's structure).
+   ///
+   /// Throws an exception if the field is null, or if it not a valid subfield of the provided parent.
+   void AddSubField(std::unique_ptr<RFieldBase> field, RFieldBase &parent);
+
    /// Adds a top-level field based on existing fields.
    RResult<void> AddProjectedField(std::unique_ptr<RFieldBase> field, FieldMappingFunc_t mapping);
    const RProjectedFields &GetProjectedFields() const { return *fProjectedFields; }

tree/ntuple/v7/src/RField.cxx

@@ -1197,6 +1197,22 @@ ROOT::Experimental::RFieldBase::EnsureCompatibleColumnTypes(const RNTupleDescrip
                             "(representation index: " + std::to_string(representationIndex) + ")"));
 }
 
+void ROOT::Experimental::RFieldBase::SetSubField(std::unique_ptr<RFieldBase> subField)
+{
+   if (fState != EState::kUnconnected)
+      throw RException(R__FAIL("invalid attempt to attach subfield to already connected field"));
+
+   for (auto &currSubField : fSubFields) {
+      if (currSubField->GetFieldName() == subField->GetFieldName() &&
+          currSubField->GetTypeName() == subField->GetTypeName()) {
+         std::swap(currSubField, subField);
+         return;
+      }
+   }
+
+   throw RException(R__FAIL("subfield \"" + subField->GetFieldName() + "\" not found"));
+}
+
 size_t ROOT::Experimental::RFieldBase::AddReadCallback(ReadCallback_t func)
 {
    fReadCallbacks.push_back(func);

tree/ntuple/v7/src/RNTupleModel.cxx

@@ -307,6 +307,22 @@ void ROOT::Experimental::RNTupleModel::AddField(std::unique_ptr<RFieldBase> fiel
    fFieldZero->Attach(std::move(field));
 }
 
+void ROOT::Experimental::RNTupleModel::AddSubField(std::unique_ptr<RFieldBase> field, RFieldBase &parent)
+{
+   EnsureNotFrozen();
+   if (!field)
+      throw RException(R__FAIL("null field"));
+   EnsureValidFieldName(field->GetFieldName());
+
+   field->SetParent(&parent);
+
+   if (fDefaultEntry)
+      fDefaultEntry->AddValue(field->CreateValue());
+   fFieldNames.insert(field->GetFieldName());
+
+   parent.SetSubField(std::move(field));
+}
+
 ROOT::Experimental::RResult<void>
 ROOT::Experimental::RNTupleModel::AddProjectedField(std::unique_ptr<RFieldBase> field, FieldMappingFunc_t mapping)
 {

tree/ntuple/v7/test/ntuple_basics.cxx

@@ -808,6 +808,58 @@ TEST(REntry, Basics)
    EXPECT_NE(&pt, e->GetPtr<void>("pt").get());
 }
 
+TEST(REntry, SubFields)
+{
+   FileRaii fileGuard("test_rentry_subfields");
+   {
+      auto model = RNTupleModel::Create();
+      model->MakeField<CustomStruct>("struct", CustomStruct{1.f, {2.f, 3.f}, {{4.f}, {5.f, 6.f, 7.f}}, "foo"});
+
+      auto ntuple = RNTupleWriter::Recreate(std::move(model), "ntuple", fileGuard.GetPath());
+      ntuple->Fill();
+   }
+
+   auto model = RNTupleModel::Create();
+   auto fldStruct = RFieldBase::Create("struct", "CustomStruct").Unwrap();
+   auto fldA = RFieldBase::Create("a", "float").Unwrap();
+
+   model->AddSubField(std::move(fldA), *fldStruct);
+
+   try {
+      auto tmpModel = model->Clone();
+      auto fldInvalidSubField = RFieldBase::Create("doesnotexist", "float").Unwrap();
+      tmpModel->AddSubField(std::move(fldInvalidSubField), *fldStruct);
+      FAIL() << "should not be able to add a subfield not present in the provided parent";
+   } catch (const RException &err) {
+      EXPECT_THAT(err.what(), testing::HasSubstr("subfield \"doesnotexist\" not found"));
+   }
+
+   model->AddField(std::move(fldStruct));
+
+   auto &entry = model->GetDefaultEntry();
+   auto ntuple = RNTupleReader::Open(std::move(model), "ntuple", fileGuard.GetPath());
+   ntuple->LoadEntry(0);
+
+   CustomStruct expectedStruct{1.f, {2.f, 3.f}, {{4.f}, {5.f, 6.f, 7.f}}, "foo"};
+
+   EXPECT_EQ(expectedStruct, *entry.GetPtr<CustomStruct>("struct"));
+   EXPECT_FLOAT_EQ(1.f, *entry.GetPtr<float>("struct.a"));
+
+   auto tokenStruct = entry.GetToken("struct");
+   EXPECT_EQ(expectedStruct, *entry.GetPtr<CustomStruct>(tokenStruct));
+
+   auto tokenStructA = entry.GetToken("struct.a");
+   EXPECT_FLOAT_EQ(1.f, *entry.GetPtr<float>(tokenStructA));
+
+   // sanity check
+   try {
+      *entry.GetPtr<std::vector<float>>("v1");
+      FAIL() << "subfields not explicitly added to the model shouldn't be present in the model";
+   } catch (const RException &err) {
+      EXPECT_THAT(err.what(), testing::HasSubstr("invalid field name: v1"));
+   }
+}
+
 TEST(RFieldBase, CreateObject)
 {
    auto ptrInt = RField<int>("name").CreateObject<int>();