Skip to content

Commit

Permalink
[ntuple] add fwd-compatibility mode to CreateModel
Browse files Browse the repository at this point in the history
Allows to reconstruct a model even if some of the fields have unknown
structural role (those fields will become RInvalidFields)
  • Loading branch information
silverweed committed Oct 11, 2024
1 parent 551eb48 commit 5891e9b
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 5 deletions.
2 changes: 2 additions & 0 deletions tree/ntuple/v7/inc/ROOT/RField.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ public:
};

/// Used in RFieldBase::Check() to record field creation failures.
/// Also used when deserializing a field that contains unknown values that may come from
/// future RNTuple versions (e.g. an unknown Structure)
class RInvalidField final : public RFieldBase {
std::string fError;

Expand Down
2 changes: 1 addition & 1 deletion tree/ntuple/v7/inc/ROOT/RNTupleDescriptor.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ public:
/// with an RNTupleReader, but it is useful, e.g., to accurately merge data.
bool fReconstructProjections = false;
/// Normally creating a model will fail if any of the reconstructed fields contains an unknown column type.
/// If this option is enabled, the model will be created and all fields containing an unknown column (directly
/// If this option is enabled, the model will be created and all fields containing unknown data (directly
/// or indirectly) will be skipped instead.
bool fForwardCompatible = false;
};
Expand Down
1 change: 1 addition & 0 deletions tree/ntuple/v7/inc/ROOT/RNTupleUtil.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ auto MakeAliasedSharedPtr(T *rawPtr)

#if defined(__GNUC__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wpragmas"
#pragma GCC diagnostic ignored "-Wenum-constexpr-conversion"
#endif
inline constexpr ENTupleStructure kTestFutureFieldStructure =
Expand Down
10 changes: 7 additions & 3 deletions tree/ntuple/v7/src/RNTupleDescriptor.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,13 @@ ROOT::Experimental::RFieldDescriptor::CreateField(const RNTupleDescriptor &ntplD
// The structure may be unknown if the descriptor comes from a deserialized field with an unknown structural role.
// For forward compatibility, we allow this case and return an InvalidField.
if (GetStructure() == ENTupleStructure::kUnknown) {
auto invalidField = std::make_unique<RInvalidField>(GetFieldName(), GetTypeName(), "");
invalidField->SetOnDiskId(fFieldId);
return invalidField;
if (continueOnError) {
auto invalidField = std::make_unique<RInvalidField>(GetFieldName(), GetTypeName(), "");
invalidField->SetOnDiskId(fFieldId);
return invalidField;
} else {
throw RException(R__FAIL("unexpected on-disk field structure value for field \"" + GetFieldName() + "\""));
}
}

if (GetTypeName().empty()) {
Expand Down
54 changes: 53 additions & 1 deletion tree/ntuple/v7/test/ntuple_compat.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ class RFutureField : public RFieldBase {
TEST(RNTupleCompat, FutureFieldStructuralRole)
{
// Write a RNTuple containing a field with an unknown structural role and verify we can
// read back the ntuple and its descriptor.
// read back the ntuple, its descriptor and reconstruct the model.

FileRaii fileGuard("test_ntuple_compat_future_field_struct.root");
{
Expand All @@ -387,4 +387,56 @@ TEST(RNTupleCompat, FutureFieldStructuralRole)
const auto &desc = reader->GetDescriptor();
const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("future"));
EXPECT_EQ(fdesc.GetLogicalColumnIds().size(), 0);

// Attempting to create a model with default options should fail
EXPECT_THROW(desc.CreateModel(), RException);

auto modelOpts = RNTupleDescriptor::RCreateModelOptions();
modelOpts.fForwardCompatible = true;
auto model = desc.CreateModel(modelOpts);
try {
model->GetField("future");
FAIL() << "trying to get a field with unknown role should fail";
} catch (const RException &err) {
EXPECT_THAT(err.what(), testing::HasSubstr("invalid field"));
}
}

TEST(RNTupleCompat, FutureFieldStructuralRole_Nested)
{
// Write a RNTuple containing a field with an unknown structural role and verify we can
// read back the ntuple, its descriptor and reconstruct the model.

FileRaii fileGuard("test_ntuple_compat_future_field_struct_nested.root");
{
auto model = RNTupleModel::Create();
std::vector<std::unique_ptr<RFieldBase>> itemFields;
itemFields.emplace_back(new RField<int>("int"));
itemFields.emplace_back(new RFutureField("future"));
auto field = std::make_unique<ROOT::Experimental::RRecordField>("record", std::move(itemFields));
model->AddField(std::move(field));
model->MakeField<float>("float");
auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath());
writer->Fill();
}

auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath());
const auto &desc = reader->GetDescriptor();
const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("record"));
EXPECT_EQ(fdesc.GetLogicalColumnIds().size(), 0);

// Attempting to create a model with default options should fail
EXPECT_THROW(desc.CreateModel(), RException);

auto modelOpts = RNTupleDescriptor::RCreateModelOptions();
modelOpts.fForwardCompatible = true;
auto model = desc.CreateModel(modelOpts);
const auto &floatFld = model->GetField("float");
EXPECT_EQ(floatFld.GetTypeName(), "float");
try {
model->GetField("record");
FAIL() << "trying to get a field with unknown role should fail";
} catch (const RException &err) {
EXPECT_THAT(err.what(), testing::HasSubstr("invalid field"));
}
}

0 comments on commit 5891e9b

Please sign in to comment.