diff --git a/roottest/root/io/hadd/CMakeLists.txt b/roottest/root/io/hadd/CMakeLists.txt index dc25e3d81b16b..ebbde6dd99ab5 100644 --- a/roottest/root/io/hadd/CMakeLists.txt +++ b/roottest/root/io/hadd/CMakeLists.txt @@ -270,3 +270,14 @@ ROOTTEST_ADD_TEST(test_hadd_regr_20872_2 PASSREGEX "root://eospublic.cern.ch//eos/root-eos/h1/dstarmb.root cannot be both the target and an input!" ) endif() + +# Verify that the hadd (or rather, the RNTupleMerger) is able to cope with RNTuples written before schema version +# 1.0.0.1 (meaning they have the wrong type name normalization) +configure_file(test_hadd_merge_rntuple_634_1.root . COPYONLY) +configure_file(test_hadd_merge_rntuple_634_2.root . COPYONLY) +configure_file(check_merge_rntuple_634.C . COPYONLY) +ROOTTEST_ADD_TEST(test_hadd_merge_rntuple_634, + PRECMD ${CMAKE_COMMAND} -E rm -f test_hadd_merge_rntuple_634_merged.root + COMMAND ${ROOT_hadd_CMD} -f test_hadd_merge_rntuple_634_merged.root test_hadd_merge_rntuple_634_1.root test_hadd_merge_rntuple_634_2.root + POSTCMD ${ROOT_root_CMD} -q check_merge_rntuple_634.C + PASSRC 0) diff --git a/roottest/root/io/hadd/check_merge_rntuple_634.C b/roottest/root/io/hadd/check_merge_rntuple_634.C new file mode 100644 index 0000000000000..81a089c0d7ee4 --- /dev/null +++ b/roottest/root/io/hadd/check_merge_rntuple_634.C @@ -0,0 +1,16 @@ +int check_merge_rntuple_634() +{ + // Verify that the given RNTuple is readable + auto reader = ROOT::RNTupleReader::Open("Events", "test_hadd_merge_rntuple_634_merged.root"); + const auto &model = reader->GetModel(); + const auto &desc = reader->GetDescriptor(); + for (const auto &fdesc : desc.GetFieldIterable(desc.GetFieldZeroId())) { + if (fdesc.GetTypeName() != ROOT::Internal::GetRenormalizedTypeName(fdesc.GetTypeName())) { + std::cerr << "Type name is not renormalized! " << fdesc.GetTypeName() << " vs " << + ROOT::Internal::GetRenormalizedTypeName(fdesc.GetTypeName()) << "\n"; + return 1; + } + } + + return 0; +} diff --git a/roottest/root/io/hadd/test_hadd_merge_rntuple_634_1.root b/roottest/root/io/hadd/test_hadd_merge_rntuple_634_1.root new file mode 100644 index 0000000000000..fa6afc42f5b3a Binary files /dev/null and b/roottest/root/io/hadd/test_hadd_merge_rntuple_634_1.root differ diff --git a/roottest/root/io/hadd/test_hadd_merge_rntuple_634_2.root b/roottest/root/io/hadd/test_hadd_merge_rntuple_634_2.root new file mode 100644 index 0000000000000..a97d966d70440 Binary files /dev/null and b/roottest/root/io/hadd/test_hadd_merge_rntuple_634_2.root differ diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index c41cb2b0238a7..1f7eddd28e7fb 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -46,6 +46,12 @@ namespace ROOT { class RNTupleModel; +namespace Experimental { +namespace Internal { +class RNTupleMerger; +} +} // namespace Experimental + namespace Internal { class RPageAllocator; @@ -445,6 +451,8 @@ public: */ // clang-format on class RPagePersistentSink : public RPageSink { + friend class ROOT::Experimental::Internal::RNTupleMerger; + private: /// Used to map the IDs of the descriptor to the physical IDs issued during header/footer serialization ROOT::Internal::RNTupleSerializer::RContext fSerializationContext; diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index aae2b1e5243c5..eeaaa0101fed9 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -1331,6 +1331,23 @@ ROOT::RResult ROOT::Internal::RNTupleDescriptorBuilder::AddClusterGroup(RC void ROOT::Internal::RNTupleDescriptorBuilder::SetSchemaFromExisting(const RNTupleDescriptor &descriptor) { fDescriptor = descriptor.CloneSchema(); + + if (descriptor.fVersionMajor == 0 && descriptor.fVersionMinor == 0 && descriptor.fVersionPatch < 1) { + // In case we are copying the schema from a pre-1.0.0.1 RNTuple we need to patch all field type names + // to use the proper normalization. + std::vector toVisit; + toVisit.push_back(fDescriptor.GetFieldZeroId()); + while (!toVisit.empty()) { + auto fieldId = toVisit.back(); + toVisit.pop_back(); + for (auto &field : fDescriptor.GetFieldIterable(fieldId)) { + auto prev = field.fTypeName; + const_cast(field).fTypeName = + ROOT::Internal::GetRenormalizedTypeName(field.fTypeName); + toVisit.push_back(field.GetId()); + } + } + } } void ROOT::Internal::RNTupleDescriptorBuilder::BeginHeaderExtension() diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index 3c7cadc71b76b..96e2eac1400c6 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -530,7 +530,8 @@ CompareDescriptorStructure(const ROOT::RNTupleDescriptor &dst, const ROOT::RNTup // Require that fields types match // TODO(gparolini): allow non-identical but compatible types - const auto &srcTyName = field.fSrc->GetTypeName(); + const auto &srcTyName = ROOT::Internal::GetRenormalizedTypeName(field.fSrc->GetTypeName()); + // This is already renormalized by construction (see RNTupleDescriptorBuilder::SetSchemaFromExisting) const auto &dstTyName = field.fDst->GetTypeName(); if (srcTyName != dstTyName) { std::stringstream ss; @@ -1147,8 +1148,8 @@ static void AddColumnsFromField(std::vector &columns, const RO } // Since we disallow merging fields of different types, src and dstFieldDesc must have the same type name. - assert(srcFieldDesc.GetTypeName() == dstFieldDesc.GetTypeName()); - info.fInMemoryType = ColumnInMemoryType(srcFieldDesc.GetTypeName(), info.fColumnType); + assert(srcDesc.GetTypeNameForComparison(srcFieldDesc) == dstFieldDesc.GetTypeName()); + info.fInMemoryType = ColumnInMemoryType(dstFieldDesc.GetTypeName(), info.fColumnType); columns.emplace_back(info); } diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 8e97138d3b6a1..23bb0fba4a24c 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -1109,6 +1109,8 @@ ROOT::Internal::RPagePersistentSink::InitFromDescriptor(const ROOT::RNTupleDescr { // Create new descriptor fDescriptorBuilder.SetSchemaFromExisting(srcDescriptor); + // This is needed to be able to use GetTypeNameForComparison() + fDescriptorBuilder.SetVersionForWriting(); const auto &descriptor = fDescriptorBuilder.GetDescriptor(); // Create column/page ranges