From c4a49705ee5ed83c099314e64a37489928d1530b Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 28 Aug 2026 00:17:35 +0200 Subject: [PATCH 1/3] [ntuple] make RPageSourceFile::LoadStreamerInfo() idempotent (cherry picked from commit 31ae4a2649a35c1dc1f6d9b8aa1962b9766f13b1) --- tree/ntuple/inc/ROOT/RPageStorageFile.hxx | 2 ++ tree/ntuple/src/RPageStorageFile.cxx | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx index 21f17d8eb3812..214c60a31a31c 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx @@ -152,6 +152,8 @@ private: RNTupleDescriptorBuilder fDescriptorBuilder; /// Populated by LoadStructureImpl(), reset at the end of Attach() RStructureBuffer fStructureBuffer; + /// Set to true after the first call to LoadStreamerInfo() + bool fHasStreamerInfo = false; RPageSourceFile(std::string_view ntupleName, const ROOT::RNTupleReadOptions &options); diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 4ce441c96fcd1..190cd35a49374 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -564,6 +564,7 @@ std::unique_ptr ROOT::Internal::RPageSourceFile::Cl auto clone = new RPageSourceFile(fNTupleName, fOptions); clone->fFile = fFile->Clone(); clone->fReader = ROOT::Internal::RMiniFileReader(clone->fFile.get()); + clone->fHasStreamerInfo = fHasStreamerInfo; return std::unique_ptr(clone); } @@ -750,5 +751,9 @@ ROOT::Internal::RPageSourceFile::LoadClusters(std::span clusterK void ROOT::Internal::RPageSourceFile::LoadStreamerInfo() { + if (fHasStreamerInfo) + return; + fReader.LoadStreamerInfo(); + fHasStreamerInfo = true; } From 16b0a59e561ed6cdd5bc733e1da994ffecd6b9f0 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 28 Aug 2026 00:18:14 +0200 Subject: [PATCH 2/3] [ntuple] load streamer info for schema evolution If during schema evolution a conversion streamer info is required, we have to load the schema info records from the ROOT file. If the RNTuple is loaded through the RMiniFile, this requires an extra call. (cherry picked from commit 8888c5537289e1d39b945b8ae927e7aad8a357d8) --- .../ntuple/evolution/NtplEvolv_v3_LinkDef.h | 3 ++- .../root/ntuple/evolution/read_ntplevolv.cxx | 25 ++++++++++++++++++- tree/ntuple/src/RFieldMeta.cxx | 4 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/roottest/root/ntuple/evolution/NtplEvolv_v3_LinkDef.h b/roottest/root/ntuple/evolution/NtplEvolv_v3_LinkDef.h index 4ceada4adf165..656309f9b83f4 100644 --- a/roottest/root/ntuple/evolution/NtplEvolv_v3_LinkDef.h +++ b/roottest/root/ntuple/evolution/NtplEvolv_v3_LinkDef.h @@ -2,6 +2,7 @@ #pragma link C++ class NtplEvolv+; -#pragma read sourceClass="NtplEvolv" version="[1-]" source="" targetClass="NtplEvolv" target="fA" code = "{ fA = 13; }" +#pragma read sourceClass="NtplEvolv" version="[1-]" source="int fA;" targetClass="NtplEvolv" target="fA" \ + code = "{ fA = onfile.fA + 13; }" #endif diff --git a/roottest/root/ntuple/evolution/read_ntplevolv.cxx b/roottest/root/ntuple/evolution/read_ntplevolv.cxx index 8819c75db45a2..2714691107b7f 100644 --- a/roottest/root/ntuple/evolution/read_ntplevolv.cxx +++ b/roottest/root/ntuple/evolution/read_ntplevolv.cxx @@ -1,17 +1,40 @@ #include +#include +#include +#include + #include "NtplEvolv_v3.hxx" #include +#include int main() { + // At this point, we expect NtplEvolv _not_ being present in the global streamer infos + for (auto si : TRangeDynCast(gROOT->GetListOfStreamerInfo())) { + if (std::string(si->GetName()) == "NtplEvolv") + return 2; + } + auto reader = ROOT::RNTupleReader::Open("ntpl", "root_test_ntpl_evolution.root"); + reader->GetModel(); + // Now, reader should have loaded the streamer info for NtplEvolv + bool streamerInfoFound = false; + for (auto si : TRangeDynCast(gROOT->GetListOfStreamerInfo())) { + if (std::string(si->GetName()) == "NtplEvolv") { + streamerInfoFound = true; + break; + } + } + if (!streamerInfoFound) + return 3; + reader->LoadEntry(0); auto a = reader->GetModel().GetDefaultEntry().GetPtr("event")->fA; std::cout << "Result of event.fA: " << a << std::endl; - return a != 13; + return a != 14; } diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 621ce474be38b..463f7882bd036 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -553,6 +553,10 @@ std::unique_ptr ROOT::RClassField::BeforeConnectPageSource(ROO // A staging class (conversion streamer info) only exists if there is at least one rule that has an // on disk source member defined. if (hasSources) { + if (fieldDesc.GetTypeVersion() != GetTypeVersion() || fieldDesc.GetTypeName() != GetTypeName()) { + // We need the on-disk streamer info for the conversion streamer info + pageSource.LoadStreamerInfo(); + } SetStagingClass(fieldDesc.GetTypeName(), fieldDesc.GetTypeVersion()); PrepareStagingArea(rules, desc, fieldDesc); for (auto &[_, si] : fStagingItems) { From c2c8519626905941ad551c3c747a216ef20695ac Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Sun, 30 Aug 2026 23:34:29 +0200 Subject: [PATCH 3/3] [ntuple] fix schema evolution of unversioned classes (cherry picked from commit c0423bd685dac967b67c2a649ba2642576ff7680) --- .../root/ntuple/unversioned/CMakeLists.txt | 38 +++++++++++++++++++ .../root/ntuple/unversioned/NtplHit_New.hxx | 11 ++++++ .../ntuple/unversioned/NtplHit_New_LinkDef.h | 9 +++++ .../root/ntuple/unversioned/NtplHit_Old.hxx | 11 ++++++ .../ntuple/unversioned/NtplHit_Old_LinkDef.h | 5 +++ .../root/ntuple/unversioned/read_ntplhit.cxx | 31 +++++++++++++++ .../root/ntuple/unversioned/write_ntplhit.cxx | 25 ++++++++++++ tree/ntuple/src/RFieldMeta.cxx | 16 +++++++- 8 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 roottest/root/ntuple/unversioned/CMakeLists.txt create mode 100644 roottest/root/ntuple/unversioned/NtplHit_New.hxx create mode 100644 roottest/root/ntuple/unversioned/NtplHit_New_LinkDef.h create mode 100644 roottest/root/ntuple/unversioned/NtplHit_Old.hxx create mode 100644 roottest/root/ntuple/unversioned/NtplHit_Old_LinkDef.h create mode 100644 roottest/root/ntuple/unversioned/read_ntplhit.cxx create mode 100644 roottest/root/ntuple/unversioned/write_ntplhit.cxx diff --git a/roottest/root/ntuple/unversioned/CMakeLists.txt b/roottest/root/ntuple/unversioned/CMakeLists.txt new file mode 100644 index 0000000000000..066d86c52be09 --- /dev/null +++ b/roottest/root/ntuple/unversioned/CMakeLists.txt @@ -0,0 +1,38 @@ +ROOTTEST_GENERATE_DICTIONARY( + ntplhit_old_dict + ${CMAKE_CURRENT_SOURCE_DIR}/NtplHit_Old.hxx + LINKDEF ${CMAKE_CURRENT_SOURCE_DIR}/NtplHit_Old_LinkDef.h + NO_ROOTMAP NO_CXXMODULE + FIXTURES_SETUP generated_ntplhit_old_dictionary +) + +ROOTTEST_GENERATE_EXECUTABLE( + write_ntplhit + write_ntplhit.cxx ntplhit_old_dict.cxx + LIBRARIES Core RIO ROOTNTuple + FIXTURES_REQUIRED generated_ntplhit_old_dictionary + FIXTURES_SETUP write_ntplhit_excutable) + +ROOTTEST_ADD_TEST(write_ntplhit + EXEC ./write_ntplhit + FIXTURES_REQUIRED write_ntplhit_excutable + FIXTURES_SETUP write_ntplhit_done) + +ROOTTEST_GENERATE_DICTIONARY( + ntplhit_new_dict + ${CMAKE_CURRENT_SOURCE_DIR}/NtplHit_New.hxx + LINKDEF ${CMAKE_CURRENT_SOURCE_DIR}/NtplHit_New_LinkDef.h + NO_ROOTMAP NO_CXXMODULE + FIXTURES_SETUP generated_ntplhit_new_dictionary +) + +ROOTTEST_GENERATE_EXECUTABLE( + read_ntplhit + read_ntplhit.cxx ntplhit_new_dict.cxx + LIBRARIES Core RIO ROOTNTuple + FIXTURES_REQUIRED generated_ntplhit_new_dictionary + FIXTURES_SETUP read_ntplhit_excutable) + +ROOTTEST_ADD_TEST(read_ntplhit + EXEC ./read_ntplhit + FIXTURES_REQUIRED read_ntplhit_excutable write_ntplhit_done) diff --git a/roottest/root/ntuple/unversioned/NtplHit_New.hxx b/roottest/root/ntuple/unversioned/NtplHit_New.hxx new file mode 100644 index 0000000000000..33ea61c827e9e --- /dev/null +++ b/roottest/root/ntuple/unversioned/NtplHit_New.hxx @@ -0,0 +1,11 @@ +#ifndef NTPL_HIT_NEW_H +#define NTPL_HIT_NEW_H + +struct Hit { + int fA_r{0}; + int fB_r{0}; + double fX_r{0}; + double fY_r{0}; +}; + +#endif // NTPL_HIT_NEW_H diff --git a/roottest/root/ntuple/unversioned/NtplHit_New_LinkDef.h b/roottest/root/ntuple/unversioned/NtplHit_New_LinkDef.h new file mode 100644 index 0000000000000..964508a83e4d0 --- /dev/null +++ b/roottest/root/ntuple/unversioned/NtplHit_New_LinkDef.h @@ -0,0 +1,9 @@ +#ifdef __ROOTCLING__ + +#pragma link C++ class Hit+; + +#pragma read sourceClass="Hit" targetClass="Hit" checksum="[2391364433]" \ + source="int fA; int fB; double fX; double fY" target="fA_r,fB_r,fX_r,fY_r" \ + code="{ fA_r = onfile.fA; fB_r = onfile.fB; fX_r = onfile.fX; fY_r = onfile.fY; }" + +#endif diff --git a/roottest/root/ntuple/unversioned/NtplHit_Old.hxx b/roottest/root/ntuple/unversioned/NtplHit_Old.hxx new file mode 100644 index 0000000000000..514e15858acab --- /dev/null +++ b/roottest/root/ntuple/unversioned/NtplHit_Old.hxx @@ -0,0 +1,11 @@ +#ifndef NTPL_HIT_OLD_H +#define NTPL_HIT_OLD_H + +struct Hit { + int fA{0}; + int fB{0}; + double fX{0}; + double fY{0}; +}; + +#endif // NTPL_HIT_OLD_H diff --git a/roottest/root/ntuple/unversioned/NtplHit_Old_LinkDef.h b/roottest/root/ntuple/unversioned/NtplHit_Old_LinkDef.h new file mode 100644 index 0000000000000..f5d7ed1387afa --- /dev/null +++ b/roottest/root/ntuple/unversioned/NtplHit_Old_LinkDef.h @@ -0,0 +1,5 @@ +#ifdef __ROOTCLING__ + +#pragma link C++ class Hit+; + +#endif diff --git a/roottest/root/ntuple/unversioned/read_ntplhit.cxx b/roottest/root/ntuple/unversioned/read_ntplhit.cxx new file mode 100644 index 0000000000000..021d53a7d0b43 --- /dev/null +++ b/roottest/root/ntuple/unversioned/read_ntplhit.cxx @@ -0,0 +1,31 @@ +#include + +#include +#include + +#include "NtplHit_New.hxx" + +static int gFails = 0; + +static void Check(char const *m, double want, double got) +{ + bool ok = want == got; + if (!ok) + ++gFails; + printf("%-4s want %-4g got %-22.17g %s\n", m, want, got, ok ? "ok" : "WRONG"); +} + +int main() +{ + auto r = ROOT::RNTupleReader::Open("r", "root_test_ntpl_unversioned.root"); + auto v = r->GetModel().GetDefaultEntry().GetPtr>("hits"); + + r->LoadEntry(0); + + Check("fA_r", 1, v->at(0).fA_r); + Check("fB_r", 2, v->at(0).fB_r); + Check("fX_r", 1.5, v->at(0).fX_r); + Check("fY_r", 2.5, v->at(0).fY_r); + + return gFails; +} diff --git a/roottest/root/ntuple/unversioned/write_ntplhit.cxx b/roottest/root/ntuple/unversioned/write_ntplhit.cxx new file mode 100644 index 0000000000000..f2586a407daf6 --- /dev/null +++ b/roottest/root/ntuple/unversioned/write_ntplhit.cxx @@ -0,0 +1,25 @@ +#include +#include + +#include + +#include +#include + +#include "NtplHit_Old.hxx" + +int main() +{ + Hit h; + h.fA = 1; + h.fB = 2; + h.fX = 1.5; + h.fY = 2.5; + auto model = ROOT::RNTupleModel::Create(); + auto pv = model->MakeField>("hits"); + auto w = ROOT::RNTupleWriter::Recreate(std::move(model), "r", "root_test_ntpl_unversioned.root"); + *pv = {h}; + w->Fill(); + printf("CHECKSUM %u\n", TClass::GetClass("Hit")->GetCheckSum()); + return 0; +} diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 463f7882bd036..93e364dccf548 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -553,11 +553,23 @@ std::unique_ptr ROOT::RClassField::BeforeConnectPageSource(ROO // A staging class (conversion streamer info) only exists if there is at least one rule that has an // on disk source member defined. if (hasSources) { - if (fieldDesc.GetTypeVersion() != GetTypeVersion() || fieldDesc.GetTypeName() != GetTypeName()) { + // For unversioned classes, the in-memory layout may by chance have the same transient version number + // than the recorded (transient, at the time of writing) on-disk version. Therefore, we also need to compare + // the checksums to find out if we need a conversion streamer info. + std::uint32_t assignedVersionForOnDiskLayout = fieldDesc.GetTypeVersion(); + R__ASSERT(fieldDesc.GetTypeChecksum()); + if (fieldDesc.GetTypeVersion() != GetTypeVersion() || *fieldDesc.GetTypeChecksum() != fClass->GetCheckSum() || + fieldDesc.GetTypeName() != GetTypeName()) { // We need the on-disk streamer info for the conversion streamer info pageSource.LoadStreamerInfo(); + + auto oldCl = TClass::GetClass(fieldDesc.GetTypeName().c_str()); + R__ASSERT(oldCl); + auto onDiskStreamerInfo = oldCl->FindStreamerInfo(*fieldDesc.GetTypeChecksum()); + R__ASSERT(onDiskStreamerInfo); + assignedVersionForOnDiskLayout = onDiskStreamerInfo->GetClassVersion(); } - SetStagingClass(fieldDesc.GetTypeName(), fieldDesc.GetTypeVersion()); + SetStagingClass(fieldDesc.GetTypeName(), assignedVersionForOnDiskLayout); PrepareStagingArea(rules, desc, fieldDesc); for (auto &[_, si] : fStagingItems) { Internal::CallConnectPageSourceOnField(*si.fField, pageSource);