diff --git a/tree/dataframe/test/CMakeLists.txt b/tree/dataframe/test/CMakeLists.txt index 78fbb17ec3d61..cf55334c25b0a 100644 --- a/tree/dataframe/test/CMakeLists.txt +++ b/tree/dataframe/test/CMakeLists.txt @@ -69,7 +69,7 @@ endif() ROOT_ADD_GTEST(dataframe_display dataframe_display.cxx LIBRARIES ROOTDataFrame) ROOT_ADD_GTEST(dataframe_ranges dataframe_ranges.cxx LIBRARIES ROOTDataFrame) -ROOT_ADD_GTEST(dataframe_leaves dataframe_leaves.cxx LIBRARIES ROOTDataFrame) +ROOT_ADD_GTEST(dataframe_leaves dataframe_leaves.cxx LIBRARIES ROOTDataFrame Physics) ROOT_ADD_GTEST(dataframe_resptr dataframe_resptr.cxx LIBRARIES ROOTDataFrame) ROOT_ADD_GTEST(dataframe_take dataframe_take.cxx LIBRARIES ROOTDataFrame) ROOT_ADD_GTEST(dataframe_entrylist dataframe_entrylist.cxx LIBRARIES ROOTDataFrame) diff --git a/tree/dataframe/test/dataframe_leaves.cxx b/tree/dataframe/test/dataframe_leaves.cxx index 1e7318c3a4964..b363b01e0580e 100644 --- a/tree/dataframe/test/dataframe_leaves.cxx +++ b/tree/dataframe/test/dataframe_leaves.cxx @@ -3,9 +3,23 @@ #include "TTree.h" #include "gtest/gtest.h" +#include +#include +#include +#include +#include using namespace ROOT::VecOps; +template +void expect_vec_float_eq(const T0 &v1, const T1 &v2) +{ + ASSERT_EQ(v1.size(), v2.size()) << "Vectors 'v1' and 'v2' are of unequal length"; + for (std::size_t i = 0ull; i < v1.size(); ++i) { + EXPECT_FLOAT_EQ(v1[i], v2[i]) << "Vectors 'v1' and 'v2' differ at index " << i; + } +} + struct BrVal { Float_t a; Int_t i; @@ -72,3 +86,44 @@ TEST(RDFLeaves, OneLeafWithDotNameClass) EXPECT_EQ(*res, 40); } +TEST(RDFLeaves, LeafFromTClonesArray) +{ + // Regression test for https://github.com/root-project/root/issues/19104 + ROOT::TestSupport::CheckDiagsRAII diagRAII; + diagRAII.requiredDiag(kWarning, "TTree::Bronch", + "Using split mode on a class: TLorentzVector with a custom Streamer"); +#ifndef NDEBUG + diagRAII.requiredDiag( + kWarning, "RTreeColumnReader::Get", + "Branch ca.fE hangs from a non-split branch. A copy is being performed in order to properly read the content."); +#endif + constexpr static auto fName{"leaffromtclonesarray.root"}; + struct Dataset { + Dataset() + { + auto f = std::make_unique(fName, "recreate"); + auto t = std::make_unique("t", "t"); + auto ca = std::make_unique("TLorentzVector"); + auto branchData = ca.get(); + auto &caRef = *ca; + t->Branch("ca", &branchData); + // Fill the array with two elements. The issue arose from the fact + // we were not computing the correct size of the correction value + // type, so the wrong offset was used when reading back next elements + // of the array + new (caRef[0]) TLorentzVector(0, 0, 0, 42.42); + new (caRef[1]) TLorentzVector(0, 0, 0, 84.84); + t->Fill(); + f->Write(); + } + + ~Dataset() { std::remove(fName); } + } _; + + std::vector expected{42.42, 84.84}; + + ROOT::RDataFrame df("t", fName); + auto resultptr = df.Take("ca.fE"); + ASSERT_EQ(resultptr->size(), 1); + expect_vec_float_eq(expected, resultptr->at(0)); +} diff --git a/tree/treeplayer/inc/TTreeReaderUtils.h b/tree/treeplayer/inc/TTreeReaderUtils.h index 48a4f124437a4..61945a00a8c51 100644 --- a/tree/treeplayer/inc/TTreeReaderUtils.h +++ b/tree/treeplayer/inc/TTreeReaderUtils.h @@ -86,6 +86,11 @@ namespace Internal { TVirtualCollectionReader() : fReadStatus(TTreeReaderValueBase::kReadNothingYet) {} virtual ~TVirtualCollectionReader(); + TVirtualCollectionReader(const TVirtualCollectionReader &) = delete; + TVirtualCollectionReader &operator=(const TVirtualCollectionReader &) = delete; + TVirtualCollectionReader(TVirtualCollectionReader &&) = delete; + TVirtualCollectionReader &operator=(TVirtualCollectionReader &&) = delete; + virtual size_t GetSize(Detail::TBranchProxy*) = 0; virtual void* At(Detail::TBranchProxy*, size_t /*idx*/) = 0; virtual bool IsContiguous(Detail::TBranchProxy *) = 0; diff --git a/tree/treeplayer/src/TBranchProxy.cxx b/tree/treeplayer/src/TBranchProxy.cxx index a44e9310ce855..068dd59af463e 100644 --- a/tree/treeplayer/src/TBranchProxy.cxx +++ b/tree/treeplayer/src/TBranchProxy.cxx @@ -433,6 +433,7 @@ bool ROOT::Detail::TBranchProxy::Setup() if (!fIsMember) fIsClone = true; fIsaPointer = false; fWhere = be->GetObject(); + fValueSize = static_cast(fWhere)->GetClass()->Size(); } else if (be->GetType()==4) { // top level TClonesArray diff --git a/tree/treeplayer/src/TTreeReaderArray.cxx b/tree/treeplayer/src/TTreeReaderArray.cxx index 2d9d8c5fdc951..10686b1c2b052 100644 --- a/tree/treeplayer/src/TTreeReaderArray.cxx +++ b/tree/treeplayer/src/TTreeReaderArray.cxx @@ -43,7 +43,13 @@ using namespace ROOT::Internal; // Reader interface for clones arrays class TClonesReader : public TVirtualCollectionReader { public: - ~TClonesReader() override {} + TClonesReader() = default; + ~TClonesReader() override = default; + TClonesReader(const TClonesReader &) = delete; + TClonesReader &operator=(const TClonesReader &) = delete; + TClonesReader(TClonesReader &&) = delete; + TClonesReader &operator=(TClonesReader &&) = delete; + TClonesArray *GetCA(ROOT::Detail::TBranchProxy *proxy) { if (!proxy->Read()) { @@ -76,8 +82,14 @@ class TClonesReader : public TVirtualCollectionReader { std::size_t GetValueSize(ROOT::Detail::TBranchProxy *proxy) override { - auto *ca = GetCA(proxy); - return ca ? ca->GetClass()->Size() : 0; + if (!proxy->Read()) { + fReadStatus = TTreeReaderValueBase::kReadError; + if (!proxy->GetSuppressErrorsForMissingBranch()) + Error("TClonesReader::GetValueSize()", "Read error in TBranchProxy."); + return 0; + } + fReadStatus = TTreeReaderValueBase::kReadSuccess; + return proxy->GetValueSize(); } }; @@ -444,6 +456,12 @@ class TBasicTypeClonesReader final : public TClonesReader { public: TBasicTypeClonesReader(Int_t offsetArg) : fOffset(offsetArg) {} + ~TBasicTypeClonesReader() final = default; + TBasicTypeClonesReader(const TBasicTypeClonesReader &) = delete; + TBasicTypeClonesReader &operator=(const TBasicTypeClonesReader &) = delete; + TBasicTypeClonesReader(TBasicTypeClonesReader &&) = delete; + TBasicTypeClonesReader &operator=(TBasicTypeClonesReader &&) = delete; + void *At(ROOT::Detail::TBranchProxy *proxy, size_t idx) override { TClonesArray *myClonesArray = GetCA(proxy);