Skip to content

Commit 606a7d8

Browse files
committed
[ntuple] fix automatic schema evolution of record fields
Ensure that record field evolution supports missing fields, reordered fields, and added fields. Mark added fields as artificial (which was missing before).
1 parent fb9be7d commit 606a7d8

File tree

2 files changed

+60
-1
lines changed

2 files changed

+60
-1
lines changed

tree/ntuple/src/RField.cxx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <iostream>
2929
#include <memory>
3030
#include <type_traits>
31+
#include <unordered_set>
3132

3233
std::unique_ptr<ROOT::RFieldBase> ROOT::RFieldZero::CloneImpl(std::string_view /*newName*/) const
3334
{
@@ -605,7 +606,20 @@ void ROOT::RRecordField::ReadInClusterImpl(RNTupleLocalIndex localIndex, void *t
605606

606607
void ROOT::RRecordField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
607608
{
608-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName | kDiffTypeVersion);
609+
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
610+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName | kDiffTypeVersion);
611+
612+
// The on-disk ID of subfields is matched by field name. So we inherently support reordering of fields
613+
// and we will ignore extra on-disk fields.
614+
// It remains to mark the extra in-memory fields as artificial.
615+
std::unordered_set<std::string> onDiskSubfields;
616+
for (const auto &subField : desc.GetFieldIterable(fieldDesc)) {
617+
onDiskSubfields.insert(subField.GetFieldName());
618+
}
619+
for (auto &f : fSubfields) {
620+
if (onDiskSubfields.count(f->GetFieldName()) == 0)
621+
CallSetArtificialOn(*f);
622+
}
609623
}
610624

611625
void ROOT::RRecordField::ConstructValue(void *where) const

tree/ntuple/test/ntuple_evolution_shape.cxx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,3 +966,48 @@ struct StreamerField {
966966
EXPECT_EVALUATE_EQ("ptrStreamerField->fInt", 2);
967967
EXPECT_EVALUATE_EQ("ptrStreamerField->fAdded", 137);
968968
}
969+
970+
TEST(RNTupleEvolution, RecordField)
971+
{
972+
FileRaii fileGuard("test_ntuple_evolution_record_field.root");
973+
{
974+
std::vector<std::unique_ptr<RFieldBase>> items;
975+
items.emplace_back(std::make_unique<RField<char>>("f1"));
976+
items.emplace_back(std::make_unique<RField<float>>("f2"));
977+
items.emplace_back(std::make_unique<RField<std::string>>("f3"));
978+
979+
auto model = ROOT::RNTupleModel::Create();
980+
model->AddField(std::make_unique<ROOT::RRecordField>("r", std::move(items)));
981+
982+
auto writer = ROOT::RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath());
983+
984+
auto r = writer->GetModel().GetDefaultEntry().GetPtr<void>("r");
985+
const auto &rf = static_cast<const ROOT::RRecordField &>(writer->GetModel().GetConstField("r"));
986+
987+
auto f1 = reinterpret_cast<char *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[0]);
988+
auto f3 = reinterpret_cast<std::string *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[2]);
989+
*f1 = 'x';
990+
*f3 = "ROOT";
991+
992+
writer->Fill();
993+
}
994+
995+
std::vector<std::unique_ptr<RFieldBase>> items;
996+
items.emplace_back(std::make_unique<RField<std::string>>("f3"));
997+
items.emplace_back(std::make_unique<RField<std::vector<float>>>("f4"));
998+
items.emplace_back(std::make_unique<RField<int>>("f1"));
999+
1000+
auto model = ROOT::RNTupleModel::Create();
1001+
model->AddField(std::make_unique<ROOT::RRecordField>("r", std::move(items)));
1002+
1003+
auto reader = RNTupleReader::Open(std::move(model), "ntpl", fileGuard.GetPath());
1004+
auto r = reader->GetModel().GetDefaultEntry().GetPtr<void>("r");
1005+
const auto &rf = static_cast<const ROOT::RRecordField &>(reader->GetModel().GetConstField("r"));
1006+
auto f3 = reinterpret_cast<std::string *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[0]);
1007+
auto f4 = reinterpret_cast<std::vector<float> *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[1]);
1008+
auto f1 = reinterpret_cast<int *>(reinterpret_cast<unsigned char *>(r.get()) + rf.GetOffsets()[2]);
1009+
reader->LoadEntry(0);
1010+
EXPECT_EQ("ROOT", *f3);
1011+
EXPECT_TRUE(f4->empty());
1012+
EXPECT_EQ('x', *f1);
1013+
}

0 commit comments

Comments
 (0)