diff --git a/tree/ntuple/inc/ROOT/RField.hxx b/tree/ntuple/inc/ROOT/RField.hxx index a826a3701dc5b..f878e914634c8 100644 --- a/tree/ntuple/inc/ROOT/RField.hxx +++ b/tree/ntuple/inc/ROOT/RField.hxx @@ -238,6 +238,8 @@ protected: // Returns the list of seen streamer infos ROOT::RExtraTypeInfoDescriptor GetExtraTypeInfo() const final; + void BeforeConnectPageSource(ROOT::Internal::RPageSource &pageSource) final; + public: RStreamerField(std::string_view fieldName, std::string_view className, std::string_view typeAlias = ""); RStreamerField(RStreamerField &&other) = default; diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 039188f9c61bc..6fcf41f646128 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -612,6 +612,7 @@ private: REntryRange fEntryRange; ///< Used by the cluster pool to prevent reading beyond the given range bool fHasStructure = false; ///< Set to true once `LoadStructure()` is called bool fIsAttached = false; ///< Set to true once `Attach()` is called + bool fHasStreamerInfosRegistered = false; ///< Set to true when RegisterStreamerInfos() is called. /// Remembers the last cluster id from which a page was requested ROOT::DescriptorId_t fLastUsedCluster = ROOT::kInvalidDescriptorId; @@ -817,6 +818,10 @@ public: // TODO(gparolini): for symmetry with SealPage(), we should either make this private or SealPage() public. RResult UnsealPage(const RSealedPage &sealedPage, const ROOT::Internal::RColumnElementBase &element); + + /// Builds the streamer info records from the descriptor's extra type info section. This is necessary when + /// connecting streamer fields so that emulated classes can be read. + void RegisterStreamerInfos(); }; // class RPageSource } // namespace Internal diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 1311841fe7c03..461551e7d74b1 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -834,6 +834,11 @@ ROOT::RStreamerField::RStreamerField(std::string_view fieldName, TClass *classp) fTraits |= kTraitTriviallyDestructible; } +void ROOT::RStreamerField::BeforeConnectPageSource(ROOT::Internal::RPageSource &pageSource) +{ + pageSource.RegisterStreamerInfos(); +} + std::unique_ptr ROOT::RStreamerField::CloneImpl(std::string_view newName) const { return std::unique_ptr(new RStreamerField(newName, GetTypeName(), GetTypeAlias())); diff --git a/tree/ntuple/src/RNTupleSerialize.cxx b/tree/ntuple/src/RNTupleSerialize.cxx index 520fbc5c212cd..aab44d7c2a584 100644 --- a/tree/ntuple/src/RNTupleSerialize.cxx +++ b/tree/ntuple/src/RNTupleSerialize.cxx @@ -2146,13 +2146,12 @@ ROOT::Internal::RNTupleSerializer::DeserializeStreamerInfos(const std::string &e TBufferFile buffer(TBuffer::kRead, extraTypeInfoContent.length(), const_cast(extraTypeInfoContent.data()), false /* adopt */); auto infoList = reinterpret_cast(buffer.ReadObject(TList::Class())); - infoList->SetOwner(); // delete the TStreamerInfo items of the list TObjLink *lnk = infoList->FirstLink(); while (lnk) { auto info = reinterpret_cast(lnk->GetObject()); info->BuildCheck(); - infoMap[info->GetNumber()] = info->GetClass()->GetStreamerInfo(); + infoMap[info->GetNumber()] = info->GetClass()->GetStreamerInfo(info->GetClassVersion()); assert(info->GetNumber() == infoMap[info->GetNumber()]->GetNumber()); lnk = lnk->Next(); } diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 1652f7ca3f4f4..0300a7501d2a1 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -589,6 +589,22 @@ ROOT::RResult ROOT::Internal::RPageSource::UnsealPage(con return page; } +void ROOT::Internal::RPageSource::RegisterStreamerInfos() +{ + if (fHasStreamerInfosRegistered) + return; + + for (const auto &extraTypeInfo : fDescriptor.GetExtraTypeInfoIterable()) { + if (extraTypeInfo.GetContentId() != EExtraTypeInfoIds::kStreamerInfo) + continue; + // We don't need the result, it's enough that during deserialization, BuildCheck() is called for every + // streamer info record. + RNTupleSerializer::DeserializeStreamerInfos(extraTypeInfo.GetContent()).Unwrap(); + } + + fHasStreamerInfosRegistered = true; +} + //------------------------------------------------------------------------------ bool ROOT::Internal::RWritePageMemoryManager::RColumnInfo::operator>(const RColumnInfo &other) const diff --git a/tree/ntuple/test/StreamerField.hxx b/tree/ntuple/test/StreamerField.hxx index a8c09eaffa086..f25905c5cb9ca 100644 --- a/tree/ntuple/test/StreamerField.hxx +++ b/tree/ntuple/test/StreamerField.hxx @@ -54,4 +54,22 @@ struct PolyContainer { std::unique_ptr fPoly; }; +template +struct OldStreamerName { + T fValue; +}; + +template +struct NewStreamerName { + T fValue; +}; + +struct TemperatureCelsius { + float fValue; +}; + +struct TemperatureKelvin { + float fValue; +}; + #endif // ROOT_RNTuple_Test_StreamerField diff --git a/tree/ntuple/test/StreamerFieldLinkDef.h b/tree/ntuple/test/StreamerFieldLinkDef.h index e7d81303ec028..b01191c905ab3 100644 --- a/tree/ntuple/test/StreamerFieldLinkDef.h +++ b/tree/ntuple/test/StreamerFieldLinkDef.h @@ -12,4 +12,11 @@ #pragma link C++ class PolyB + ; #pragma link C++ options = rntupleStreamerMode(true) class PolyContainer + ; +#pragma link C++ options = rntupleStreamerMode(true), version(3) class OldStreamerName < int> + ; +#pragma link C++ options = rntupleStreamerMode(true), version(3) class NewStreamerName < int> + ; +#pragma read sourceClass = "OldStreamerName" targetClass = "NewStreamerName" version = "[3]" + +#pragma link C++ options = rntupleStreamerMode(true) class TemperatureCelsius + ; +#pragma link C++ options = rntupleStreamerMode(true) class TemperatureKelvin + ; + #endif diff --git a/tree/ntuple/test/ntuple_evolution.cxx b/tree/ntuple/test/ntuple_evolution.cxx index e59967e315fbc..e1307b079cfb1 100644 --- a/tree/ntuple/test/ntuple_evolution.cxx +++ b/tree/ntuple/test/ntuple_evolution.cxx @@ -863,3 +863,60 @@ struct RenamedIntermediateDerived : public RenamedIntermediate2 { EXPECT_THAT(err.what(), testing::HasSubstr("incompatible type name for field")); } } + +TEST(RNTupleEvolution, StreamerField) +{ + FileRaii fileGuard("test_ntuple_evolution_streamer_field.root"); + + ExecInFork([&] { + // The child process writes the file and exits, but the file must be preserved to be read by the parent. + fileGuard.PreserveFile(); + + ASSERT_TRUE(gInterpreter->Declare(R"( +struct StreamerField { + int fInt = 1; + int fAnotherInt = 3; + + ClassDefNV(StreamerField, 2) +}; +)")); + + auto model = RNTupleModel::Create(); + model->AddField(std::make_unique("f", "StreamerField")); + + auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); + writer->Fill(); + + void *ptr = writer->GetModel().GetDefaultEntry().GetPtr("f").get(); + DeclarePointer("StreamerField", "ptrStreamerField", ptr); + ProcessLine("ptrStreamerField->fInt = 2;"); + writer->Fill(); + + // Reset / close the writer and flush the file. + writer.reset(); + }); + + ASSERT_TRUE(gInterpreter->Declare(R"( +struct StreamerField { + int fInt = 0; + int fAdded = 137; + // removed fAnotherInt + + ClassDefNV(StreamerField, 3) +}; +)")); + + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + ASSERT_EQ(2, reader->GetNEntries()); + + void *ptr = reader->GetModel().GetDefaultEntry().GetPtr("f").get(); + DeclarePointer("StreamerField", "ptrStreamerField", ptr); + + reader->LoadEntry(0); + EXPECT_EVALUATE_EQ("ptrStreamerField->fInt", 1); + EXPECT_EVALUATE_EQ("ptrStreamerField->fAdded", 137); + + reader->LoadEntry(1); + EXPECT_EVALUATE_EQ("ptrStreamerField->fInt", 2); + EXPECT_EVALUATE_EQ("ptrStreamerField->fAdded", 137); +} diff --git a/tree/ntuple/test/rfield_streamer.cxx b/tree/ntuple/test/rfield_streamer.cxx index 57d34982a90c3..7ee35bb2b9882 100644 --- a/tree/ntuple/test/rfield_streamer.cxx +++ b/tree/ntuple/test/rfield_streamer.cxx @@ -305,3 +305,53 @@ TEST(RField, StreamerMergeIncremental) EXPECT_TRUE(seenStreamerInfos[2]); EXPECT_TRUE(seenStreamerInfos[3]); } + +TEST(RField, StreamerSchemaEvolution) +{ + FileRaii fileGuard("test_ntuple_rfield_streamer_schema_evolution.root"); + { + auto model = RNTupleModel::Create(); + model->AddField(RFieldBase::Create("f", "OldStreamerName").Unwrap()); + auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); + auto ptrF = writer->GetModel().GetDefaultEntry().GetPtr>("f"); + ptrF->fValue = 137; + writer->Fill(); + } + + auto model = RNTupleModel::Create(); + model->AddField(RFieldBase::Create("f", "NewStreamerName").Unwrap()); + auto reader = RNTupleReader::Open(std::move(model), "ntpl", fileGuard.GetPath()); + + ASSERT_EQ(1U, reader->GetNEntries()); + auto ptrF = reader->GetModel().GetDefaultEntry().GetPtr>("f"); + reader->LoadEntry(0); + EXPECT_EQ(137, ptrF->fValue); +} + +TEST(RField, StreamerClassMismatch) +{ + FileRaii fileGuard("test_ntuple_rfield_streamer_class_mismatch.root"); + { + auto model = RNTupleModel::Create(); + model->AddField(RFieldBase::Create("f", "TemperatureCelsius").Unwrap()); + auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); + auto ptrF = writer->GetModel().GetDefaultEntry().GetPtr("f"); + ptrF->fValue = 100.; + writer->Fill(); + } + + auto model = RNTupleModel::Create(); + model->AddField(RFieldBase::Create("f", "TemperatureKelvin").Unwrap()); + auto reader = RNTupleReader::Open(std::move(model), "ntpl", fileGuard.GetPath()); + + ASSERT_EQ(1U, reader->GetNEntries()); + auto ptrF = reader->GetModel().GetDefaultEntry().GetPtr("f"); + + // TODO(jblomer): this should fail with an exception when we connect the page source + ROOT::TestSupport::CheckDiagsRAII diagRAII; + diagRAII.requiredDiag(kError, "TBufferFile::ReadVersion", "Could not find the StreamerInfo with a checksum of", + false /* matchFullMessage */); + diagRAII.requiredDiag(kError, "TBufferFile::CheckByteCount", "object of class TemperatureKelvin read too few bytes", + false /* matchFullMessage */); + reader->LoadEntry(0); +}