diff --git a/tree/ntuple/doc/BinaryFormatSpecification.md b/tree/ntuple/doc/BinaryFormatSpecification.md index 967f0580be0c1..2a87e514a8752 100644 --- a/tree/ntuple/doc/BinaryFormatSpecification.md +++ b/tree/ntuple/doc/BinaryFormatSpecification.md @@ -1102,6 +1102,7 @@ The on-disk representation is identical to a `std::vector`, using two fields: - Child field of type `T` with name `_0`. The field's type name is the type of the SoA class. +The type checksum and the type version of the SoA class is stored as field information. The SoA flag of the field descriptor must be set. ### ROOT::RNTupleCardinality diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx index a0eee042b89c0..6c2755e633e79 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx @@ -98,12 +98,16 @@ public: std::vector SplitValue(const RValue &value) const final; size_t GetValueSize() const final; size_t GetAlignment() const final { return fMaxAlignment; } + std::uint32_t GetTypeVersion() const final; + std::uint32_t GetTypeChecksum() const final; /// For polymorphic classes (that declare or inherit at least one virtual method), return the expected dynamic type /// of any user object. If the class is not polymorphic, return nullptr. /// TODO(jblomer): use information in unique pointer field const std::type_info *GetPolymorphicTypeInfo() const; // TODO(jblomer) // void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final; + + TClass *GetSoAClass() const { return fSoAClass; } }; } // namespace Experimental diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index f82e83ea7061a..2761e391116ae 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -539,6 +539,10 @@ protected: /// Throws an exception if the fields don't match. /// Optionally, a set of bits can be provided that should be ignored in the comparison. RResult EnsureMatchingOnDiskField(const RNTupleDescriptor &desc, std::uint32_t ignoreBits = 0) const; + /// Convenience wrapper for the common case of calling EnsureMatchinOnDiskField() for collections. Collections + /// may differ in type name (most collections schema evolve into each other). An on-disk SoA collection may also + /// have any type version whereas all other collections need to have type version 0. + RResult EnsureMatchingOnDiskCollection(const RNTupleDescriptor &desc) const; /// Many fields accept a range of type prefixes for schema evolution, /// e.g. std::unique_ptr< and std::optional< for nullable fields RResult diff --git a/tree/ntuple/src/RField.cxx b/tree/ntuple/src/RField.cxx index 2fa31c5424e5a..dc41bd91023de 100644 --- a/tree/ntuple/src/RField.cxx +++ b/tree/ntuple/src/RField.cxx @@ -104,7 +104,12 @@ void ROOT::RCardinalityField::ReconcileOnDiskField(const RNTupleDescriptor &desc " expects an on-disk leaf field of the same type\n" + Internal::GetTypeTraceReport(*this, desc))); } - } else if (fieldDesc.GetStructure() != ENTupleStructure::kCollection) { + } else if (fieldDesc.GetStructure() == ENTupleStructure::kCollection) { + if (!fieldDesc.IsSoACollection() && fieldDesc.GetTypeVersion() != 0) { + throw RException(R__FAIL("invalid on-disk type version for RCardinalityField " + GetQualifiedFieldName() + + "\n" + Internal::GetTypeTraceReport(*this, desc))); + } + } else { throw RException(R__FAIL("invalid on-disk structural role for RCardinalityField " + GetQualifiedFieldName() + "\n" + Internal::GetTypeTraceReport(*this, desc))); } diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index 38b70e5ad60b5..0fd55c0183a67 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -1070,6 +1070,14 @@ ROOT::RFieldBase::EnsureMatchingOnDiskField(const RNTupleDescriptor &desc, std:: return R__FAIL(errMsg.str() + "\n" + Internal::GetTypeTraceReport(*this, desc)); } +ROOT::RResult ROOT::RFieldBase::EnsureMatchingOnDiskCollection(const RNTupleDescriptor &desc) const +{ + std::uint32_t ignoreBits = kDiffTypeName; + if (desc.GetFieldDescriptor(GetOnDiskId()).IsSoACollection()) + ignoreBits |= kDiffTypeVersion; + return EnsureMatchingOnDiskField(desc, ignoreBits); +} + ROOT::RResult ROOT::RFieldBase::EnsureMatchingTypePrefix(const RNTupleDescriptor &desc, const std::vector &prefixes) const { diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index c8e331f30a3f5..bf8d21c459b41 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -693,6 +693,12 @@ ROOT::Experimental::RSoAField::RSoAField(std::string_view fieldName, TClass *clS } catch (ROOT::RException &e) { throw RException(R__FAIL("invalid record type of SoA field " + GetTypeName() + " [" + e.what() + "]")); } + R__ASSERT(fSoAClass->GetClassVersion() >= 0); + if (static_cast(fSoAClass->GetClassVersion()) != fSubfields[0]->GetTypeVersion()) { + throw RException(R__FAIL(std::string("version mismatch between SoA type and underlying record type: ") + + std::to_string(fSoAClass->GetClassVersion()) + " vs. " + + std::to_string(fSubfields[0]->GetTypeVersion()))); + } fRecordMemberFields = fSubfields[0]->GetMutableSubfields(); std::unordered_map recordFieldNameToIdx; @@ -764,7 +770,7 @@ ROOT::Experimental::RSoAField::RSoAField(std::string_view fieldName, TClass *clS if (ROOT::Internal::NeedsMetaNameAsAlias(fSoAClass->GetName(), renormalizedAlias)) fTypeAlias = renormalizedAlias; - fTraits |= kTraitSoACollection; + fTraits |= kTraitSoACollection | kTraitTypeChecksum; } std::unique_ptr ROOT::Experimental::RSoAField::CloneImpl(std::string_view newName) const @@ -861,6 +867,16 @@ size_t ROOT::Experimental::RSoAField::GetValueSize() const return fSoAClass->GetClassSize(); } +std::uint32_t ROOT::Experimental::RSoAField::GetTypeVersion() const +{ + return fSoAClass->GetClassVersion(); +} + +std::uint32_t ROOT::Experimental::RSoAField::GetTypeChecksum() const +{ + return fSoAClass->GetCheckSum(); +} + const std::type_info *ROOT::Experimental::RSoAField::GetPolymorphicTypeInfo() const { // TODO(jblomer): factor out @@ -1140,7 +1156,7 @@ void ROOT::RProxiedCollectionField::GenerateColumns(const ROOT::RNTupleDescripto void ROOT::RProxiedCollectionField::ReconcileOnDiskField(const RNTupleDescriptor &desc) { - EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError(); + EnsureMatchingOnDiskCollection(desc).ThrowOnError(); } void ROOT::RProxiedCollectionField::ConstructValue(void *where) const @@ -1211,7 +1227,7 @@ void ROOT::RMapField::ReconcileOnDiskField(const RNTupleDescriptor &desc) { static const std::vector prefixesRegular = {"std::map<", "std::unordered_map<"}; - EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError(); + EnsureMatchingOnDiskCollection(desc).ThrowOnError(); switch (fMapType) { case EMapType::kMap: @@ -1247,7 +1263,7 @@ void ROOT::RSetField::ReconcileOnDiskField(const RNTupleDescriptor &desc) static const std::vector prefixesRegular = {"std::set<", "std::unordered_set<", "std::map<", "std::unordered_map<"}; - EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError(); + EnsureMatchingOnDiskCollection(desc).ThrowOnError(); switch (fSetType) { case ESetType::kSet: diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index 505b014911381..9c96ee2990d03 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -408,7 +408,7 @@ std::unique_ptr ROOT::RRVecField::BeforeConnectPageSource(Inte void ROOT::RRVecField::ReconcileOnDiskField(const RNTupleDescriptor &desc) { - EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError(); + EnsureMatchingOnDiskCollection(desc).ThrowOnError(); } void ROOT::RRVecField::ConstructValue(void *where) const @@ -618,7 +618,7 @@ std::unique_ptr ROOT::RVectorField::BeforeConnectPageSource(In void ROOT::RVectorField::ReconcileOnDiskField(const RNTupleDescriptor &desc) { - EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError(); + EnsureMatchingOnDiskCollection(desc).ThrowOnError(); } void ROOT::RVectorField::RVectorDeleter::operator()(void *objPtr, bool dtorOnly) @@ -761,7 +761,7 @@ void ROOT::RField>::ReconcileOnDiskField(const RNTupleDescript } fOnDiskNRepetitions = fieldDesc.GetNRepetitions(); } else { - EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError(); + EnsureMatchingOnDiskCollection(desc).ThrowOnError(); } } @@ -860,8 +860,7 @@ void ROOT::RArrayAsRVecField::ReadInClusterImpl(RNTupleLocalIndex localIndex, vo void ROOT::RArrayAsRVecField::ReconcileOnDiskField(const RNTupleDescriptor &desc) { - EnsureMatchingOnDiskField(desc, kDiffTypeName | kDiffTypeVersion | kDiffStructure | kDiffNRepetitions) - .ThrowOnError(); + EnsureMatchingOnDiskField(desc, kDiffTypeName | kDiffStructure | kDiffNRepetitions).ThrowOnError(); const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId()); if (fieldDesc.GetTypeName().rfind("std::array<", 0) != 0) { throw RException(R__FAIL("RArrayAsRVecField " + GetQualifiedFieldName() + " expects an on-disk array field\n" + @@ -961,7 +960,7 @@ void ROOT::RArrayAsVectorField::ReadInClusterImpl(ROOT::RNTupleLocalIndex localI void ROOT::RArrayAsVectorField::ReconcileOnDiskField(const RNTupleDescriptor &desc) { - EnsureMatchingOnDiskField(desc, kDiffTypeName | kDiffTypeVersion | kDiffStructure | kDiffNRepetitions); + EnsureMatchingOnDiskField(desc, kDiffTypeName | kDiffStructure | kDiffNRepetitions); const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId()); if (fieldDesc.GetTypeName().rfind("std::array<", 0) != 0) { diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index ecacdae49495f..4ce441c96fcd1 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -100,6 +100,8 @@ void ROOT::Internal::RPageSinkFile::UpdateSchema(const ROOT::Internal::RNTupleMo cl = classField->GetClass(); } else if (auto streamerField = dynamic_cast(field)) { cl = streamerField->GetClass(); + } else if (auto soaField = dynamic_cast(field)) { + cl = soaField->GetSoAClass(); } if (!cl) return; diff --git a/tree/ntuple/test/SoAField.hxx b/tree/ntuple/test/SoAField.hxx index d31e5f04241ba..717bc02805045 100644 --- a/tree/ntuple/test/SoAField.hxx +++ b/tree/ntuple/test/SoAField.hxx @@ -19,6 +19,10 @@ struct SoAUnknownRecord { ClassDefNV(SoAUnknownRecord, 2); }; +struct SoAVersionMismatch { + ClassDefNV(SoAVersionMismatch, 4); +}; + struct RecordBase { ClassDefNV(RecordBase, 2); }; diff --git a/tree/ntuple/test/SoAFieldLinkDef.h b/tree/ntuple/test/SoAFieldLinkDef.h index 79041223dbacc..8fa0e977c800f 100644 --- a/tree/ntuple/test/SoAFieldLinkDef.h +++ b/tree/ntuple/test/SoAFieldLinkDef.h @@ -4,6 +4,7 @@ #pragma link C++ options=rntupleSoARecord(Record) class SoA+; #pragma link C++ options=rntupleSoARecord(xyz) class SoAUnknownRecord+; +#pragma link C++ options=rntupleSoARecord(Record) class SoAVersionMismatch+; #pragma link C++ class RecordBase+; #pragma link C++ class RecordDerived+; diff --git a/tree/ntuple/test/ntuple_soa.cxx b/tree/ntuple/test/ntuple_soa.cxx index f32c17c6d4a31..26d9fbe8ac6be 100644 --- a/tree/ntuple/test/ntuple_soa.cxx +++ b/tree/ntuple/test/ntuple_soa.cxx @@ -11,6 +11,8 @@ #include "SoAFieldXML.h" #include +#include +#include #include #include @@ -54,11 +56,18 @@ TEST(RNTuple, SoACheck) try { auto f = std::make_unique("f", "SoAUnknownRecord"); - FAIL() << "creating SoA field with missing record typedef should fail"; + FAIL() << "creating SoA field with unknown underlying record type should fail"; } catch (const ROOT::RException &e) { EXPECT_THAT(e.what(), ::testing::HasSubstr("invalid record type of SoA field SoAUnknownRecord")); } + try { + auto f = std::make_unique("f", "SoAVersionMismatch"); + FAIL() << "creating SoA field with a class version different from the underlying record type's should fail"; + } catch (const ROOT::RException &e) { + EXPECT_THAT(e.what(), ::testing::HasSubstr("version mismatch between SoA type and underlying record type")); + } + try { auto f = std::make_unique("f", "SoAOnDerivedRecord"); FAIL() << "creating SoA field on derived record should fail"; @@ -136,12 +145,36 @@ TEST(RNTuple, SoADescriptor) auto reader = ROOT::RNTupleReader::Open("ntpl", fileGuard.GetPath()); const auto &desc = reader->GetDescriptor(); - EXPECT_TRUE(desc.GetFieldDescriptor(desc.FindFieldId("f1")).IsSoACollection()); - EXPECT_EQ(ROOT::ENTupleStructure::kCollection, desc.GetFieldDescriptor(desc.FindFieldId("f1")).GetStructure()); - EXPECT_TRUE(desc.GetFieldDescriptor(desc.FindFieldId("f2")).IsSoACollection()); - EXPECT_EQ(ROOT::ENTupleStructure::kCollection, desc.GetFieldDescriptor(desc.FindFieldId("f2")).GetStructure()); - EXPECT_FALSE(desc.GetFieldDescriptor(desc.FindFieldId("f3")).IsSoACollection()); - EXPECT_EQ(ROOT::ENTupleStructure::kCollection, desc.GetFieldDescriptor(desc.FindFieldId("f3")).GetStructure()); + const auto &f1Desc = desc.GetFieldDescriptor(desc.FindFieldId("f1")); + EXPECT_TRUE(f1Desc.IsSoACollection()); + EXPECT_EQ(ROOT::ENTupleStructure::kCollection, f1Desc.GetStructure()); + EXPECT_EQ(TClass::GetClass("SoA")->GetClassVersion(), f1Desc.GetTypeVersion()); + EXPECT_TRUE(f1Desc.GetTypeChecksum()); + EXPECT_EQ(TClass::GetClass("SoA")->GetCheckSum(), *f1Desc.GetTypeChecksum()); + const auto &f2Desc = desc.GetFieldDescriptor(desc.FindFieldId("f2")); + EXPECT_TRUE(f2Desc.IsSoACollection()); + EXPECT_EQ(TClass::GetClass("SoA")->GetClassVersion(), f2Desc.GetTypeVersion()); + EXPECT_EQ(ROOT::ENTupleStructure::kCollection, f2Desc.GetStructure()); + EXPECT_TRUE(f2Desc.GetTypeChecksum()); + EXPECT_EQ(TClass::GetClass("SoA")->GetCheckSum(), *f2Desc.GetTypeChecksum()); + const auto &f3Desc = desc.GetFieldDescriptor(desc.FindFieldId("f3")); + EXPECT_FALSE(f3Desc.IsSoACollection()); + EXPECT_EQ(0u, f3Desc.GetTypeVersion()); + EXPECT_EQ(ROOT::ENTupleStructure::kCollection, f3Desc.GetStructure()); + EXPECT_FALSE(f3Desc.GetTypeChecksum()); +} + +TEST(RNTuple, SoAStreamerInfo) +{ + ROOT::TestSupport::FileRaii fileGuard("test_ntuple_soa_streamer_info.root"); + + auto model = ROOT::RNTupleModel::Create(); + model->AddField(std::make_unique("f", "SoASimple")); + auto writer = ROOT::RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); + writer.reset(); + + auto file = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str())); + EXPECT_NE(nullptr, file->GetStreamerInfoList()->FindObject("SoASimple")); } TEST(RNTuple, SoAEmpty) @@ -203,4 +236,9 @@ TEST(RNTuple, SoASimple) EXPECT_FLOAT_EQ(4.0, v(2).at(0).fY); EXPECT_FLOAT_EQ(5.0, v(2).at(1).fX); EXPECT_FLOAT_EQ(6.0, v(2).at(1).fY); + + auto card = reader->GetView>("f"); + EXPECT_EQ(1u, card(0)); + EXPECT_EQ(0u, card(1)); + EXPECT_EQ(2u, card(2)); }