Hugo-WB commented on code in PR #16353:
URL: https://github.com/apache/iceberg/pull/16353#discussion_r4006835107
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1451,6 +1451,7 @@ private Builder rewriteSnapshotsInternal(Collection<Long>
idsToRemove, boolean s
}
removeStatistics(snapshotId);
removePartitionStatistics(snapshotId);
+ removeEncryptionKey(snapshot.keyId());
Review Comment:
👍 I see. If we're in favor of not relying on this invariant, I agree this
PR should not be implemented:
Curious for your thoughts/review on:
https://github.com/apache/iceberg/pull/18102 then!
Will defer to others that are more familiar with the original motivations
behind making snapshot encryption keys stored in the snapshot metadata on
whether we should rely on this invariant of snapshot -> key being 1:1.
I think it would be nice if encryption keys were removed with the snapshot
and not require expire_snapshots with cleanExpiredMetadata.
Curious for thoughts from @ggershinsky and @singhpk234
--
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]
