Skip to content

Commit d3767a4

Browse files
jblomerjblomer
committed
[ntuple] automatic evolution from T --> unique_ptr/optional<T>
1 parent 73a06b6 commit d3767a4

File tree

3 files changed

+87
-14
lines changed

3 files changed

+87
-14
lines changed

‎tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ class RNullableField : public RFieldBase {
195195
ROOT::Internal::RColumnIndex fNWritten{0};
196196

197197
protected:
198+
// For reading, indicates that we read type T as a nullable field of type T, i.e. the value is always present
199+
bool fIsEvolvedFromInnerType = false;
200+
198201
const RFieldBase::RColumnRepresentations &GetColumnRepresentations() const final;
199202
void GenerateColumns() final;
200203
void GenerateColumns(const ROOT::RNTupleDescriptor &) final;

‎tree/ntuple/src/RField.cxx‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -849,18 +849,20 @@ const ROOT::RFieldBase::RColumnRepresentations &ROOT::RNullableField::GetColumnR
849849
{ENTupleColumnType::kIndex64},
850850
{ENTupleColumnType::kSplitIndex32},
851851
{ENTupleColumnType::kIndex32}},
852-
{});
852+
{{}});
853853
return representations;
854854
}
855855

856856
void ROOT::RNullableField::GenerateColumns()
857857
{
858+
R__ASSERT(!fIsEvolvedFromInnerType);
858859
GenerateColumnsImpl<ROOT::Internal::RColumnIndex>();
859860
}
860861

861862
void ROOT::RNullableField::GenerateColumns(const ROOT::RNTupleDescriptor &desc)
862863
{
863-
GenerateColumnsImpl<ROOT::Internal::RColumnIndex>(desc);
864+
if (!fIsEvolvedFromInnerType)
865+
GenerateColumnsImpl<ROOT::Internal::RColumnIndex>(desc);
864866
}
865867

866868
std::size_t ROOT::RNullableField::AppendNull()
@@ -881,8 +883,16 @@ void ROOT::RNullableField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
881883
{
882884
static const std::vector<std::string> prefixes = {"std::optional<", "std::unique_ptr<"};
883885

884-
EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError();
885-
EnsureMatchingTypePrefix(desc, prefixes).ThrowOnError();
886+
auto success = EnsureMatchingOnDiskField(desc, kDiffTypeName);
887+
if (!success) {
888+
fIsEvolvedFromInnerType = true;
889+
} else {
890+
success = EnsureMatchingTypePrefix(desc, prefixes);
891+
fIsEvolvedFromInnerType = !success;
892+
}
893+
894+
if (fIsEvolvedFromInnerType)
895+
fSubfields[0]->SetOnDiskId(GetOnDiskId());
886896
}
887897

888898
ROOT::RNTupleLocalIndex ROOT::RNullableField::GetItemIndex(ROOT::NTupleSize_t globalIndex)
@@ -951,16 +961,28 @@ void *ROOT::RUniquePtrField::PrepareRead(void *to, bool hasOnDiskValue)
951961

952962
void ROOT::RUniquePtrField::ReadGlobalImpl(ROOT::NTupleSize_t globalIndex, void *to)
953963
{
954-
auto itemIndex = GetItemIndex(globalIndex);
955-
const bool hasOnDiskValue = itemIndex.GetIndexInCluster() != ROOT::kInvalidNTupleIndex;
964+
RNTupleLocalIndex itemIndex;
965+
if (!fIsEvolvedFromInnerType)
966+
itemIndex = GetItemIndex(globalIndex);
967+
const bool hasOnDiskValue = fIsEvolvedFromInnerType || itemIndex.GetIndexInCluster() != ROOT::kInvalidNTupleIndex;
956968
auto valuePtr = PrepareRead(to, hasOnDiskValue);
957-
if (hasOnDiskValue)
958-
CallReadOn(*fSubfields[0], itemIndex, valuePtr);
969+
if (hasOnDiskValue) {
970+
if (fIsEvolvedFromInnerType) {
971+
CallReadOn(*fSubfields[0], globalIndex, valuePtr);
972+
} else {
973+
CallReadOn(*fSubfields[0], itemIndex, valuePtr);
974+
}
975+
}
959976
}
960977

961978
void ROOT::RUniquePtrField::ReadInClusterImpl(ROOT::RNTupleLocalIndex localIndex, void *to)
962979
{
963-
auto itemIndex = GetItemIndex(localIndex);
980+
RNTupleLocalIndex itemIndex;
981+
if (!fIsEvolvedFromInnerType) {
982+
itemIndex = GetItemIndex(localIndex);
983+
} else {
984+
itemIndex = localIndex;
985+
}
964986
const bool hasOnDiskValue = itemIndex.GetIndexInCluster() != ROOT::kInvalidNTupleIndex;
965987
auto valuePtr = PrepareRead(to, hasOnDiskValue);
966988
if (hasOnDiskValue)
@@ -1043,16 +1065,28 @@ void ROOT::ROptionalField::PrepareRead(void *to, bool hasOnDiskValue)
10431065

10441066
void ROOT::ROptionalField::ReadGlobalImpl(ROOT::NTupleSize_t globalIndex, void *to)
10451067
{
1046-
auto itemIndex = GetItemIndex(globalIndex);
1047-
const bool hasOnDiskValue = itemIndex.GetIndexInCluster() != ROOT::kInvalidNTupleIndex;
1068+
RNTupleLocalIndex itemIndex;
1069+
if (!fIsEvolvedFromInnerType)
1070+
itemIndex = GetItemIndex(globalIndex);
1071+
const bool hasOnDiskValue = fIsEvolvedFromInnerType || itemIndex.GetIndexInCluster() != ROOT::kInvalidNTupleIndex;
10481072
PrepareRead(to, hasOnDiskValue);
1049-
if (hasOnDiskValue)
1050-
CallReadOn(*fSubfields[0], itemIndex, to);
1073+
if (hasOnDiskValue) {
1074+
if (fIsEvolvedFromInnerType) {
1075+
CallReadOn(*fSubfields[0], globalIndex, to);
1076+
} else {
1077+
CallReadOn(*fSubfields[0], itemIndex, to);
1078+
}
1079+
}
10511080
}
10521081

10531082
void ROOT::ROptionalField::ReadInClusterImpl(ROOT::RNTupleLocalIndex localIndex, void *to)
10541083
{
1055-
auto itemIndex = GetItemIndex(localIndex);
1084+
RNTupleLocalIndex itemIndex;
1085+
if (!fIsEvolvedFromInnerType) {
1086+
itemIndex = GetItemIndex(localIndex);
1087+
} else {
1088+
itemIndex = localIndex;
1089+
}
10561090
const bool hasOnDiskValue = itemIndex.GetIndexInCluster() != ROOT::kInvalidNTupleIndex;
10571091
PrepareRead(to, hasOnDiskValue);
10581092
if (hasOnDiskValue)

‎tree/ntuple/test/ntuple_evolution_type.cxx‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,42 @@ TEST(RNTupleEvolution, ArrayAsVector)
283283
EXPECT_TRUE(aAsBool(0)[1]);
284284
}
285285

286+
TEST(RNTupleEvolution, CheckNullable)
287+
{
288+
FileRaii fileGuard("test_ntuple_evolution_check_nullable.root");
289+
{
290+
auto model = ROOT::RNTupleModel::Create();
291+
auto o = model->MakeField<std::optional<std::int32_t>>("o");
292+
auto u = model->MakeField<std::unique_ptr<std::int32_t>>("u");
293+
auto i = model->MakeField<std::int32_t>("i");
294+
auto writer = ROOT::RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath());
295+
296+
*o = 7;
297+
*u = std::make_unique<std::int32_t>(11);
298+
*i = 13;
299+
writer->Fill();
300+
}
301+
302+
auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath());
303+
304+
auto v1 = reader->GetView<std::unique_ptr<std::int64_t>>("o");
305+
auto v2 = reader->GetView<std::optional<std::int64_t>>("u");
306+
auto v3 = reader->GetView<std::unique_ptr<std::int64_t>>("i");
307+
auto v4 = reader->GetView<std::optional<std::int64_t>>("i");
308+
309+
try {
310+
reader->GetView<std::optional<std::string>>("i");
311+
FAIL() << "evolution of a nullable field with an invalid inner field should throw";
312+
} catch (const ROOT::RException &err) {
313+
EXPECT_THAT(err.what(), testing::HasSubstr("of type std::string is incompatible with on-disk field"));
314+
}
315+
316+
EXPECT_EQ(7, *v1(0));
317+
EXPECT_EQ(11, *v2(0));
318+
EXPECT_EQ(13, *v3(0));
319+
EXPECT_EQ(13, *v4(0));
320+
}
321+
286322
TEST(RNTupleEvolution, NullableToVector)
287323
{
288324
FileRaii fileGuard("test_ntuple_evolution_nullable_to_vector.root");

0 commit comments

Comments
 (0)