gaborkaszab commented on code in PR #17545:
URL: https://github.com/apache/iceberg/pull/17545#discussion_r4048964666


##########
api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java:
##########
@@ -140,6 +145,16 @@ public InputFile newInputFile(ManifestListFile 
manifestList) {
     }
   }
 
+  @Override
+  public InputFile newInputFile(String location, String keyId) {
+    if (keyId != null) {

Review Comment:
   I think such a precondition would break the contract of `EncryptionFileIO`. 
If I'm not mistaken, `EncryptingFileIO` theoretically can be created even if 
the table is not encrypted, and other functions here also seem to branch on 
`key_metadata` being null or not, where null defaults to the non-encrypting 
case. I think for consistency we should follow that pattern with the `keyId` 
variation of `newFileIO` functions.
   WDYT @RussellSpitzer ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to