-
Notifications
You must be signed in to change notification settings - Fork 483
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
[C++] read encrypted ORC file #1993
base: main
Are you sure you want to change the base?
Conversation
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.
Thank you for making a PR, @zxf216 .
We have been tracking this via
BTW, could you make the CI happy by fixing all failures?
Thanks @zxf216! Let me take a first pass. cc @coderex2522 @ffacs |
|
I converted it to |
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 @zxf216 again for the excellent work! I just took a preliminary look and have left some inline comments. As you can see that we have various CI jobs for test cases, code format, license check, etc. I strongly recommend splitting the PR into smaller ones to make the review process easier.
@@ -837,5 +837,4 @@ namespace orc { | |||
} | |||
return nullptr; | |||
} | |||
|
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.
Please revert the unnecessary change.
@@ -39,7 +39,7 @@ namespace orc { | |||
// classes that hold data members so we can maintain binary compatibility | |||
struct ReaderOptionsPrivate; | |||
struct RowReaderOptionsPrivate; | |||
|
|||
class KeyProvider; |
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.
The c++/include/orc
folder is public and files in it will be installed. If the KeyProvider
is also public, please move its definition to the include folder as well. Otherwise, we should remove the related function from here.
@@ -0,0 +1,247 @@ | |||
// Copyright 2010-present vivo, Inc. All rights reserved. |
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.
Please note that we have a license check which does not allow custom modification to the license header in this repo. I'm not sure if this violates any ASF rule to add something like this. @dongjoon-hyun Do you have any concern?
std::shared_ptr<SecretKeySpec> getStripeKey(long stripe); | ||
}; | ||
|
||
// 加密变体,每个列一个 |
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.
Please use English for comment.
Co-authored-by: Gang Wu <[email protected]>
Co-authored-by: Gang Wu <[email protected]>
Co-authored-by: Gang Wu <[email protected]>
What changes were proposed in this pull request?
you can use C++ version code to read encrypted ORC files
ORC file structure:links
Why are the changes needed?
The C++ version is not yet able to read encrypted ORC files now
How was this patch tested?