diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index e216f0c0d80fb..cf2892972af0c 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -446,6 +446,9 @@ std::unique_ptr ROOT::RClassField::BeforeConnectPageSource(ROO // On-disk members that are not targeted by an I/O rule; all other sub fields of the in-memory class // will be marked as artificial (added member in a new class version or member set by rule). std::unordered_set regularSubfields; + // We generally don't support changing the number of base classes, with the exception of changing from/to zero + // base classes. The variable stores the number of on-disk base classes. + int nOnDiskBaseClasses = 0; if (GetOnDiskId() == kInvalidDescriptorId) { // This can happen for added base classes or added members of class type @@ -468,6 +471,8 @@ std::unique_ptr ROOT::RClassField::BeforeConnectPageSource(ROO for (auto linkId : fieldDesc.GetLinkIds()) { const auto &subFieldDesc = desc.GetFieldDescriptor(linkId); regularSubfields.insert(subFieldDesc.GetFieldName()); + if (!subFieldDesc.GetFieldName().empty() && subFieldDesc.GetFieldName()[0] == ':') + nOnDiskBaseClasses++; } rules = FindRules(&fieldDesc); @@ -510,10 +515,20 @@ std::unique_ptr ROOT::RClassField::BeforeConnectPageSource(ROO } // Iterate over all sub fields in memory and mark those as missing that are not in the descriptor. + int nInMemoryBaseClasses = 0; for (auto &field : fSubfields) { - if (regularSubfields.count(field->GetFieldName()) == 0) { + const auto &fieldName = field->GetFieldName(); + if (regularSubfields.count(fieldName) == 0) { CallSetArtificialOn(*field); } + if (!fieldName.empty() && fieldName[0] == ':') + nInMemoryBaseClasses++; + } + + if (nInMemoryBaseClasses && nOnDiskBaseClasses && nInMemoryBaseClasses != nOnDiskBaseClasses) { + throw RException(R__FAIL(std::string("incompatible number of base classes for field ") + GetFieldName() + ": " + + GetTypeName() + ", " + std::to_string(nInMemoryBaseClasses) + " vs. " + + std::to_string(nOnDiskBaseClasses))); } return nullptr; diff --git a/tree/ntuple/test/ntuple_evolution_shape.cxx b/tree/ntuple/test/ntuple_evolution_shape.cxx index 0c19b02f22a22..1c24963f8c39d 100644 --- a/tree/ntuple/test/ntuple_evolution_shape.cxx +++ b/tree/ntuple/test/ntuple_evolution_shape.cxx @@ -593,20 +593,12 @@ struct AddedSecondBaseDerived : public AddedSecondBaseFirst, public AddedSecondB )")); auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); - ASSERT_EQ(2, reader->GetNEntries()); - - void *ptr = reader->GetModel().GetDefaultEntry().GetPtr("f").get(); - DeclarePointer("AddedSecondBaseDerived", "ptrAddedSecondBaseDerived", ptr); - - reader->LoadEntry(0); - EXPECT_EVALUATE_EQ("ptrAddedSecondBaseDerived->fFirstBase", 1); - EXPECT_EVALUATE_EQ("ptrAddedSecondBaseDerived->fSecondBase", 2); - EXPECT_EVALUATE_EQ("ptrAddedSecondBaseDerived->fDerived", 3); - - reader->LoadEntry(1); - EXPECT_EVALUATE_EQ("ptrAddedSecondBaseDerived->fFirstBase", 71); - EXPECT_EVALUATE_EQ("ptrAddedSecondBaseDerived->fSecondBase", 2); - EXPECT_EVALUATE_EQ("ptrAddedSecondBaseDerived->fDerived", 93); + try { + reader->GetModel(); + FAIL() << "model reconstruction should fail"; + } catch (const ROOT::RException &err) { + EXPECT_THAT(err.what(), testing::HasSubstr("incompatible number of base classes")); + } } TEST(RNTupleEvolution, PrependSecondBaseClass) @@ -662,7 +654,7 @@ struct PrependSecondBaseDerived : public PrependSecondBaseSecond, public Prepend reader->GetModel(); FAIL() << "model reconstruction should fail"; } catch (const ROOT::RException &err) { - EXPECT_THAT(err.what(), testing::HasSubstr("incompatible type name for field")); + EXPECT_THAT(err.what(), testing::HasSubstr("incompatible number of base classes")); } } @@ -844,6 +836,61 @@ struct RemovedIntermediateDerived : public RemovedIntermediateBase { } } +TEST(RNTupleEvolution, RemovedSecondBaseClass) +{ + FileRaii fileGuard("test_ntuple_evolution_removed_second_base_class.root"); + + ExecInFork([&] { + // The child process writes the file and exits, but the file must be preserved to be read by the parent. + fileGuard.PreserveFile(); + + ASSERT_TRUE(gInterpreter->Declare(R"( +struct RemovedSecondBaseFirst { + int fFirstBase = 1; +}; +struct RemovedSecondBaseSecond { + int fSecondBase = 2; +}; +struct RemovedSecondBaseDerived : public RemovedSecondBaseFirst, public RemovedSecondBaseSecond { + int fDerived = 3; +}; +)")); + + auto model = RNTupleModel::Create(); + model->AddField(RFieldBase::Create("f", "RemovedSecondBaseDerived").Unwrap()); + + auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); + writer->Fill(); + + // Reset / close the writer and flush the file. + { + // TStreamerInfo::Build will report a warning for interpreted classes (but only for base classes). + // See also https://github.com/root-project/root/issues/9371 + ROOT::TestSupport::CheckDiagsRAII diagRAII; + diagRAII.optionalDiag(kWarning, "TStreamerInfo::Build", "has no streamer or dictionary", + /*matchFullMessage=*/false); + writer.reset(); + } + }); + + ASSERT_TRUE(gInterpreter->Declare(R"( +struct RemovedSecondBaseFirst { + int fFirstBase = 1; +}; +struct RemovedSecondBaseDerived : public RemovedSecondBaseFirst { + int fDerived = 3; +}; +)")); + + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + try { + reader->GetModel(); + FAIL() << "model reconstruction should fail"; + } catch (const ROOT::RException &err) { + EXPECT_THAT(err.what(), testing::HasSubstr("incompatible number of base classes")); + } +} + TEST(RNTupleEvolution, RenamedBaseClass) { // RNTuple currently does not support automatic schema evolution when a class is renamed.