[Spark][MR]: Reuse InputFilesDecryptor #4595
Conversation
There was a problem hiding this comment.
cc @ggershinsky @RussellSpitzer @flyrain you guys are probably interested in this (particularly @ggershinsky).
|
LGTM. A nice refactor, which follows the same approach in #2905. Do we have any test from Spark/Hive side? Also cc @stevenzwu since he is the author of #2905. |
|
Nice optimization, LGTM |
|
Thanks for your review and sorry for my late reply.
This just did some small code optimizations so I didn't add the test. What kind of tests do you think should be added? @flyrain |
It's OK to not add tests. I'm just curious that which test covers the changed code. |
I think we don't have relevant tests cause encryption hasn't been implemented yet, only PlaintextEncryptionManager at the moment. |
It is a bit scary, but probably is fine since we've got #2905 in. |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
Reuse InputFilesDecryptor in Spark BaseDataReader and MR IcebergInputFormat to reduce code duplication.