Skip to content

[RDataFrame] Fix FilterAvailable for aliased columns - #23729

Open
MSK-005 wants to merge 1 commit into
root-project:masterfrom
MSK-005:fix-filteravailable-alias-and-define
Open

MSK-005 wants to merge 1 commit into
root-project:masterfrom
MSK-005:fix-filteravailable-alias-and-define

Conversation

@MSK-005

@MSK-005 MSK-005 commented Oct 15, 2026 •

Copy link
Copy Markdown

This Pull request:

Changes or fixes:

Previously, running FilterAvailable() on an aliased column of a TTree resulted in that column not being read and, consequently, the function returned 0 entries. The bug was that, in the case of TTree, RColumnRegister::GetReader(), RColumnRegister::GetReaderUnchecked(), and GetColumnReader() did not resolve the alias before fetching a reader. This resulted in the functions looking for column names that did not exist, and thus returned nullptr.

The fix is to resolve the alias names before anything else. A regression test has also been added.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR addresses only the Alias bug from issue #22759. Together with PR #24055 (review pending), it fixes the TTree case in issue #22759.

@github-actions

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 21h 47m 30s ⏱️
 3 889 tests  3 887 ✅ 0 💤 2 ❌
79 458 runs  79 456 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit e5f0721.

@vepadulano

Copy link
Copy Markdown
Member

Dear @MSK-005 ,

Thanks for submitting this contribution! I have run the CI, the failures seem to be unrelated at the moment.

I wanted to ask you for clarification regarding your PR description. You mention that this PR is going to fix two bugs. When I read the description of the second bug, I cannot find anything in the code changes that reflects it. Are there any code changes at the moment that address the second bug you mention?

In more general terms, when opening a PR ideally it should involve one semantic change, to make the review process easier. At the very least, the changes related to two different topics (e.g. fixing two different bugs) should go to separate commits.

Can you clarify how this PR fixes the second bug? Also, can you add at least another test to make the second bug evident and confirm the fix? Again, ideally the second bug fix should go in a separate PR.

@vepadulano vepadulano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above.

@MSK-005

MSK-005 commented Oct 18, 2026 •

Copy link
Copy Markdown
Author

Dear @vepadulano,

I have edited my PR's description. This PR fixes only the Alias bug and doesn't address the Define bug. I am labelling this PR as ready for review. The PR to fix the Define bug is #24055.

I apologize for any inconvenience caused.

@MSK-005
MSK-005 marked this pull request as ready for review October 18, 2026 09:21
@MSK-005
MSK-005 requested a review from hageboeck as a code owner October 18, 2026 09:21
@MSK-005
MSK-005 requested a review from vepadulano October 18, 2026 09:26
@MSK-005 MSK-005 closed this Nov 8, 2026
@MSK-005
MSK-005 deleted the fix-filteravailable-alias-and-define branch November 8, 2026 14:26
@MSK-005
MSK-005 restored the fix-filteravailable-alias-and-define branch November 8, 2026 14:34
@MSK-005 MSK-005 reopened this Nov 8, 2026
@MSK-005
MSK-005 force-pushed the fix-filteravailable-alias-and-define branch from e5f0721 to 147b050 Compare November 8, 2026 18:11

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants