-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-36820: [C++] Remove a needless return in Acero ScanNode::AddScanTasks #36821
Conversation
|
(I'm not sure the details of this code too.) If we don't need to return |
Let me wait for weston's idea...Actually I'm not sure whether we need to return |
@westonpace would you mind take a look? Just two line fix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I changed the need for this future in #32768 but it was very clumsy of me to leave this return statement here. Sorry.
You could probably change this method to return void but the method will also hopefully be going away entirely as part of #35889 so I'm fine if you want to stick with this minimal change.
And yes, we should be able to remove |
Just want to remove it because I seen this while reading the code. If you think it's not neccessary, feel free to close this patch :-) |
Close this issue and wait for #35889 to be merged :-) |
Rationale for this change
ScanNode::AddScanTasks
has multiple return value. This patch remove the latter one.What changes are included in this PR?
Just remove some code.
Are these changes tested?
no
Are there any user-facing changes?
no