From 728d56a24e55871610554619db734193d8c363ec Mon Sep 17 00:00:00 2001 From: Prakhar Singh Date: Sat, 30 May 2026 11:57:56 +0530 Subject: [PATCH] [ntuple] fix merging of pre-ROOT-6.36 NTuples Pre-ROOT-6.36 RNTuples (spec 1.0.0.0) stored field type names using ROOT Meta normalization (e.g. "Float_t" for float) instead of canonical C++ names. The merger rejected such files when merging with current-spec NTuples because it compared type names as raw strings. Reading back merged outputs derived from such files also failed in ReconcileFloatingPointField() for the same reason. Closes https://github.com/root-project/root/issues/22130 --- tree/ntuple/src/RField.cxx | 4 +- tree/ntuple/src/RNTupleMerger.cxx | 13 +++-- tree/ntuple/test/ntuple_merger.cxx | 89 ++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/tree/ntuple/src/RField.cxx b/tree/ntuple/src/RField.cxx index e994b3851b073..112465f3926e2 100644 --- a/tree/ntuple/src/RField.cxx +++ b/tree/ntuple/src/RField.cxx @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -158,7 +159,8 @@ void ROOT::RSimpleField::ReconcileFloatingPointField(const RNTupleDescriptor EnsureMatchingOnDiskField(desc, kDiffTypeName); const RFieldDescriptor &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId()); - if (!(fieldDesc.GetTypeName() == "float" || fieldDesc.GetTypeName() == "double")) { + const auto onDiskTypeName = ROOT::Internal::GetRenormalizedTypeName(fieldDesc.GetTypeName()); + if (!(onDiskTypeName == "float" || onDiskTypeName == "double")) { throw RException(R__FAIL("unexpected on-disk type name '" + fieldDesc.GetTypeName() + "' for field of type '" + GetTypeName() + "'\n" + Internal::GetTypeTraceReport(*this, desc))); } diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index 6ff1ad0ac8679..d8b2e70f254f0 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -530,8 +530,11 @@ 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 &dstTyName = field.fDst->GetTypeName(); + // Renormalize both sides to handle pre-ROOT-6.36 files that stored META-normalized type names + // (e.g. "Float_t") instead of canonical C++ names (e.g. "float"). Renormalizing a canonical + // name is a no-op, so this is safe for current-format files as well. + const auto srcTyName = ROOT::Internal::GetRenormalizedTypeName(field.fSrc->GetTypeName()); + const auto dstTyName = ROOT::Internal::GetRenormalizedTypeName(field.fDst->GetTypeName()); if (srcTyName != dstTyName) { std::stringstream ss; ss << "Field `" << fieldName @@ -1147,8 +1150,10 @@ 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(ROOT::Internal::GetRenormalizedTypeName(srcFieldDesc.GetTypeName()) == + ROOT::Internal::GetRenormalizedTypeName(dstFieldDesc.GetTypeName())); + info.fInMemoryType = + ColumnInMemoryType(ROOT::Internal::GetRenormalizedTypeName(srcFieldDesc.GetTypeName()), info.fColumnType); columns.emplace_back(info); } diff --git a/tree/ntuple/test/ntuple_merger.cxx b/tree/ntuple/test/ntuple_merger.cxx index de27ead34d3f9..40e681e6afe48 100644 --- a/tree/ntuple/test/ntuple_merger.cxx +++ b/tree/ntuple/test/ntuple_merger.cxx @@ -4025,3 +4025,92 @@ TEST(RNTupleMerger, MergeNewerVersion) } } } + +TEST(RNTupleMerger, MergePreV634TypeName) +{ + // Regression test for https://github.com/root-project/root/issues/22130: + // merging a pre-ROOT-6.36 RNTuple (spec 1.0.0.0, META-normalized type names) must succeed. + + // Simulate a pre-ROOT-6.36 RNTuple: spec version 1.0.0.0, field type stored as "Float_t" + // (what ROOT Meta produced for float fields before type-name renormalization was introduced). + FileRaii fileOld("test_ntuple_merge_pre634_old.root"); + { + RNTupleDescriptorBuilder descBuilder; + descBuilder.SetVersion(1, 0, 0, 0); + descBuilder.SetNTuple("ntuple", ""); + + descBuilder.AddField( + RFieldDescriptorBuilder::FromField(ROOT::RFieldZero()).FieldId(0).MakeDescriptor().Unwrap()); + descBuilder.AddField(RFieldDescriptorBuilder() + .FieldId(1) + .FieldName("x") + .TypeName("Float_t") + .Structure(ROOT::ENTupleStructure::kPlain) + .MakeDescriptor() + .Unwrap()); + descBuilder.AddFieldLink(0, 1).ThrowOnError(); + descBuilder.AddColumn(RColumnDescriptorBuilder() + .LogicalColumnId(0) + .PhysicalColumnId(0) + .FieldId(1) + .BitsOnStorage(32) + .Type(ROOT::ENTupleColumnType::kSplitReal32) + .Index(0) + .MakeDescriptor() + .Unwrap()); + + RNTupleWriteOptions options; + auto writer = RNTupleFileWriter::Recreate("ntuple", fileOld.GetPath(), EContainerFormat::kTFile, options); + RNTupleSerializer serializer; + + auto ctx = serializer.SerializeHeader(nullptr, descBuilder.GetDescriptor()).Unwrap(); + auto buffer = std::make_unique(ctx.GetHeaderSize()); + ctx = serializer.SerializeHeader(buffer.get(), descBuilder.GetDescriptor()).Unwrap(); + writer->WriteNTupleHeader(buffer.get(), ctx.GetHeaderSize(), ctx.GetHeaderSize()); + + auto szFooter = serializer.SerializeFooter(nullptr, descBuilder.GetDescriptor(), ctx).Unwrap(); + buffer = std::make_unique(szFooter); + serializer.SerializeFooter(buffer.get(), descBuilder.GetDescriptor(), ctx); + writer->WriteNTupleFooter(buffer.get(), szFooter, szFooter); + + writer->Commit(); + writer = nullptr; + } + + // Current-spec source with the same field, type name stored as "float". + FileRaii fileNew("test_ntuple_merge_pre634_new.root"); + { + auto model = RNTupleModel::Create(); + auto fieldX = model->MakeField("x"); + auto ntuple = RNTupleWriter::Recreate(std::move(model), "ntuple", fileNew.GetPath()); + *fieldX = 3.14f; + ntuple->Fill(); + } + + // Merge must succeed: "Float_t" and "float" are the same type after renormalization. + FileRaii fileOut("test_ntuple_merge_pre634_out.root"); + { + std::vector> sources; + sources.push_back(RPageSource::Create("ntuple", fileOld.GetPath(), RNTupleReadOptions())); + sources.push_back(RPageSource::Create("ntuple", fileNew.GetPath(), RNTupleReadOptions())); + std::vector sourcePtrs; + for (const auto &s : sources) { + sourcePtrs.push_back(s.get()); + } + + auto destination = std::make_unique("ntuple", fileOut.GetPath(), RNTupleWriteOptions()); + RNTupleMerger merger{std::move(destination)}; + RNTupleMergeOptions opts; + opts.fMergingMode = ENTupleMergingMode::kStrict; + auto res = merger.Merge(sourcePtrs, opts); + EXPECT_TRUE(bool(res)) << res.GetError()->GetReport(); + } + + // fileOld has 0 entries, fileNew has 1 — merged output must have exactly 1. + { + auto reader = RNTupleReader::Open("ntuple", fileOut.GetPath()); + EXPECT_EQ(1u, reader->GetNEntries()); + auto viewX = reader->GetView("x"); + EXPECT_FLOAT_EQ(3.14f, viewX(0)); + } +}