Skip to content

[ntuple] finalize automatic schema evolution between collections #20007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[ntuple] finalize automatic schema evolution between collections #20007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2554f8d
[ntuple] enforce correct subfield name in untyped collection
jblomer Sep 29, 2025
2231faf
[ntuple] fix set field reconciliation
jblomer Sep 29, 2025
3d9bf61
[ntuple] fix map field reconciliation
jblomer Sep 29, 2025
f420cd5
[ntuple] test automatic schema evolution of collections
jblomer Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tree/dataframe/test/datasource_ntuple.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ TEST_F(RNTupleDSTest, AlternativeColumnTypes)

auto multipleAlternativeTypes =
df.Define("nJets", [](const std::vector<float> &jets) { return jets.size(); }, {"jets"})
.Define("smallestJet", [](const std::set<float> &jets) { return *(jets.begin()); }, {"jets"})
.Define("smallestJet", [](const std::multiset<float> &jets) { return *(jets.begin()); }, {"jets"})
.Min<float>("smallestJet")
.GetValue();
EXPECT_FLOAT_EQ(1.f, multipleAlternativeTypes);
Expand Down
22 changes: 20 additions & 2 deletions tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ protected:
/// (Attach() and setting fItemSize)
RProxiedCollectionField(std::string_view fieldName, TClass *classp);

std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const final;
std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const override;
const RColumnRepresentations &GetColumnRepresentations() const final;
void GenerateColumns() final;
void GenerateColumns(const ROOT::RNTupleDescriptor &desc) final;
Expand All @@ -165,7 +165,7 @@ protected:
std::size_t AppendImpl(const void *from) final;
void ReadGlobalImpl(ROOT::NTupleSize_t globalIndex, void *to) final;

void ReconcileOnDiskField(const RNTupleDescriptor &desc) final;
void ReconcileOnDiskField(const RNTupleDescriptor &desc) override;

void CommitClusterImpl() final { fNWritten = 0; }

Expand Down Expand Up @@ -277,6 +277,15 @@ public:
kMultiMap,
kUnorderedMultiMap
};

private:
EMapType fMapType;

protected:
std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const final;
void ReconcileOnDiskField(const RNTupleDescriptor &desc) final;

public:
RMapField(std::string_view fieldName, EMapType mapType, std::unique_ptr<RFieldBase> itemField);
RMapField(RMapField &&other) = default;
RMapField &operator=(RMapField &&other) = default;
Expand Down Expand Up @@ -364,6 +373,15 @@ public:
kMultiSet,
kUnorderedMultiSet
};

private:
ESetType fSetType;

protected:
std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const final;
void ReconcileOnDiskField(const RNTupleDescriptor &desc) final;

public:
RSetField(std::string_view fieldName, ESetType setType, std::unique_ptr<RFieldBase> itemField);
RSetField(RSetField &&other) = default;
RSetField &operator=(RSetField &&other) = default;
Expand Down
47 changes: 45 additions & 2 deletions tree/ntuple/src/RFieldMeta.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,23 +871,66 @@ void ROOT::RProxiedCollectionField::AcceptVisitor(ROOT::Detail::RFieldVisitor &v
//------------------------------------------------------------------------------

ROOT::RMapField::RMapField(std::string_view fieldName, EMapType mapType, std::unique_ptr<RFieldBase> itemField)
: RProxiedCollectionField(fieldName, EnsureValidClass(BuildMapTypeName(mapType, itemField.get())))
: RProxiedCollectionField(fieldName, EnsureValidClass(BuildMapTypeName(mapType, itemField.get()))), fMapType(mapType)
{
auto *itemClass = fProxy->GetValueClass();
fItemSize = itemClass->GetClassSize();

Attach(std::move(itemField));
}

std::unique_ptr<ROOT::RFieldBase> ROOT::RMapField::CloneImpl(std::string_view newName) const
{
return std::make_unique<RMapField>(newName, fMapType, fSubfields[0]->Clone(fSubfields[0]->GetFieldName()));
}

void ROOT::RMapField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
{
static const std::vector<std::string> prefixesRegular = {"std::map<", "std::unordered_map<", "std::set<"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly here: std::unordered_set?


const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName).ThrowOnError();

switch (fMapType) {
case EMapType::kMap:
case EMapType::kUnorderedMap: EnsureMatchingTypePrefix(fieldDesc, prefixesRegular).ThrowOnError(); break;
default:
break;
// no restrictions for multimaps
}
}

//------------------------------------------------------------------------------

ROOT::RSetField::RSetField(std::string_view fieldName, ESetType setType, std::unique_ptr<RFieldBase> itemField)
: ROOT::RProxiedCollectionField(fieldName, EnsureValidClass(BuildSetTypeName(setType, *itemField)))
: ROOT::RProxiedCollectionField(fieldName, EnsureValidClass(BuildSetTypeName(setType, *itemField))),
fSetType(setType)
{
fItemSize = itemField->GetValueSize();
Attach(std::move(itemField));
}

std::unique_ptr<ROOT::RFieldBase> ROOT::RSetField::CloneImpl(std::string_view newName) const
{
return std::make_unique<RSetField>(newName, fSetType, fSubfields[0]->Clone(fSubfields[0]->GetFieldName()));
}

void ROOT::RSetField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
{
static const std::vector<std::string> prefixesRegular = {"std::set<", "std::unordered_set<", "std::map<"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we discussed this, but I forgot the reasoning again, sorry: Why can we read into a kUnorderedSet from a std::map, but not from a std::unordered_map?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think from the point of view of I/O we could. But to make it happen, we would need an unordered_set<pair>, which is missing a default hash function...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from a std::map, but not from a std::unordered_map
we would need an unordered_set<pair>, which is missing a default hash function...

Why is this an issue for a std::unordered_map?

And is it a fundamental issue or just an issue for the test? I.e. if a user had written a map and wanted to read it into a unordered_set they would have have provided the necessary hash overload, isn't it?


const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName).ThrowOnError();

switch (fSetType) {
case ESetType::kSet:
case ESetType::kUnorderedSet: EnsureMatchingTypePrefix(fieldDesc, prefixesRegular).ThrowOnError(); break;
default:
break;
// no restrictions for multisets
}
}

//------------------------------------------------------------------------------

namespace {
Expand Down
2 changes: 1 addition & 1 deletion tree/ntuple/src/RFieldSequenceContainer.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ ROOT::RVectorField::RVectorField(std::string_view fieldName, std::unique_ptr<RFi
std::unique_ptr<ROOT::RVectorField>
ROOT::RVectorField::CreateUntyped(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField)
{
return std::unique_ptr<ROOT::RVectorField>(new RVectorField(fieldName, std::move(itemField), ""));
return std::unique_ptr<ROOT::RVectorField>(new RVectorField(fieldName, itemField->Clone("_0"), ""));
}

std::unique_ptr<ROOT::RFieldBase> ROOT::RVectorField::CloneImpl(std::string_view newName) const
Expand Down
5 changes: 5 additions & 0 deletions tree/ntuple/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ ROOT_GENERATE_DICTIONARY(RNTupleDescriptorDict ${CMAKE_CURRENT_SOURCE_DIR}/RNTup

ROOT_ADD_GTEST(ntuple_endian ntuple_endian.cxx LIBRARIES ROOTNTuple)
ROOT_ADD_GTEST(ntuple_evolution_type ntuple_evolution_type.cxx LIBRARIES ROOTNTuple)
ROOT_GENERATE_DICTIONARY(STLContainerEvolutionDict ${CMAKE_CURRENT_SOURCE_DIR}/STLContainerEvolution.hxx
MODULE ntuple_evolution_type
LINKDEF STLContainerEvolutionLinkDef.h
OPTIONS -inlineInputHeader
DEPENDENCIES CustomStruct)
if(NOT MSVC)
# These unit tests rely on fork(), which is not available on Windows.
ROOT_ADD_GTEST(ntuple_evolution_shape ntuple_evolution_shape.cxx LIBRARIES ROOTNTuple)
Expand Down
17 changes: 17 additions & 0 deletions tree/ntuple/test/STLContainerEvolution.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#ifndef ROOT_RNTuple_Test_STLContainerEvolution
#define ROOT_RNTuple_Test_STLContainerEvolution

#include <map>
#include <set>
#include <unordered_set>
#include <unordered_map>
#include <utility>
#include <vector>

template <typename T>
struct CollectionProxy {
using ValueType = T;
std::vector<T> v; //!
};

#endif
36 changes: 36 additions & 0 deletions tree/ntuple/test/STLContainerEvolutionLinkDef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#ifdef __CLING__

#pragma link C++ class std::set<int>+;
#pragma link C++ class std::set<short int>+;
#pragma link C++ class std::set<std::pair<int, int>>+;
#pragma link C++ class std::set<std::pair<short int, short int>>+;

#pragma link C++ class std::unordered_set<int>+;
#pragma link C++ class std::unordered_set<short int>+;

#pragma link C++ class std::multiset<int>+;
#pragma link C++ class std::multiset<short int>+;
#pragma link C++ class std::multiset<std::pair<int, int>>+;
#pragma link C++ class std::multiset<std::pair<short int, short int>>+;

#pragma link C++ class std::unordered_multiset<int>+;
#pragma link C++ class std::unordered_multiset<short int>+;

#pragma link C++ class std::map<int, int>+;
#pragma link C++ class std::map<short int, short int>+;

#pragma link C++ class std::unordered_map<int, int>+;
#pragma link C++ class std::unordered_map<short int, short int>+;

#pragma link C++ class std::multimap<int, int>+;
#pragma link C++ class std::multimap<short int, short int>+;

#pragma link C++ class std::unordered_multimap<int, int>+;
#pragma link C++ class std::unordered_multimap<short int, short int>+;

#pragma link C++ class CollectionProxy<int>+;
#pragma link C++ class CollectionProxy<short int>+;
#pragma link C++ class CollectionProxy<std::pair<int, int>>+;
#pragma link C++ class CollectionProxy<std::pair<short int, short int>>+;

#endif
Loading
Loading