ORC-485. Add API to write encrypted files. #386
Conversation
omalley
force-pushed
the
orc-485
branch
2 times, most recently
from
b236036
to
ded9b20
Compare
omalley
force-pushed
the
orc-485
branch
3 times, most recently
from
737bb63
to
1709627
Compare
Fixes apache#386 Change-Id: I0a4d6b86256ee1658c4f52df54679b8f1d6e9433 Signed-off-by: Owen O'Malley <omalley@apache.org>
|
Any comments on this before I commit it? |
Fixes apache#386 Signed-off-by: Owen O'Malley <omalley@apache.org>
| * Get the encryption key for this column. | ||
| * @return the encryption key. | ||
| */ | ||
| EncryptionKey getKey(); |
There was a problem hiding this comment.
use getColumnKey() to be more descriptive? Also the scope of these keys are stripe or file? Can it be explicitly called out?
There was a problem hiding this comment.
They are for the entire file, I'll update the name.
There was a problem hiding this comment.
More precisely, it is the key description so I updated the name to getKeyDescription.
| * Get the local key for the footer. | ||
| * @return the local decrypted key or null if it isn't available | ||
| */ | ||
| Key getFooterKey() throws IOException; |
There was a problem hiding this comment.
getStripeFooterKey() or if file level getFileFooterKey()?
There was a problem hiding this comment.
It is the file footer, so I'll update this name too.
| @@ -114,7 +114,7 @@ | |||
| * Get the statistics about the columns in the file. | |||
| * @return the information about the column | |||
| */ | |||
| ColumnStatistics[] getStatistics(); | |||
| ColumnStatistics[] getStatistics() throws IOException; | |||
There was a problem hiding this comment.
is this required? when does this throw exception?
There was a problem hiding this comment.
Good catch. I can't add a throw here without breaking backwards compatibility. In general, we won't throw here, but if the key provider is flaky, we can get an exception. I guess I'll make it an unchecked exception.
| @@ -642,6 +642,11 @@ | |||
| </exclusion> | |||
| </exclusions> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>org.jetbrains</groupId> | |||
There was a problem hiding this comment.
nit: required? can we use Objects.requireNotNull instead?
There was a problem hiding this comment.
I really want an annotation to mark the intent and help with static analysis. I don't need a run time check.
These changes provide most of the API changes for column encryption.