[ntuple] Remove unused RNTupleJoinTableMethods#21468
[ntuple] Remove unused RNTupleJoinTableMethods#21468enirolf wants to merge 2 commits intoroot-project:masterfrom
RNTupleJoinTableMethods#21468Conversation
These methods currently have no practical use. They can be re-added if the need for them arises again.
The TODO on line 138 of `RNTupleJoinTable.cxx` will be addressed in a follow-up PR.
Test Results 22 files 22 suites 3d 4h 59m 59s ⏱️ For more details on these failures, see this check. Results for commit 697333f. ♻️ This comment has been updated with latest results. |
| ROOT::NTupleSize_t | ||
| ROOT::Experimental::Internal::RNTupleJoinTable::GetEntryIndex(const std::vector<void *> &valuePtrs) const | ||
| ROOT::NTupleSize_t ROOT::Experimental::Internal::RNTupleJoinTable::GetEntryIndex(const std::vector<void *> &valuePtrs, | ||
| bool throwOnMultipleMatches) const |
There was a problem hiding this comment.
I find this bool param a bit odd: can't the function always return the first index + e.g. a bool telling whether multiple matches exist? At that point the caller can decide if they want to throw or not.
If throwing on multiple matches was a common enough pattern then I'd see the point of this, but it seems that the caller almost never wants to throw..
There was a problem hiding this comment.
If this is currently not used at all, we could also, for the time being, not care if there are multiple matches or not.
| ROOT::NTupleSize_t | ||
| ROOT::Experimental::Internal::RNTupleJoinTable::GetEntryIndex(const std::vector<void *> &valuePtrs) const | ||
| ROOT::NTupleSize_t ROOT::Experimental::Internal::RNTupleJoinTable::GetEntryIndex(const std::vector<void *> &valuePtrs, | ||
| bool throwOnMultipleMatches) const |
There was a problem hiding this comment.
If this is currently not used at all, we could also, for the time being, not care if there are multiple matches or not.
| if (throwOnMultipleMatches && entriesForMapping->size() > 1) { | ||
| // TODO (fdegeus): make error message more informative | ||
| throw RException(R__FAIL("found more than one corresponding entry index")); | ||
| } |
There was a problem hiding this comment.
This will trigger for multiple matches in one partition, but not for multiple matches in different partitions. Is this the expected semantics? If not, and if it's indeed not needed at the moment, I would agree with not adding it for now...
5 participants
These methods currently are not used in practice, or foreseen to be used. They can be re-added if the need arises.