[ntuple] fix auto schema evolution of base classes

tree/ntuple/src/RFieldMeta.cxx

@@ -446,6 +446,9 @@ std::unique_ptr<ROOT::RFieldBase> 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<std::string> 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::RFieldBase> 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::RFieldBase> 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;

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<void>("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.