smaheshwar-pltr commented on code in PR #17404:
URL: https://github.com/apache/iceberg/pull/17404#discussion_r3670129887
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -126,44 +125,59 @@ protected String tableName() {
@Override
public FileIO io() {
+ // Avoid current() (which may refresh) on the unencrypted fast path.
+ return tableKeyId == null ? fileIO : io(current());
+ }
+
+ @Override
+ public EncryptionManager encryption() {
+ return tableKeyId == null ? PlaintextEncryptionManager.instance() :
encryption(current());
+ }
+
+ private FileIO io(TableMetadata metadata) {
if (tableKeyId == null) {
return fileIO;
}
- if (encryptingFileIO == null) {
- encryptingFileIO = EncryptingFileIO.combine(fileIO, encryption());
- }
-
- return encryptingFileIO;
+ return EncryptingFileIO.combine(fileIO, encryption(metadata));
}
- @Override
- public EncryptionManager encryption() {
- if (encryptionManager != null) {
- return encryptionManager;
+ /**
+ * Returns an {@link EncryptionManager} sourced from the given metadata's
keys. Callers pass the
+ * metadata whose keys the manager must resolve: {@code current()} for a
committed table, or the
+ * uncommitted metadata for staged transaction operations. Memoized by
metadata identity so
+ * repeated calls with the same metadata reuse one manager.
+ */
+ private EncryptionManager encryption(TableMetadata metadata) {
+ if (tableKeyId == null) {
+ return PlaintextEncryptionManager.instance();
}
- if (tableKeyId != null) {
- Preconditions.checkArgument(
- keyManagementClient != null,
- "Cannot create encryption manager without a key management client.
Consider setting the '%s' catalog property",
- CatalogProperties.ENCRYPTION_KMS_IMPL);
-
- Map<String, String> encryptionProperties =
- ImmutableMap.of(
- TableProperties.ENCRYPTION_TABLE_KEY,
- tableKeyId,
- TableProperties.ENCRYPTION_DEK_LENGTH,
- String.valueOf(encryptionDekLength));
-
- encryptionManager =
- EncryptionUtil.createEncryptionManager(
- encryptedKeys, encryptionProperties, keyManagementClient);
- } else {
- return PlaintextEncryptionManager.instance();
+ synchronized (managerCacheLock) {
Review Comment:
remember https://github.com/smaheshwar-pltr/iceberg/pull/10 PR description
where there's no sync in table operations but that's fine. also think abt how
that pr desc applies to this pr in general
--
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]
