Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Fix the ORC decoding bug for the timestamp data #17570
base: branch-25.02
Are you sure you want to change the base?
Fix the ORC decoding bug for the timestamp data #17570
Changes from 13 commits
c9e706c
bd93523
9055ebf
7ebb33c
0c53f08
6df48c6
ba00561
1f4e0ea
16a8590
2896eb6
4eee986
22b6e70
ce352ea
2029eb2
5c47aaa
9d2cac0
cd0d272
c2a2992
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Could we use a
cuda::std::optional
instead?Aside: if we had C++23 the optional usage in this file would be a great use case for
std::optional::and_then
.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.
A naive question from someone with very limited knowledge of our I/O: I only see one usage of this function outside of the
gpuDecodeOrcColumnData
kernel, and that's insidegpuDecodeNullsAndStringDictionaries
. Are guaranteed to not need this cache in that case (again, asking naively since I don't really know what that function does).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.
I haven't studied the function
gpuDecodeNullsAndStringDictionaries
yet, and I'm on the conservative side applying the cache to existing code.I'm not quite sure about how to use
cuda::std::optional
and what's its advantage when it comes to function default arguments. Since the function will have side effect on the optional object, should I pass it by referencecuda::std::optional<X>&
? But this way I can't give it a default valuecuda::std::nullopt
, and have to passcuda::std::nullopt
to all function calls that don't needX
. Is my understanding correct?