From 5733368741e47aa6034ac3e1a29923d6b06333da Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Tue, 28 Oct 2025 23:49:48 +0100 Subject: [PATCH 01/13] [tree] Forward friend tree update notification to all external friends When one of the friend TChain of the current TTree/TChain is switching to a different file, this triggers a notification via TFriendElement::MarkUpdated. In general, the TTree/TChain that sees there was an update in one of its friends cannot know a priori whether it is the "owner" of the relationship between the branches of the TTree being updated and the memory region where the values should be read into (e.g. the address of the object the user passed to `SetBranchAddress`). In order to ensure smooth update and arbitrarily long TTree/TChain friendship nested layers, this commit percolates the update notification up to the chain of [TTree,TChain]::LoadTree calls. --- tree/tree/src/TChain.cxx | 10 ++++++++++ tree/tree/src/TTree.cxx | 8 ++++++++ 2 files changed, 18 insertions(+) diff --git a/tree/tree/src/TChain.cxx b/tree/tree/src/TChain.cxx index 2ef501117ffe1..06e3901ba7d5e 100644 --- a/tree/tree/src/TChain.cxx +++ b/tree/tree/src/TChain.cxx @@ -1311,6 +1311,16 @@ Long64_t TChain::RefreshFriendAddresses() br->SetAutoDelete(true); } } + + // We cannot know a priori if the branch(es) of the friend TChain(s) that were just + // updated were supposed to be connected to one of the TChainElement of this chain + // or possibly to another TChainElement belonging to another chain that has befriended + // this chain (i.e., one of the "external friends"). Thus, we forward the notification + // that one or more friend trees were updated to the friends of this chain. + if (fExternalFriends) + for (auto external_fe : ROOT::Detail::TRangeStaticCast(*fExternalFriends)) + external_fe->MarkUpdated(); + if (fPlayer) { fPlayer->UpdateFormulaLeaves(); } diff --git a/tree/tree/src/TTree.cxx b/tree/tree/src/TTree.cxx index 3f100ace22eb5..0eafaf9f99664 100644 --- a/tree/tree/src/TTree.cxx +++ b/tree/tree/src/TTree.cxx @@ -6612,6 +6612,14 @@ Long64_t TTree::LoadTree(Long64_t entry) if (fNotify) { if(!fNotify->Notify()) return -6; } + // We cannot know a priori if the branch(es) of the friend TChain(s) that were just + // updated were supposed to be connected to possibly a TChainElement of another chain + // that has befriended this TTree (i.e., one of the "external friends"). Thus, we + // forward the notification that one or more friend trees were updated to the friends + // of this TTree. + if (fExternalFriends) + for (auto external_fe : ROOT::Detail::TRangeStaticCast(*fExternalFriends)) + external_fe->MarkUpdated(); } } From 78ea7fb096bfcb080e005af20ab30bcd5ce7722a Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Wed, 29 Oct 2025 15:21:07 +0100 Subject: [PATCH 02/13] [tree] Consider friendship hierarchies in GetFriendAlias Extend the logic of TTree::GetFriendAlias to not only consider immediate friends but also friends of the current tree (relevant if the caller is a TChain but the friendship is established by the current TTree in the chain). Also take care of looking for the friend alias recursively in both the immediate friends and the friends of the current tree. --- tree/tree/src/TTree.cxx | 72 +++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 21 deletions(-) diff --git a/tree/tree/src/TTree.cxx b/tree/tree/src/TTree.cxx index 0eafaf9f99664..35b4f9699d054 100644 --- a/tree/tree/src/TTree.cxx +++ b/tree/tree/src/TTree.cxx @@ -6111,7 +6111,7 @@ TTree* TTree::GetFriend(const char *friendname) const /// that you may not be able to take advantage of this feature. /// -const char* TTree::GetFriendAlias(TTree* tree) const +const char *TTree::GetFriendAlias(TTree *tree) const { if ((tree == this) || (tree == GetTree())) { return nullptr; @@ -6122,30 +6122,60 @@ const char* TTree::GetFriendAlias(TTree* tree) const if (kGetFriendAlias & fFriendLockStatus) { return nullptr; } - if (!fFriends) { + + // This is a TTree and it does not have any friends, we can return early + if (GetTree() == this && !fFriends) return nullptr; - } - TFriendLock lock(const_cast(this), kGetFriendAlias); - TIter nextf(fFriends); - TFriendElement* fe = nullptr; - while ((fe = (TFriendElement*) nextf())) { - TTree* t = fe->GetTree(); - if (t == tree) { - return fe->GetName(); + + TFriendLock lock(const_cast(this), kGetFriendAlias); + + auto lookForFriendNameInListOfFriends = [tree](const TList &friends) -> const char * { + for (auto *frEl : ROOT::Detail::TRangeStaticCast(friends)) { + auto *frElTree = frEl->GetTree(); + // Simplest case: we found a friend which tree is the same as the input tree + if (frElTree == tree) + return frEl->GetName(); + // Try again: the friend tree might be actually a TChain + if (frElTree && frElTree->GetTree() == tree) + return frEl->GetName(); } - // Case of a chain: - if (t && t->GetTree() == tree) { - return fe->GetName(); + return nullptr; + }; + + // First, look for the immediate friends of this tree + if (fFriends) { + const char *friendAlias = lookForFriendNameInListOfFriends(*fFriends); + if (friendAlias) + return friendAlias; + } + + // Then, check if this is a TChain and the current tree has friends + // The non-redundant scenario here is that the currently-available + // inner TTree of this TChain has a list of friends which the TChain + // itself doesn't know anything about. + if (const auto *innerListOfFriends = GetTree()->GetListOfFriends(); + innerListOfFriends && innerListOfFriends != fFriends) { + const char *friendAlias = lookForFriendNameInListOfFriends(*innerListOfFriends); + if (friendAlias) + return friendAlias; + } + + // Recursively look into the list of friends of this tree + if (fFriends) { + for (auto *frEl : ROOT::Detail::TRangeStaticCast(*fFriends)) { + const char *friendAlias = frEl->GetTree()->GetFriendAlias(tree); + if (friendAlias) + return friendAlias; } } - // After looking at the first level, - // let's see if it is a friend of friends. - nextf.Reset(); - fe = nullptr; - while ((fe = (TFriendElement*) nextf())) { - const char* res = fe->GetTree()->GetFriendAlias(tree); - if (res) { - return res; + + // Recursively look into the list of friends of the inner tree + if (const auto *innerListOfFriends = GetTree()->GetListOfFriends(); + innerListOfFriends && innerListOfFriends != fFriends) { + for (auto *frEl : ROOT::Detail::TRangeStaticCast(*innerListOfFriends)) { + const char *friendAlias = frEl->GetTree()->GetFriendAlias(tree); + if (friendAlias) + return friendAlias; } } return nullptr; From 67e33dc87dd1f2eb3be9dc2452de5d2a7dda4e56 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Fri, 31 Oct 2025 20:43:43 +0100 Subject: [PATCH 03/13] [tree] Fix integer overflow in TTree::Scan --- tree/treeplayer/src/TTreePlayer.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tree/treeplayer/src/TTreePlayer.cxx b/tree/treeplayer/src/TTreePlayer.cxx index 154fc3966be7d..1180d543bee2b 100644 --- a/tree/treeplayer/src/TTreePlayer.cxx +++ b/tree/treeplayer/src/TTreePlayer.cxx @@ -2711,9 +2711,7 @@ Long64_t TTreePlayer::Scan(const char *varexp, const char *selection, fSelectedRows = 0; Int_t tnumber = -1; bool exitloop = false; - for (entry=firstentry; - entry<(firstentry+nentries) && !exitloop; - entry++) { + for (entry = firstentry; entry - firstentry < nentries && !exitloop; entry++) { entryNumber = fTree->GetEntryNumber(entry); if (entryNumber < 0) break; Long64_t localEntry = fTree->LoadTree(entryNumber); From 087f68f756f167fb7553126887adb4446d6a339b Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Fri, 31 Oct 2025 20:44:57 +0100 Subject: [PATCH 04/13] [tree] Fix integer overflow in TTree::Draw --- tree/treeplayer/src/TTreePlayer.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/treeplayer/src/TTreePlayer.cxx b/tree/treeplayer/src/TTreePlayer.cxx index 1180d543bee2b..018be6645014c 100644 --- a/tree/treeplayer/src/TTreePlayer.cxx +++ b/tree/treeplayer/src/TTreePlayer.cxx @@ -2291,7 +2291,7 @@ Long64_t TTreePlayer::Process(TSelector *selector,Option_t *option, Long64_t nen fSelectorUpdate = selector; UpdateFormulaLeaves(); - for (entry=firstentry;entryGetEntryNumber(entry); if (entryNumber < 0) break; if (timer && timer->ProcessEvents()) break; From 55e27f664936d45a3ced59e030fd219d249a4077 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Thu, 30 Oct 2025 15:45:56 +0100 Subject: [PATCH 05/13] [tree] Allow branch lookup preceded by own TChain name TTree::FindBranch already took into account the case of the user wanting to find a branch in the tree by preceding the branch name with the tree name itself. The same logic was not applied symmetrically in TChain::FindBranch, so it's now introduced. The logic is written such that it only performs a copy of the lookup string after having checked that it starts with the name of the chain itself followed by a dot. --- tree/tree/src/TChain.cxx | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/tree/tree/src/TChain.cxx b/tree/tree/src/TChain.cxx index 06e3901ba7d5e..6eddb200bd12b 100644 --- a/tree/tree/src/TChain.cxx +++ b/tree/tree/src/TChain.cxx @@ -64,6 +64,8 @@ the trees in the chain. #include "strlcpy.h" #include "snprintf.h" +#include +#include "ROOT/StringUtils.hxx" //////////////////////////////////////////////////////////////////////////////// /// Default constructor. @@ -802,16 +804,35 @@ Long64_t TChain::Draw(const char* varexp, const char* selection, //////////////////////////////////////////////////////////////////////////////// /// See TTree::GetReadEntry(). -TBranch* TChain::FindBranch(const char* branchname) +TBranch *TChain::FindBranch(const char *branchname) { - if (fTree) { - return fTree->FindBranch(branchname); - } - LoadTree(0); - if (fTree) { - return fTree->FindBranch(branchname); + auto findBranchImpl = [this](const char *resolvedBranchName) -> TBranch * { + if (fTree) { + return fTree->FindBranch(resolvedBranchName); + } + LoadTree(0); + if (fTree) { + return fTree->FindBranch(resolvedBranchName); + } + return nullptr; + }; + + // This will allow the branchname to be preceded by the name of this chain. + // See similar code in TTree::FindBranch + std::string_view branchNameView{branchname}; + std::string_view chainPrefix = GetName(); + + if (ROOT::StartsWith(branchNameView, chainPrefix)) { + branchNameView.remove_prefix(chainPrefix.length()); + if (!branchNameView.empty() && branchNameView.front() == '.') { + branchNameView.remove_prefix(1); + // We're only removing characters from the beginning of the view so we + // don't need to worry about missing null-termination character + return findBranchImpl(branchNameView.data()); + } } - return nullptr; + + return findBranchImpl(branchname); } //////////////////////////////////////////////////////////////////////////////// From 87b1ed93defb2ebe0e9f9f65375267e6bfa319cb Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Sat, 1 Nov 2025 14:11:07 +0100 Subject: [PATCH 06/13] [tree] Allow leaf lookup preceded by own TChain name TTree::FindLeaf already took into account the case of the user wanting to find a leaf in the tree by preceding the branch name with the tree name itself. The same logic was not applied symmetrically in TChain::FindLeaf, so it's now introduced. The logic is written such that it only performs a copy of the lookup string after having checked that it starts with the name of the chain itself followed by a dot. --- tree/tree/src/TChain.cxx | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/tree/tree/src/TChain.cxx b/tree/tree/src/TChain.cxx index 6eddb200bd12b..7cd168e87b55b 100644 --- a/tree/tree/src/TChain.cxx +++ b/tree/tree/src/TChain.cxx @@ -840,14 +840,33 @@ TBranch *TChain::FindBranch(const char *branchname) TLeaf* TChain::FindLeaf(const char* searchname) { - if (fTree) { - return fTree->FindLeaf(searchname); - } - LoadTree(0); - if (fTree) { - return fTree->FindLeaf(searchname); + auto findLeafImpl = [this](const char *resolvedBranchName) -> TLeaf * { + if (fTree) { + return fTree->FindLeaf(resolvedBranchName); + } + LoadTree(0); + if (fTree) { + return fTree->FindLeaf(resolvedBranchName); + } + return nullptr; + }; + + // This will allow the branchname to be preceded by the name of this chain. + // See similar code in TTree::FindLeaf + std::string_view branchNameView{searchname}; + std::string_view chainPrefix = GetName(); + + if (ROOT::StartsWith(branchNameView, chainPrefix)) { + branchNameView.remove_prefix(chainPrefix.length()); + if (!branchNameView.empty() && branchNameView.front() == '.') { + branchNameView.remove_prefix(1); + // We're only removing characters from the beginning of the view so we + // don't need to worry about missing null-termination character + return findLeafImpl(branchNameView.data()); + } } - return nullptr; + + return findLeafImpl(searchname); } //////////////////////////////////////////////////////////////////////////////// From 9966e42ec551d2799a91171a924002b90fe59d8b Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Fri, 31 Oct 2025 20:44:19 +0100 Subject: [PATCH 07/13] [tree] Add regression test for integer overflow in TTree::Scan --- tree/treeplayer/test/regressions.cxx | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tree/treeplayer/test/regressions.cxx b/tree/treeplayer/test/regressions.cxx index 65e28e8b98220..7a0f9d082fa37 100644 --- a/tree/treeplayer/test/regressions.cxx +++ b/tree/treeplayer/test/regressions.cxx @@ -13,10 +13,13 @@ #include #include +#include "TTreePlayer.h" + #include "gtest/gtest.h" #include #include +#include // ROOT-10702 TEST(TTreeReaderRegressions, CompositeTypeWithNameClash) @@ -336,3 +339,62 @@ TEST(TTreeReaderRegressions, XYZVectors) gSystem->Unlink(filename); } } + +// https://github.com/root-project/root/issues/20226 +TEST(TTreeScan, IntOverflow) +{ + struct DatasetRAII { + const char *fTreeName{"tree_20226"}; + const char *fFileName{"tree_20226.root"}; + DatasetRAII() + { + auto file = std::make_unique(fFileName, "recreate"); + auto tree = std::make_unique(fTreeName, fTreeName); + + int val{}; + tree->Branch("val", &val); + for (; val < 10; val++) + tree->Fill(); + file->Write(); + } + + ~DatasetRAII() { std::remove(fFileName); } + } dataset; + + auto file = std::make_unique(dataset.fFileName); + std::unique_ptr tree{file->Get(dataset.fTreeName)}; + + std::ostringstream strCout; + { + if (auto *treePlayer = static_cast(tree->GetPlayer())) { + struct FileRAII { + const char *fPath; + FileRAII(const char *name) : fPath(name) {} + ~FileRAII() { std::remove(fPath); } + } redirectFile{"tree_20226_regression_redirect.txt"}; + treePlayer->SetScanRedirect(true); + treePlayer->SetScanFileName(redirectFile.fPath); + tree->Scan("val", "", "", TTree::kMaxEntries, 3); + + std::ifstream redirectStream(redirectFile.fPath); + std::stringstream redirectOutput; + redirectOutput << redirectStream.rdbuf(); + + const static std::string expectedScanOut{ + R"Scan(************************ +* Row * val * +************************ +* 3 * 3 * +* 4 * 4 * +* 5 * 5 * +* 6 * 6 * +* 7 * 7 * +* 8 * 8 * +* 9 * 9 * +************************ +)Scan"}; + EXPECT_EQ(redirectOutput.str(), expectedScanOut); + } else + throw std::runtime_error("Could not retrieve TTreePlayer from main tree!"); + } +} \ No newline at end of file From 77ecbdc05ab0362b6aea9c0c3c4198bfc15954e6 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Fri, 31 Oct 2025 20:45:11 +0100 Subject: [PATCH 08/13] [tree] Add regression test for integer overflow in TTree::Draw --- tree/treeplayer/test/regressions.cxx | 31 ++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tree/treeplayer/test/regressions.cxx b/tree/treeplayer/test/regressions.cxx index 7a0f9d082fa37..68ec824e0b961 100644 --- a/tree/treeplayer/test/regressions.cxx +++ b/tree/treeplayer/test/regressions.cxx @@ -397,4 +397,35 @@ TEST(TTreeScan, IntOverflow) } else throw std::runtime_error("Could not retrieve TTreePlayer from main tree!"); } +} + +// https://github.com/root-project/root/issues/20228 +TEST(TTreeDraw, IntOverflow) +{ + struct DatasetRAII { + const char *fTreeName{"tree_20228"}; + const char *fFileName{"tree_20228.root"}; + DatasetRAII() + { + auto file = std::make_unique(fFileName, "recreate"); + auto tree = std::make_unique(fTreeName, fTreeName); + + int val{}; + tree->Branch("val", &val); + for (; val < 10; val++) + tree->Fill(); + file->Write(); + } + + ~DatasetRAII() { std::remove(fFileName); } + } dataset; + + auto file = std::make_unique(dataset.fFileName); + std::unique_ptr tree{file->Get(dataset.fTreeName)}; + + // TTree::Draw returns the number of entries selected. In this case it should be 7, + // but due to the regression, it was zero + tree->SetMaxEntryLoop(TTree::kMaxEntries); + auto nEntriesSelected = tree->Draw("val", "", "", TTree::kMaxEntries, 3); + EXPECT_EQ(nEntriesSelected, 7); } \ No newline at end of file From 4d90586ec7590b794e85cf55eb56a10541c1915b Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Wed, 29 Oct 2025 18:10:52 +0100 Subject: [PATCH 09/13] [tree] Test complex friendship hierarchies to the same chain The general idea for this test suite is to have one single source of data, a TChain, that is befriended by a long enough friendship hierarchy, namely four consecutive layers of friends. The outermost dataset is the one that drives the event loop, either via SetBranchAddress + raw loop or via TTree::Scan. The data comes from the innermost TChain. All the combinations of TTree/TChain at the various steps of the hierarchy are tried. Furthermore, TTree::Scan is called with an increasingly larger starting entry to ensure that even when starting e.g. from an entry of the second tree of the innermost chain the Scan works well. --- tree/treeplayer/test/CMakeLists.txt | 2 + tree/treeplayer/test/gh20033.cxx | 434 ++++++++++++++++++++++++++++ 2 files changed, 436 insertions(+) create mode 100644 tree/treeplayer/test/gh20033.cxx diff --git a/tree/treeplayer/test/CMakeLists.txt b/tree/treeplayer/test/CMakeLists.txt index 0d84cd4c802e2..713341479770e 100644 --- a/tree/treeplayer/test/CMakeLists.txt +++ b/tree/treeplayer/test/CMakeLists.txt @@ -19,6 +19,8 @@ ROOT_ADD_GTEST(treeplayer_gh16804 gh16804.cxx LIBRARIES TreePlayer) ROOT_ADD_GTEST(treeplayer_gh16805 gh16805.cxx LIBRARIES TreePlayer) +ROOT_ADD_GTEST(treeplayer_gh20033 gh20033.cxx LIBRARIES TreePlayer) + ROOT_ADD_GTEST(treeplayer_leafs leafs.cxx LIBRARIES TreePlayer) add_custom_command(TARGET treeplayer_leafs POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/data.h data.h) diff --git a/tree/treeplayer/test/gh20033.cxx b/tree/treeplayer/test/gh20033.cxx new file mode 100644 index 0000000000000..f4b6fa47a1aa3 --- /dev/null +++ b/tree/treeplayer/test/gh20033.cxx @@ -0,0 +1,434 @@ +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include + +// Meaning of the parameters: first 4 booleans indicate whether to use TTree or TChain for that +// step, the int indicates which entry to start from when calling Scan +struct GH20033Regression : public ::testing::TestWithParam> { + + constexpr static std::array fStepZeroFileNames{"tree_gh20033_regression_stepzero_0.root", + "tree_gh20033_regression_stepzero_1.root"}; + + constexpr static auto fStepOneFileName{"tree_gh20033_regression_stepone.root"}; + constexpr static auto fStepTwoFileName{"tree_gh20033_regression_steptwo.root"}; + constexpr static auto fStepThreeFileName{"tree_gh20033_regression_stepthree.root"}; + constexpr static auto fStepFourFileName{"tree_gh20033_regression_stepfour.root"}; + + constexpr static auto fStepZeroTreeName{"stepzerotree"}; + constexpr static auto fStepOneTreeName{"steponetree"}; + constexpr static auto fStepTwoTreeName{"steptwotree"}; + constexpr static auto fStepThreeTreeName{"stepthreetree"}; + constexpr static auto fStepFourTreeName{"stepfourtree"}; + + constexpr static auto fTotalEntries{20}; + + // In the test we will try calling TTree::Scan with different starting entries, + // namely 0, 3, 5, 7, 9, 10, 11, 13, 15, 17, 19 + const static std::unordered_map fStartingEntriesToOutput; + constexpr static std::array fScanExpectedOutputs{ + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 0 * 0 * 0 * +* 1 * 1 * 2 * +* 2 * 2 * 4 * +* 3 * 3 * 6 * +* 4 * 4 * 8 * +* 5 * 5 * 10 * +* 6 * 6 * 12 * +* 7 * 7 * 14 * +* 8 * 8 * 16 * +* 9 * 9 * 18 * +* 10 * 10 * 20 * +* 11 * 11 * 22 * +* 12 * 12 * 24 * +* 13 * 13 * 26 * +* 14 * 14 * 28 * +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 3 * 3 * 6 * +* 4 * 4 * 8 * +* 5 * 5 * 10 * +* 6 * 6 * 12 * +* 7 * 7 * 14 * +* 8 * 8 * 16 * +* 9 * 9 * 18 * +* 10 * 10 * 20 * +* 11 * 11 * 22 * +* 12 * 12 * 24 * +* 13 * 13 * 26 * +* 14 * 14 * 28 * +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 5 * 5 * 10 * +* 6 * 6 * 12 * +* 7 * 7 * 14 * +* 8 * 8 * 16 * +* 9 * 9 * 18 * +* 10 * 10 * 20 * +* 11 * 11 * 22 * +* 12 * 12 * 24 * +* 13 * 13 * 26 * +* 14 * 14 * 28 * +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 7 * 7 * 14 * +* 8 * 8 * 16 * +* 9 * 9 * 18 * +* 10 * 10 * 20 * +* 11 * 11 * 22 * +* 12 * 12 * 24 * +* 13 * 13 * 26 * +* 14 * 14 * 28 * +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 9 * 9 * 18 * +* 10 * 10 * 20 * +* 11 * 11 * 22 * +* 12 * 12 * 24 * +* 13 * 13 * 26 * +* 14 * 14 * 28 * +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 10 * 10 * 20 * +* 11 * 11 * 22 * +* 12 * 12 * 24 * +* 13 * 13 * 26 * +* 14 * 14 * 28 * +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 11 * 11 * 22 * +* 12 * 12 * 24 * +* 13 * 13 * 26 * +* 14 * 14 * 28 * +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 13 * 13 * 26 * +* 14 * 14 * 28 * +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 15 * 15 * 30 * +* 16 * 16 * 32 * +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 17 * 17 * 34 * +* 18 * 18 * 36 * +* 19 * 19 * 38 * +****************************************** +)Scan", + R"Scan(****************************************** +* Row * stepZeroBr1 * stepZeroBr2 * +****************************************** +* 19 * 19 * 38 * +****************************************** +)Scan"}; + + static void MakeStepZeroFile(const char *name, const char *treename, int first, int last) + { + auto file = std::make_unique(name, "RECREATE"); + auto tree = std::make_unique(treename, treename); + + int br1{}; + int br2{}; + tree->Branch("stepZeroBr1", &br1); + tree->Branch("stepZeroBr2", &br2); + + for (br1 = first; br1 < last; ++br1) { + br2 = 2 * br1; + tree->Fill(); + } + + file->Write(); + } + + static void MakeOtherStepsFile(const char *name, const char *treename, int nEntries) + { + auto file = std::make_unique(name, "RECREATE"); + auto tree = std::make_unique(treename, treename); + + // empty entries, but crucially same number as the number of entries + // in the step zero chain, to ensure alignment + for (int i = 0; i < nEntries; ++i) { + tree->Fill(); + } + + file->Write(); + } + + static void SetUpTestSuite() + { + MakeStepZeroFile(fStepZeroFileNames[0], fStepZeroTreeName, 0, 10); + MakeStepZeroFile(fStepZeroFileNames[1], fStepZeroTreeName, 10, 20); + MakeOtherStepsFile(fStepOneFileName, fStepOneTreeName, fTotalEntries); + MakeOtherStepsFile(fStepTwoFileName, fStepTwoTreeName, fTotalEntries); + MakeOtherStepsFile(fStepThreeFileName, fStepThreeTreeName, fTotalEntries); + MakeOtherStepsFile(fStepFourFileName, fStepFourTreeName, fTotalEntries); + } + + static void TearDownTestSuite() + { + for (const auto &fileName : fStepZeroFileNames) + std::remove(fileName); + + std::remove(fStepOneFileName); + std::remove(fStepTwoFileName); + std::remove(fStepThreeFileName); + std::remove(fStepFourFileName); + } +}; + +const std::unordered_map GH20033Regression::fStartingEntriesToOutput = { + {0, 0}, {3, 1}, {5, 2}, {7, 3}, {9, 4}, {10, 5}, {11, 6}, {13, 7}, {15, 8}, {17, 9}, {19, 10}}; + +TEST_P(GH20033Regression, Test) +{ + // General idea: there is one TChain dataset which contains the actual + // data to traverse, in this case two branches. this TChain is connected + // to a hierarchy of friends. In general this could be arbitrarily long. + // For the purpose of this test, this is the friendship chain: + // - TChain "step zero" + // --> befriended by TTree "step one" + // (this TTree could either be standalone, or part of a TChain itself) + // --> befriended by TTree "step two" + // (this TTree could either be standalone, or part of a TChain itself) + // + // When the boundaries between trees of the TChain "step zero" is reached, the act + // of switching over to a new tree should trigger an update notification across + // the whole friendship chain, most importantly because the memory address provided + // by the user is known by the top-most befriender (TChain "step two"). + // This test ensures smooth update notification triggers across the friendship chain, + // irrespective of whether the intermediate datasets are TTree/TChain. + + auto &&[stepOneTChain, stepTwoTChain, stepThreeTChain, stepFourTChain, startingEntry] = GetParam(); + + // The step zero dataset needs to be a TChain and have more than one TTree + // to recreate the scenarios when one component of the friendship chain + // is a TChain switching to a new TTree and thus triggering the update + auto stepZeroDataset = std::make_unique(fStepZeroTreeName); + for (const auto &fileName : fStepZeroFileNames) + stepZeroDataset->Add(fileName); + + // Step one dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepOneFileLifeLine{}; + std::unique_ptr stepOneDataset{}; + if (stepOneTChain) { + auto chain = std::make_unique(fStepOneTreeName); + chain->Add(fStepOneFileName); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepZeroDataset.get()); + stepOneDataset = std::move(chain); + } else { + stepOneFileLifeLine = std::make_unique(fStepOneFileName); + stepOneDataset = std::unique_ptr{stepOneFileLifeLine->Get(fStepOneTreeName)}; + stepOneDataset->AddFriend(stepZeroDataset.get()); + } + + // Step two dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepTwoFileLifeLine{}; + std::unique_ptr stepTwoDataset{}; + if (stepTwoTChain) { + auto chain = std::make_unique(fStepTwoTreeName); + chain->Add(fStepTwoFileName); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepOneDataset.get()); + stepTwoDataset = std::move(chain); + } else { + stepTwoFileLifeLine = std::make_unique(fStepTwoFileName); + stepTwoDataset = std::unique_ptr{stepTwoFileLifeLine->Get(fStepTwoTreeName)}; + stepTwoDataset->AddFriend(stepOneDataset.get()); + } + + // Step three dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepThreeFileLifeLine{}; + std::unique_ptr stepThreeDataset{}; + if (stepThreeTChain) { + auto chain = std::make_unique(fStepThreeTreeName); + chain->Add(fStepThreeFileName); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepTwoDataset.get()); + stepThreeDataset = std::move(chain); + } else { + stepThreeFileLifeLine = std::make_unique(fStepThreeFileName); + stepThreeDataset = std::unique_ptr{stepThreeFileLifeLine->Get(fStepThreeTreeName)}; + stepThreeDataset->AddFriend(stepTwoDataset.get()); + } + + // Step four dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepFourFileLifeLine{}; + std::unique_ptr stepFourDataset{}; + if (stepFourTChain) { + auto chain = std::make_unique(fStepFourTreeName); + chain->Add(fStepFourFileName); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepThreeDataset.get()); + stepFourDataset = std::move(chain); + } else { + stepFourFileLifeLine = std::make_unique(fStepFourFileName); + stepFourDataset = std::unique_ptr{stepFourFileLifeLine->Get(fStepFourTreeName)}; + stepFourDataset->AddFriend(stepThreeDataset.get()); + } + + // Set the branch addresses on the top-most befriender, i.e. step two dataset. + // This is what establishes the memory addresses where the user wants data to be read into. + // These addresses need to be propagated across the friendship chain, so that they can be + // connected to the true TBranch addresses to fill the data as its being read from disk. + int stepZeroBr1 = -1, stepZeroBr2 = -1; + ASSERT_EQ(stepFourDataset->SetBranchAddress("stepZeroBr1", &stepZeroBr1), 0); + ASSERT_EQ(stepFourDataset->SetBranchAddress("stepZeroBr2", &stepZeroBr2), 0); + + std::vector expectedStepZeroBr1(fTotalEntries); + std::vector expectedStepZeroBr2(fTotalEntries); + std::iota(expectedStepZeroBr1.begin(), expectedStepZeroBr1.end(), 0); + std::generate(expectedStepZeroBr2.begin(), expectedStepZeroBr2.end(), [n = 0]() mutable { + auto res = n * 2; + n++; + return res; + }); + + for (Long64_t i = 0; i < stepFourDataset->GetEntriesFast(); ++i) { + stepFourDataset->GetEntry(i); + EXPECT_EQ(expectedStepZeroBr1[i], stepZeroBr1); + EXPECT_EQ(expectedStepZeroBr2[i], stepZeroBr2); + } + + // Now test with TTree::Scan + std::ostringstream strCout; + { + if (auto *treePlayer = static_cast(stepFourDataset->GetPlayer())) { + struct FileRAII { + const char *fPath; + FileRAII(const char *name) : fPath(name) {} + ~FileRAII() { std::remove(fPath); } + } redirectFile{"tree_gh20033_morefriends_redirect.txt"}; + treePlayer->SetScanRedirect(true); + treePlayer->SetScanFileName(redirectFile.fPath); + stepFourDataset->Scan("stepZeroBr1:stepZeroBr2", "", "colsize=12", TTree::kMaxEntries, startingEntry); + + std::ifstream redirectStream(redirectFile.fPath); + std::stringstream redirectOutput; + redirectOutput << redirectStream.rdbuf(); + + EXPECT_EQ(redirectOutput.str(), fScanExpectedOutputs[fStartingEntriesToOutput.at(startingEntry)]); + } else + throw std::runtime_error("Could not retrieve TTreePlayer from main tree!"); + } +} + +INSTANTIATE_TEST_SUITE_P( + Run, GH20033Regression, + ::testing::Combine(::testing::Values(false, true), ::testing::Values(false, true), ::testing::Values(false, true), + ::testing::Values(false, true), ::testing::Values(0, 3, 5, 7, 9, 10, 11, 13, 15, 17, 19)), + // Extra parenthesis around lambda to avoid preprocessor errors, see + // https://stackoverflow.com/questions/79438894/lambda-with-structured-bindings-inside-macro-call + ([](const testing::TestParamInfo ¶mInfo) { + auto &&[stepOneTChain, stepTwoTChain, stepThreeTChain, stepFourTChain, startingEntry] = paramInfo.param; + // googletest only accepts ASCII alphanumeric characters for labels + std::string label{}; + if (stepOneTChain) + label += "StepOneTChain_"; + else + label += "StepOneTTree_"; + if (stepTwoTChain) + label += "StepTwoTChain_"; + else + label += "StepTwoTTree_"; + if (stepThreeTChain) + label += "StepThreeTChain_"; + else + label += "StepThreeTTree_"; + if (stepFourTChain) + label += "StepFourTChain_"; + else + label += "StepFourTTree_"; + label += std::to_string(startingEntry); + return label; + })); From 8a30ac533f28151fcf1a4a24cc69c19bde8d89e5 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Thu, 30 Oct 2025 01:18:08 +0100 Subject: [PATCH 10/13] [tree] Test same branch name appearing in various friendship levels Add a test suite for the scenario of multiple levels of friendship hierarchy where all TTree/TChain datasets in the hierarchy have a branch with exactly the same name. Call TTree::Scan and make sure that the correct branch from the correct TTree is accessed with every possible combination of using TTree/TChain to steer the execution or only one or more of the branches of the hierarchy. This scenario represents the use case of comparing the same quantity which can pass through various derivation steps and each step must be compared with the other ones. --- tree/treeplayer/test/CMSDASClasses.hxx | 69 ++++ tree/treeplayer/test/CMSDASClassesLinkDef.hxx | 8 + tree/treeplayer/test/CMakeLists.txt | 3 +- tree/treeplayer/test/gh20033.cxx | 380 ++++++++++++++++++ 4 files changed, 459 insertions(+), 1 deletion(-) create mode 100644 tree/treeplayer/test/CMSDASClasses.hxx create mode 100644 tree/treeplayer/test/CMSDASClassesLinkDef.hxx diff --git a/tree/treeplayer/test/CMSDASClasses.hxx b/tree/treeplayer/test/CMSDASClasses.hxx new file mode 100644 index 0000000000000..c030b54e21d26 --- /dev/null +++ b/tree/treeplayer/test/CMSDASClasses.hxx @@ -0,0 +1,69 @@ +#ifndef CMS_DAS_CLASSES +#define CMS_DAS_CLASSES + +#include +#include + +namespace DAS { +struct Weight { + int v{1}; + int i{0}; + operator int() const { return v; } + + // https://root.cern/manual/io_custom_classes/#restrictions-on-types-root-io-can-handle + Weight() = default; + + // For vector initialization in test + Weight(int a, int b) : v(a), i(b) {} + + // https://root.cern/manual/io_custom_classes/#the-classdef-macro + // NV avoids marking the methods in I/O as virtual (not needed for this struct) + ClassDefNV(Weight, 1); +}; + +// Also this is a new type and it gets stored to disk, needs a dictionary +using Weights = std::vector; + +// Stored, needs a dictionary +struct AbstractEvent { + // https://root.cern/manual/io_custom_classes/#restrictions-on-types-root-io-can-handle + AbstractEvent() = default; + + virtual ~AbstractEvent() = default; + // Destructor is defined, thus we need to implement rule of five + // https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c21-if-you-define-or-delete-any-copy-move-or-destructor-function-define-or-delete-them-all + AbstractEvent(const AbstractEvent &) = default; + AbstractEvent &operator=(const AbstractEvent &) = default; + AbstractEvent(AbstractEvent &&) = default; + AbstractEvent &operator=(AbstractEvent &&) = default; + + Weights weights; + + inline int Weight() const { return weights.front(); } + + // https://root.cern/manual/io_custom_classes/#the-classdef-macro + ClassDef(AbstractEvent, 1); +}; + +struct GenEvent : public AbstractEvent { + // Default constructor for ROOT I/O + GenEvent() = default; + + // This is a derived class of a pure virtual class, we need to override all virtual functions + // https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final + ~GenEvent() override = default; + // Destructor is defined, thus we need to implement rule of five + // https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c21-if-you-define-or-delete-any-copy-move-or-destructor-function-define-or-delete-them-all + GenEvent(const GenEvent &) = default; + GenEvent &operator=(const GenEvent &) = default; + GenEvent(GenEvent &&) = default; + GenEvent &operator=(GenEvent &&) = default; + + // https://root.cern/manual/io_custom_classes/#the-classdef-macro + // Override because this is a derived class + ClassDefOverride(GenEvent, 1); +}; + +} // namespace DAS + +#endif // CMS_DAS_CLASSES diff --git a/tree/treeplayer/test/CMSDASClassesLinkDef.hxx b/tree/treeplayer/test/CMSDASClassesLinkDef.hxx new file mode 100644 index 0000000000000..f5aa6e0debbf0 --- /dev/null +++ b/tree/treeplayer/test/CMSDASClassesLinkDef.hxx @@ -0,0 +1,8 @@ +#ifdef __CLING__ + +#pragma link C++ class DAS::Weight+; +#pragma link C++ class std::vector+; +#pragma link C++ class DAS::AbstractEvent+; +#pragma link C++ class DAS::GenEvent+; + +#endif diff --git a/tree/treeplayer/test/CMakeLists.txt b/tree/treeplayer/test/CMakeLists.txt index 713341479770e..0a89dd44a061c 100644 --- a/tree/treeplayer/test/CMakeLists.txt +++ b/tree/treeplayer/test/CMakeLists.txt @@ -19,7 +19,8 @@ ROOT_ADD_GTEST(treeplayer_gh16804 gh16804.cxx LIBRARIES TreePlayer) ROOT_ADD_GTEST(treeplayer_gh16805 gh16805.cxx LIBRARIES TreePlayer) -ROOT_ADD_GTEST(treeplayer_gh20033 gh20033.cxx LIBRARIES TreePlayer) +ROOT_GENERATE_DICTIONARY(CMSDASClassesDict ${CMAKE_CURRENT_SOURCE_DIR}/CMSDASClasses.hxx LINKDEF CMSDASClassesLinkDef.hxx NO_CXXMODULE DEPENDENCIES Core Tree RIO) +ROOT_ADD_GTEST(treeplayer_gh20033 gh20033.cxx CMSDASClassesDict.cxx LIBRARIES TreePlayer) ROOT_ADD_GTEST(treeplayer_leafs leafs.cxx LIBRARIES TreePlayer) add_custom_command(TARGET treeplayer_leafs POST_BUILD diff --git a/tree/treeplayer/test/gh20033.cxx b/tree/treeplayer/test/gh20033.cxx index f4b6fa47a1aa3..98ac63c078637 100644 --- a/tree/treeplayer/test/gh20033.cxx +++ b/tree/treeplayer/test/gh20033.cxx @@ -13,6 +13,8 @@ #include +#include "CMSDASClasses.hxx" + // Meaning of the parameters: first 4 booleans indicate whether to use TTree or TChain for that // step, the int indicates which entry to start from when calling Scan struct GH20033Regression : public ::testing::TestWithParam> { @@ -432,3 +434,381 @@ INSTANTIATE_TEST_SUITE_P( label += std::to_string(startingEntry); return label; })); + +// A similar situation as above, but now all TTree/TChain datasets in the hierarchy have the same +// branch, with different values. The std::string parameters represent the name of the expression +// to pass to TTree::Scan for the corresponding friend dataset branch +struct GH20033SameBranchName + : public ::testing::TestWithParam> { + + constexpr static std::array fStepZeroFileNames{"tree_gh20033_samebranchname_stepzero_0.root", + "tree_gh20033_samebranchname_stepzero_1.root"}; + + constexpr static auto fStepOneFileName{"tree_gh20033_samebranchname_stepone.root"}; + constexpr static auto fStepTwoFileName{"tree_gh20033_samebranchname_steptwo.root"}; + constexpr static auto fStepThreeFileName{"tree_gh20033_samebranchname_stepthree.root"}; + constexpr static auto fStepFourFileName{"tree_gh20033_samebranchname_stepfour.root"}; + + constexpr static auto fStepZeroTreeName{"step0tree"}; + constexpr static auto fStepOneTreeName{"step1tree"}; + constexpr static auto fStepTwoTreeName{"step2tree"}; + constexpr static auto fStepThreeTreeName{"step3tree"}; + constexpr static auto fStepFourTreeName{"step4tree"}; + + // The names of the chains use the token 'chai' for simplicity, it has same number of characters as 'tree', to make + // it easier to build the expected TTree::Scan output header later on in the test itself. + constexpr static auto fStepZeroChainName{"step0chai"}; + constexpr static auto fStepOneChainName{"step1chai"}; + constexpr static auto fStepTwoChainName{"step2chai"}; + constexpr static auto fStepThreeChainName{"step3chai"}; + constexpr static auto fStepFourChainName{"step4chai"}; + + constexpr static auto fTotalEntries{4}; + + static void WriteData(const char *name, const char *treename, int first, int last) + { + auto file = std::make_unique(name, "RECREATE"); + auto tree = std::make_unique(treename, treename); + + int val{}; + tree->Branch("value", &val); + + DAS::GenEvent evt{}; + evt.weights = std::vector{{0, 1}}; + tree->Branch("genEvent", &evt); + + for (val = first; val < last; ++val) { + evt.weights[0].v = val; + evt.weights[0].i = val; + tree->Fill(); + } + + file->Write(); + } + + GH20033SameBranchName() + { + auto &&[_1, _2, _3, _4, stepZeroScan, stepOneScan, stepTwoScan, stepThreeScan, stepFourScan, _5] = GetParam(); + + // All scan strings are empty, we will skip the test, avoid useless work here + if (stepZeroScan.empty() && stepOneScan.empty() && stepTwoScan.empty() && stepThreeScan.empty() && + stepFourScan.empty()) + return; + + // We want to write out TTrees in files which might have either the same name of the TChain that will + // wrap them, or their own different name than the one of the TChain that will wrap them later in the + // test. + WriteData(fStepZeroFileNames[0], (stepZeroScan.empty() ? fStepZeroTreeName : stepZeroScan.c_str()), 0, + fTotalEntries / 2); + WriteData(fStepZeroFileNames[1], (stepZeroScan.empty() ? fStepZeroTreeName : stepZeroScan.c_str()), + fTotalEntries / 2, fTotalEntries); + WriteData(fStepOneFileName, (stepOneScan.empty() ? fStepOneTreeName : stepOneScan.c_str()), 100, + 100 + fTotalEntries); + WriteData(fStepTwoFileName, (stepTwoScan.empty() ? fStepTwoTreeName : stepTwoScan.c_str()), 200, + 200 + fTotalEntries); + WriteData(fStepThreeFileName, (stepThreeScan.empty() ? fStepThreeTreeName : stepThreeScan.c_str()), 300, + 300 + fTotalEntries); + WriteData(fStepFourFileName, (stepFourScan.empty() ? fStepFourTreeName : stepFourScan.c_str()), 400, + 400 + fTotalEntries); + } + + ~GH20033SameBranchName() + { + for (const auto &fileName : fStepZeroFileNames) + std::remove(fileName); + + std::remove(fStepOneFileName); + std::remove(fStepTwoFileName); + std::remove(fStepThreeFileName); + std::remove(fStepFourFileName); + } +}; + +TEST_P(GH20033SameBranchName, Test) +{ + // General idea: all the datasets in the hierarchy of friendships share the same + // branch name "val". Each dataset will have different values to distinguish itself. + // Try all different combinations of TTree::Scan such that all different branches + // in the friend hierarchy are tested, specifying the corresponding dataset name + + auto &&[stepOneTChain, stepTwoTChain, stepThreeTChain, stepFourTChain, stepZeroScan, stepOneScan, stepTwoScan, + stepThreeScan, stepFourScan, expressionName] = GetParam(); + if (stepZeroScan.empty() && stepOneScan.empty() && stepTwoScan.empty() && stepThreeScan.empty() && + stepFourScan.empty()) + GTEST_SKIP() << "Skipping test with empty TTree::Scan expression"; + + // Precompute which was the name of the TTree written to disk for this test run + const char *stepZeroToDiskName = stepZeroScan.empty() ? fStepZeroTreeName : stepZeroScan.c_str(); + const char *stepOneToDiskName = stepOneScan.empty() ? fStepOneTreeName : stepOneScan.c_str(); + const char *stepTwoToDiskName = stepTwoScan.empty() ? fStepTwoTreeName : stepTwoScan.c_str(); + const char *stepThreeToDiskName = stepThreeScan.empty() ? fStepThreeTreeName : stepThreeScan.c_str(); + const char *stepFourToDiskName = stepFourScan.empty() ? fStepFourTreeName : stepFourScan.c_str(); + + // The step zero dataset needs to be a TChain and have more than one TTree + // to recreate the scenarios when one component of the friendship chain + // is a TChain switching to a new TTree and thus triggering the update + auto stepZeroDataset = std::make_unique(fStepZeroChainName); + for (const auto &fileName : fStepZeroFileNames) + stepZeroDataset->Add((std::string(fileName) + "?#" + stepZeroToDiskName).c_str()); + + // Step one dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepOneFileLifeLine{}; + std::unique_ptr stepOneDataset{}; + if (stepOneTChain) { + auto chain = std::make_unique(fStepOneChainName); + chain->Add((std::string(fStepOneFileName) + "?#" + stepOneToDiskName).c_str()); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepZeroDataset.get()); + stepOneDataset = std::move(chain); + } else { + stepOneFileLifeLine = std::make_unique(fStepOneFileName); + stepOneDataset = std::unique_ptr{stepOneFileLifeLine->Get(stepOneToDiskName)}; + stepOneDataset->AddFriend(stepZeroDataset.get()); + } + + // Step two dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepTwoFileLifeLine{}; + std::unique_ptr stepTwoDataset{}; + if (stepTwoTChain) { + auto chain = std::make_unique(fStepTwoChainName); + chain->Add((std::string(fStepTwoFileName) + "?#" + stepTwoToDiskName).c_str()); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepOneDataset.get()); + stepTwoDataset = std::move(chain); + } else { + stepTwoFileLifeLine = std::make_unique(fStepTwoFileName); + stepTwoDataset = std::unique_ptr{stepTwoFileLifeLine->Get(stepTwoToDiskName)}; + stepTwoDataset->AddFriend(stepOneDataset.get()); + } + + // Step three dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepThreeFileLifeLine{}; + std::unique_ptr stepThreeDataset{}; + if (stepThreeTChain) { + auto chain = std::make_unique(fStepThreeChainName); + chain->Add((std::string(fStepThreeFileName) + "?#" + stepThreeToDiskName).c_str()); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepTwoDataset.get()); + stepThreeDataset = std::move(chain); + } else { + stepThreeFileLifeLine = std::make_unique(fStepThreeFileName); + stepThreeDataset = std::unique_ptr{stepThreeFileLifeLine->Get(stepThreeToDiskName)}; + stepThreeDataset->AddFriend(stepTwoDataset.get()); + } + + // Step four dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepFourFileLifeLine{}; + std::unique_ptr stepFourDataset{}; + if (stepFourTChain) { + auto chain = std::make_unique(fStepFourChainName); + chain->Add((std::string(fStepFourFileName) + "?#" + stepFourToDiskName).c_str()); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepThreeDataset.get()); + stepFourDataset = std::move(chain); + } else { + stepFourFileLifeLine = std::make_unique(fStepFourFileName); + stepFourDataset = std::unique_ptr{stepFourFileLifeLine->Get(stepFourToDiskName)}; + stepFourDataset->AddFriend(stepThreeDataset.get()); + } + + // Build the TTree::Scan call + std::string scanExpression{}; + // step zero is always a TChain, we can safely use the name of the chain + if (!stepZeroScan.empty()) + scanExpression += "step0chai." + expressionName + ":"; + + // Other steps can be either accessed via TTree or TChain + // In the case of TTree, the scan expression can only + // have the name of the TTree written on disk. In the + // case of TChain, the scan expression has the name of the + // TChain, which could be different than the name of the TTree + // it wraps. + if (!stepOneScan.empty()) { + if (stepOneTChain) + scanExpression += "step1chai." + expressionName + ":"; + else + scanExpression += std::string(stepOneToDiskName) + "." + expressionName + ":"; + } + + if (!stepTwoScan.empty()) { + if (stepTwoTChain) + scanExpression += "step2chai." + expressionName + ":"; + else + scanExpression += std::string(stepTwoToDiskName) + "." + expressionName + ":"; + } + + if (!stepThreeScan.empty()) { + if (stepThreeTChain) + scanExpression += "step3chai." + expressionName + ":"; + else + scanExpression += std::string(stepThreeToDiskName) + "." + expressionName + ":"; + } + + if (!stepFourScan.empty()) { + if (stepFourTChain) + scanExpression += "step4chai." + expressionName + ":"; + else + scanExpression += std::string(stepFourToDiskName) + "." + expressionName + ":"; + } + + // The scan expression can never be empty, we have already dealt with that case by skipping the test at the beginning + if (scanExpression.back() == ':') + scanExpression.pop_back(); + + // Build the expected TTree::Scan output + std::string expectedScan{}; + const auto rowHeaderNChars{12}; // 10 is the Row header, +2 column delimiters + const auto colNChars{15 + 2 + 1}; // colsize=15, +2 margin, +1 right column delimiter + // Header, opening row + std::string starRow(rowHeaderNChars, '*'); + if (!stepZeroScan.empty()) + starRow += std::string(colNChars, '*'); + if (!stepOneScan.empty()) + starRow += std::string(colNChars, '*'); + if (!stepTwoScan.empty()) + starRow += std::string(colNChars, '*'); + if (!stepThreeScan.empty()) + starRow += std::string(colNChars, '*'); + if (!stepFourScan.empty()) + starRow += std::string(colNChars, '*'); + starRow += "\n"; + + expectedScan += starRow; + + // Header, branch titles + expectedScan += "* Row *"; + // We keep the same colsize in the output scan to make the building of the expected output easier. Cut the longer + // expression name to the same length as the other branch name. + std::string expressionToken{(expressionName == "genEvent.Weight()" ? "genEv" : "value")}; + if (!stepZeroScan.empty()) + expectedScan += " step0chai." + expressionToken + " *"; + if (!stepOneScan.empty()) + expectedScan += " " + std::string(stepOneTChain ? "step1chai" : stepOneToDiskName) + "." + expressionToken + " *"; + if (!stepTwoScan.empty()) + expectedScan += " " + std::string(stepTwoTChain ? "step2chai" : stepTwoToDiskName) + "." + expressionToken + " *"; + if (!stepThreeScan.empty()) + expectedScan += + " " + std::string(stepThreeTChain ? "step3chai" : stepThreeToDiskName) + "." + expressionToken + " *"; + if (!stepFourScan.empty()) + expectedScan += + " " + std::string(stepFourTChain ? "step4chai" : stepFourToDiskName) + "." + expressionToken + " *"; + expectedScan += "\n"; + + // Header, closing row + expectedScan += starRow; + + // main body rows + for (std::string n : {"0", "1", "2", "3"}) { + expectedScan += "* " + n + " *"; + if (!stepZeroScan.empty()) + expectedScan += " " + n + " *"; + if (!stepOneScan.empty()) + expectedScan += " 10" + n + " *"; + if (!stepTwoScan.empty()) + expectedScan += " 20" + n + " *"; + if (!stepThreeScan.empty()) + expectedScan += " 30" + n + " *"; + if (!stepFourScan.empty()) + expectedScan += " 40" + n + " *"; + expectedScan += "\n"; + } + + // Ending row + expectedScan += starRow; + + // Now test with TTree::Scan + std::ostringstream strCout; + { + if (auto *treePlayer = static_cast(stepFourDataset->GetPlayer())) { + struct FileRAII { + const char *fPath; + FileRAII(const char *name) : fPath(name) {} + ~FileRAII() { std::remove(fPath); } + } redirectFile{"tree_gh20033_samebranchname_redirect.txt"}; + treePlayer->SetScanRedirect(true); + treePlayer->SetScanFileName(redirectFile.fPath); + stepFourDataset->Scan(scanExpression.c_str(), "", "colsize=15"); + + std::ifstream redirectStream(redirectFile.fPath); + std::stringstream redirectOutput; + redirectOutput << redirectStream.rdbuf(); + + EXPECT_EQ(redirectOutput.str(), expectedScan); + } else + throw std::runtime_error("Could not retrieve TTreePlayer from main tree!"); + } +} + +INSTANTIATE_TEST_SUITE_P( + Run, GH20033SameBranchName, + ::testing::Combine( + // Whether to use TTree or TChain to represent one of the dataset layerss + ::testing::Values(false, true), ::testing::Values(false, true), ::testing::Values(false, true), + ::testing::Values(false, true), + // Components of the TTree::Scan call + ::testing::Values("", "step0tree", "step0chai"), ::testing::Values("", "step1tree", "step1chai"), + ::testing::Values("", "step2tree", "step2chai"), ::testing::Values("", "step3tree", "step3chai"), + ::testing::Values("", "step4tree", "step4chai"), + // Expression to evaluate + ::testing::Values("value", "genEvent.Weight()")), + // Extra parenthesis around lambda to avoid preprocessor errors, see + // https://stackoverflow.com/questions/79438894/lambda-with-structured-bindings-inside-macro-call + ([](const testing::TestParamInfo ¶mInfo) { + auto &&[stepOneTChain, stepTwoTChain, stepThreeTChain, stepFourTChain, stepZeroScan, stepOneScan, stepTwoScan, + stepThreeScan, stepFourScan, expressionName] = paramInfo.param; + // googletest only accepts ASCII alphanumeric characters for labels + std::string label{}; + if (stepOneTChain) + label += "StepOneTChain_"; + else + label += "StepOneTTree_"; + if (stepTwoTChain) + label += "StepTwoTChain_"; + else + label += "StepTwoTTree_"; + if (stepThreeTChain) + label += "StepThreeTChain_"; + else + label += "StepThreeTTree_"; + if (stepFourTChain) + label += "StepFourTChain_"; + else + label += "StepFourTTree_"; + + if (!stepZeroScan.empty()) + label += "StepZeroScan_" + stepZeroScan + "_"; + else + label += "StepZeroNoScan_"; + if (!stepOneScan.empty()) + label += "StepOneScan_" + stepOneScan + "_"; + else + label += "StepOneNoScan_"; + if (!stepTwoScan.empty()) + label += "StepTwoScan_" + stepTwoScan + "_"; + else + label += "StepTwoNoScan_"; + if (!stepThreeScan.empty()) + label += "StepThreeScan_" + stepThreeScan + "_"; + else + label += "StepThreeNoScan_"; + if (!stepFourScan.empty()) + label += "StepFourScan_" + stepFourScan + "_"; + else + label += "StepFourNoScan_"; + + if (expressionName == "value") + label += "value"; + else + label += "genEventWeight"; + + return label; + })); From d2fde120f44ce6004d6ae21600d2b1d0bbeafdb5 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Thu, 30 Oct 2025 10:22:14 +0100 Subject: [PATCH 11/13] [tree] Test a more complex TTree::Scan expression Introduce a test with a long friendship hierarchy where data is accessed via TTree::Scan. Every dataset in the hierarchy has the same branch name. The Scan expression uses different branch names from every dataset in the same component of the expression. This checks that the branch addresses are properly set and they don't get mixed up. --- tree/treeplayer/test/gh20033.cxx | 205 +++++++++++++++++++++++++++++++ 1 file changed, 205 insertions(+) diff --git a/tree/treeplayer/test/gh20033.cxx b/tree/treeplayer/test/gh20033.cxx index 98ac63c078637..19155ababf1b0 100644 --- a/tree/treeplayer/test/gh20033.cxx +++ b/tree/treeplayer/test/gh20033.cxx @@ -812,3 +812,208 @@ INSTANTIATE_TEST_SUITE_P( return label; })); + +// Another scenario where all datasets in the hierarchy have the same branch name but the different branches from the +// different datasets are accessed together in the same parts of a TTree::Scan expression. This checks that the various +// branch addresses are properly set and they don't get mixed up. +struct GH20033ComplexScanExpression : public ::testing::TestWithParam> { + + constexpr static std::array fStepZeroFileNames{ + "tree_gh20033_complexscanexpression_stepzero_0.root", "tree_gh20033_complexscanexpression_stepzero_1.root", + "tree_gh20033_complexscanexpression_stepzero_2.root"}; + + constexpr static auto fStepOneFileName{"tree_gh20033_complexscanexpression_stepone.root"}; + constexpr static auto fStepTwoFileName{"tree_gh20033_complexscanexpression_steptwo.root"}; + constexpr static auto fStepThreeFileName{"tree_gh20033_complexscanexpression_stepthree.root"}; + constexpr static auto fStepFourFileName{"tree_gh20033_complexscanexpression_stepfour.root"}; + + constexpr static auto fStepZeroTreeName{"stepzerotree"}; + constexpr static auto fStepOneTreeName{"steponetree"}; + constexpr static auto fStepTwoTreeName{"steptwotree"}; + constexpr static auto fStepThreeTreeName{"stepthreetree"}; + constexpr static auto fStepFourTreeName{"stepfourtree"}; + + constexpr static auto fTotalEntries{10}; + + static void WriteData(const char *name, const char *treename, int first, int last) + { + auto file = std::make_unique(name, "RECREATE"); + auto tree = std::make_unique(treename, treename); + + int val{}; + tree->Branch("value", &val); + + for (val = first; val < last; ++val) { + tree->Fill(); + } + + file->Write(); + } + + static void SetUpTestSuite() + { + WriteData(fStepZeroFileNames[0], fStepZeroTreeName, 0, 4); + WriteData(fStepZeroFileNames[1], fStepZeroTreeName, 4, 7); + WriteData(fStepZeroFileNames[2], fStepZeroTreeName, 7, 10); + WriteData(fStepOneFileName, fStepOneTreeName, 100, 100 + fTotalEntries); + WriteData(fStepTwoFileName, fStepTwoTreeName, 200, 200 + fTotalEntries); + WriteData(fStepThreeFileName, fStepThreeTreeName, 300, 300 + fTotalEntries); + WriteData(fStepFourFileName, fStepFourTreeName, 400, 400 + fTotalEntries); + } + + static void TearDownTestSuite() + { + for (const auto &fileName : fStepZeroFileNames) + std::remove(fileName); + + std::remove(fStepOneFileName); + std::remove(fStepTwoFileName); + std::remove(fStepThreeFileName); + std::remove(fStepFourFileName); + } +}; + +TEST_P(GH20033ComplexScanExpression, Test) +{ + + auto &&[stepOneTChain, stepTwoTChain, stepThreeTChain, stepFourTChain] = GetParam(); + + // The step zero dataset needs to be a TChain and have more than one TTree + // to recreate the scenarios when one component of the friendship chain + // is a TChain switching to a new TTree and thus triggering the update + auto stepZeroDataset = std::make_unique(fStepZeroTreeName); + for (const auto &fileName : fStepZeroFileNames) + stepZeroDataset->Add(fileName); + + // Step one dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepOneFileLifeLine{}; + std::unique_ptr stepOneDataset{}; + if (stepOneTChain) { + auto chain = std::make_unique(fStepOneTreeName); + chain->Add(fStepOneFileName); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepZeroDataset.get()); + stepOneDataset = std::move(chain); + } else { + stepOneFileLifeLine = std::make_unique(fStepOneFileName); + stepOneDataset = std::unique_ptr{stepOneFileLifeLine->Get(fStepOneTreeName)}; + stepOneDataset->AddFriend(stepZeroDataset.get()); + } + + // Step two dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepTwoFileLifeLine{}; + std::unique_ptr stepTwoDataset{}; + if (stepTwoTChain) { + auto chain = std::make_unique(fStepTwoTreeName); + chain->Add(fStepTwoFileName); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepOneDataset.get()); + stepTwoDataset = std::move(chain); + } else { + stepTwoFileLifeLine = std::make_unique(fStepTwoFileName); + stepTwoDataset = std::unique_ptr{stepTwoFileLifeLine->Get(fStepTwoTreeName)}; + stepTwoDataset->AddFriend(stepOneDataset.get()); + } + + // Step three dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepThreeFileLifeLine{}; + std::unique_ptr stepThreeDataset{}; + if (stepThreeTChain) { + auto chain = std::make_unique(fStepThreeTreeName); + chain->Add(fStepThreeFileName); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepTwoDataset.get()); + stepThreeDataset = std::move(chain); + } else { + stepThreeFileLifeLine = std::make_unique(fStepThreeFileName); + stepThreeDataset = std::unique_ptr{stepThreeFileLifeLine->Get(fStepThreeTreeName)}; + stepThreeDataset->AddFriend(stepTwoDataset.get()); + } + + // Step four dataset, can be a TChain or a standalone TTree. + // In the former case, the first tree of the TChain is used + // to befriend the chain from the previous step. + std::unique_ptr stepFourFileLifeLine{}; + std::unique_ptr stepFourDataset{}; + if (stepFourTChain) { + auto chain = std::make_unique(fStepFourTreeName); + chain->Add(fStepFourFileName); + chain->GetEntry(0); + chain->GetTree()->AddFriend(stepThreeDataset.get()); + stepFourDataset = std::move(chain); + } else { + stepFourFileLifeLine = std::make_unique(fStepFourFileName); + stepFourDataset = std::unique_ptr{stepFourFileLifeLine->Get(fStepFourTreeName)}; + stepFourDataset->AddFriend(stepThreeDataset.get()); + } + + std::ostringstream strCout; + { + if (auto *treePlayer = static_cast(stepFourDataset->GetPlayer())) { + struct FileRAII { + const char *fPath; + FileRAII(const char *name) : fPath(name) {} + ~FileRAII() { std::remove(fPath); } + } redirectFile{"tree_gh20033_complexscanexpression_redirect.txt"}; + treePlayer->SetScanRedirect(true); + treePlayer->SetScanFileName(redirectFile.fPath); + stepFourDataset->Scan("stepfourtree.value * stepzerotree.value:steponetree.value + steptwotree.value:" + "stepthreetree.value - stepzerotree.value", + "stepthreetree.value > 303", "colsize=50"); + + std::ifstream redirectStream(redirectFile.fPath); + std::stringstream redirectOutput; + redirectOutput << redirectStream.rdbuf(); + + EXPECT_EQ( + redirectOutput.str(), + R"Scan(*************************************************************************************************************************************************************************** +* Row * stepfourtree.value * stepzerotree.value * steponetree.value + steptwotree.value * stepthreetree.value - stepzerotree.value * +*************************************************************************************************************************************************************************** +* 4 * 1616 * 308 * 300 * +* 5 * 2025 * 310 * 300 * +* 6 * 2436 * 312 * 300 * +* 7 * 2849 * 314 * 300 * +* 8 * 3264 * 316 * 300 * +* 9 * 3681 * 318 * 300 * +*************************************************************************************************************************************************************************** +)Scan"); + } else + throw std::runtime_error("Could not retrieve TTreePlayer from main tree!"); + } +} + +INSTANTIATE_TEST_SUITE_P( + Run, GH20033ComplexScanExpression, + ::testing::Combine(::testing::Values(false, true), ::testing::Values(false, true), ::testing::Values(false, true), + ::testing::Values(false, true)), + // Extra parenthesis around lambda to avoid preprocessor errors, see + // https://stackoverflow.com/questions/79438894/lambda-with-structured-bindings-inside-macro-call + ([](const testing::TestParamInfo ¶mInfo) { + auto &&[stepOneTChain, stepTwoTChain, stepThreeTChain, stepFourTChain] = paramInfo.param; + // googletest only accepts ASCII alphanumeric characters for labels + std::string label{}; + if (stepOneTChain) + label += "StepOneTChain_"; + else + label += "StepOneTTree_"; + if (stepTwoTChain) + label += "StepTwoTChain_"; + else + label += "StepTwoTTree_"; + if (stepThreeTChain) + label += "StepThreeTChain_"; + else + label += "StepThreeTTree_"; + if (stepFourTChain) + label += "StepFourTChain"; + else + label += "StepFourTTree"; + return label; + })); From b9bd32b1cf79c6cae36ad23ebff301ba696cda84 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Fri, 31 Oct 2025 00:04:25 +0100 Subject: [PATCH 12/13] [tree] Add regression test for branch name lookup Tests that a TChain with a name different than its inner TTree can find a branch of the TTree when the user passes a branch lookup string of the form "chainName.branchName". --- tree/treeplayer/test/regressions.cxx | 61 +++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/tree/treeplayer/test/regressions.cxx b/tree/treeplayer/test/regressions.cxx index 68ec824e0b961..700c98b3a1f5d 100644 --- a/tree/treeplayer/test/regressions.cxx +++ b/tree/treeplayer/test/regressions.cxx @@ -428,4 +428,63 @@ TEST(TTreeDraw, IntOverflow) tree->SetMaxEntryLoop(TTree::kMaxEntries); auto nEntriesSelected = tree->Draw("val", "", "", TTree::kMaxEntries, 3); EXPECT_EQ(nEntriesSelected, 7); -} \ No newline at end of file +} + +// https://github.com/root-project/root/issues/20248 +TEST(TTreeScan, chainNameWithDifferentTreeName) +{ + struct DatasetRAII { + const char *fTreeName{"tree_20248"}; + const char *fFileName{"tree_20248.root"}; + DatasetRAII() + { + auto file = std::make_unique(fFileName, "recreate"); + auto tree = std::make_unique(fTreeName, fTreeName); + + int val{}; + tree->Branch("val", &val); + for (; val < 5; val++) + tree->Fill(); + file->Write(); + } + + ~DatasetRAII() { std::remove(fFileName); } + } dataset; + + TChain c{"differentNameForChain"}; + c.Add((std::string(dataset.fFileName) + "?#" + dataset.fTreeName).c_str()); + + ASSERT_NE(c.FindBranch("differentNameForChain.val"), nullptr); + + std::ostringstream strCout; + { + if (auto *treePlayer = static_cast(c.GetPlayer())) { + struct FileRAII { + const char *fPath; + FileRAII(const char *name) : fPath(name) {} + ~FileRAII() { std::remove(fPath); } + } redirectFile{"tree_20248_regression_redirect.txt"}; + treePlayer->SetScanRedirect(true); + treePlayer->SetScanFileName(redirectFile.fPath); + c.Scan("differentNameForChain.val", "", "colsize=30"); + + std::ifstream redirectStream(redirectFile.fPath); + std::stringstream redirectOutput; + redirectOutput << redirectStream.rdbuf(); + + const static std::string expectedScanOut{ + R"Scan(********************************************* +* Row * differentNameForChain.val * +********************************************* +* 0 * 0 * +* 1 * 1 * +* 2 * 2 * +* 3 * 3 * +* 4 * 4 * +********************************************* +)Scan"}; + EXPECT_EQ(redirectOutput.str(), expectedScanOut); + } else + throw std::runtime_error("Could not retrieve TTreePlayer from main tree!"); + } +} From aacd4c160d3e3532517e397c87dd08b56d77da9e Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Fri, 31 Oct 2025 00:14:19 +0100 Subject: [PATCH 13/13] [tree] Add regression test for TTree friend branch name lookup Tests that a TTree that has a TChain friend can retrieve a branch from the inner TTree of the friend TChain by prefixing the branch name with the chain name, even if the inner TTree of the friend TChain might have a different name than the chain name. --- tree/treeplayer/test/regressions.cxx | 79 ++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tree/treeplayer/test/regressions.cxx b/tree/treeplayer/test/regressions.cxx index 700c98b3a1f5d..5973230bdf632 100644 --- a/tree/treeplayer/test/regressions.cxx +++ b/tree/treeplayer/test/regressions.cxx @@ -488,3 +488,82 @@ TEST(TTreeScan, chainNameWithDifferentTreeName) throw std::runtime_error("Could not retrieve TTreePlayer from main tree!"); } } + +// https://github.com/root-project/root/issues/20249 +TEST(TTreeScan, TTreeGetBranchOfFriendTChain) +{ + struct DatasetRAII { + const char *fTreeNameStepZero{"tree_20249_zero"}; + const char *fFileNameStepZero{"tree_20249_zero.root"}; + const char *fTreeNameStepOne{"tree_20249_one"}; + const char *fFileNameStepOne{"tree_20249_one.root"}; + + void WriteData(const char *name, const char *treename, int first, int last) + { + auto file = std::make_unique(name, "RECREATE"); + auto tree = std::make_unique(treename, treename); + + int value{}; + tree->Branch("value", &value); + + for (value = first; value < last; ++value) { + tree->Fill(); + } + + file->Write(); + } + + DatasetRAII() + { + WriteData(fFileNameStepZero, fTreeNameStepZero, 3, 7); + WriteData(fFileNameStepOne, fTreeNameStepOne, 0, 4); + } + + ~DatasetRAII() + { + std::remove(fFileNameStepZero); + std::remove(fFileNameStepOne); + } + } dataset; + + auto chain0 = std::make_unique("stepzerochain"); + chain0->Add((std::string(dataset.fFileNameStepZero) + "?#" + dataset.fTreeNameStepZero).c_str()); + + auto file1 = std::make_unique(dataset.fFileNameStepOne); + std::unique_ptr tree1{file1->Get(dataset.fTreeNameStepOne)}; + tree1->AddFriend(chain0.get()); + + ASSERT_NE(tree1->FindBranch("stepzerochain.value"), nullptr); + + std::ostringstream strCout; + { + if (auto *treePlayer = static_cast(tree1->GetPlayer())) { + struct FileRAII { + const char *fPath; + FileRAII(const char *name) : fPath(name) {} + ~FileRAII() { std::remove(fPath); } + } redirectFile{"tree_20249_regression_redirect.txt"}; + treePlayer->SetScanRedirect(true); + treePlayer->SetScanFileName(redirectFile.fPath); + + tree1->Scan("value:stepzerochain.value", "", "colsize=24"); + + std::ifstream redirectStream(redirectFile.fPath); + std::stringstream redirectOutput; + redirectOutput << redirectStream.rdbuf(); + + const static std::string expectedScanOut{ + R"Scan(****************************************************************** +* Row * value * stepzerochain.value * +****************************************************************** +* 0 * 0 * 3 * +* 1 * 1 * 4 * +* 2 * 2 * 5 * +* 3 * 3 * 6 * +****************************************************************** +)Scan"}; + EXPECT_EQ(redirectOutput.str(), expectedScanOut); + } else + throw std::runtime_error("Could not retrieve TTreePlayer from main tree!"); + } +}