smaheshwar-pltr commented on code in PR #13225:
URL: https://github.com/apache/iceberg/pull/13225#discussion_r2714066223


##########
spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/sql/TestTableEncryption.java:
##########
@@ -184,12 +195,15 @@ public void testConcurrentReplaceTransactions() {
             .buildTable(tableIdent, schema)
             .withProperty("encryption.key-id", UnitestKMS.MASTER_KEY_NAME1)
             .replaceTransaction();
-    firstReplace.newFastAppend().appendFile(file).commit();
     firstReplace.commitTransaction();
 
+    // This second replace transaction fails but then retries after refreshing 
latest metadata.
     secondReplace.commitTransaction();
 
     Table afterSecondReplace = validationCatalog.loadTable(tableIdent);
+
+    // This tests that encryption keys are maintained on refreshing different 
metadata - if
+    // they are not, the table will be unreadable and this will fail.

Review Comment:
   (I've verified that this test fails without the encryption-key tracking 
changes described in https://github.com/apache/iceberg/pull/14752. In other 
words, if we instead always construct the encryption manager using the 
`current` metadata in the table operations class which is wrong, this test 
fails and is the only one to fail. I've also documented the nuance here)



-- 
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