diff --git a/docs/src/main/sphinx/object-storage/metastores.md b/docs/src/main/sphinx/object-storage/metastores.md index 7cafbed03f33..b266c367a9c9 100644 --- a/docs/src/main/sphinx/object-storage/metastores.md +++ b/docs/src/main/sphinx/object-storage/metastores.md @@ -208,6 +208,39 @@ properties: - ::: +(iceberg-hive-catalog)= +### Iceberg-specific Hive catalog configuration properties + +When using the Hive catalog, the Iceberg connector supports the same +{ref}`general Thrift metastore configuration properties ` +as previously described with the following additional property: + +:::{list-table} Iceberg Hive catalog configuration property +:widths: 35, 50, 15 +:header-rows: 1 + +* - Property name + - Description + - Default +* - `iceberg.hive-catalog.locking-enabled` + - Commit to tables using Hive locks. + - `true` +::: + +:::{warning} +Setting `iceberg.hive-catalog.locking-enabled=false` will cause the catalog to +commit to tables without using Hive locks. This should only be set to false if all +following conditions are met: + +* [HIVE-26882](https://issues.apache.org/jira/browse/HIVE-26882) is available on + the Hive metastore server. Requires version 2.3.10, 4.0.0-beta-1 or later. +* [HIVE-28121](https://issues.apache.org/jira/browse/HIVE-28121) is available on + the Hive metastore server, if it is backed by MySQL or MariaDB. Requires version + 2.3.10, 4.1.0, 4.0.1 or later. +* All other catalogs committing to tables that this catalogs commits to are also + on Iceberg 1.3 or later, and disabled Hive locks on commit. +::: + (hive-thrift-metastore-authentication)= ### Thrift metastore authentication diff --git a/lib/trino-metastore/src/main/java/io/trino/metastore/HiveMetastore.java b/lib/trino-metastore/src/main/java/io/trino/metastore/HiveMetastore.java index ff2f44ee0946..74459612e4f0 100644 --- a/lib/trino-metastore/src/main/java/io/trino/metastore/HiveMetastore.java +++ b/lib/trino-metastore/src/main/java/io/trino/metastore/HiveMetastore.java @@ -90,7 +90,7 @@ default boolean useSparkTableStatistics() * alter one field of a table object previously acquired from getTable is * probably not what you want. */ - void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges); + void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Map environmentContext); void renameTable(String databaseName, String tableName, String newDatabaseName, String newTableName); diff --git a/lib/trino-metastore/src/main/java/io/trino/metastore/cache/CachingHiveMetastore.java b/lib/trino-metastore/src/main/java/io/trino/metastore/cache/CachingHiveMetastore.java index 0c22b77d39f9..a173d65c9e56 100644 --- a/lib/trino-metastore/src/main/java/io/trino/metastore/cache/CachingHiveMetastore.java +++ b/lib/trino-metastore/src/main/java/io/trino/metastore/cache/CachingHiveMetastore.java @@ -651,10 +651,10 @@ public void dropTable(String databaseName, String tableName, boolean deleteData) } @Override - public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges) + public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Map environmentContext) { try { - delegate.replaceTable(databaseName, tableName, newTable, principalPrivileges); + delegate.replaceTable(databaseName, tableName, newTable, principalPrivileges, environmentContext); } finally { invalidateTable(databaseName, tableName); diff --git a/lib/trino-metastore/src/main/java/io/trino/metastore/tracing/TracingHiveMetastore.java b/lib/trino-metastore/src/main/java/io/trino/metastore/tracing/TracingHiveMetastore.java index 4549a69af5cc..287813c26558 100644 --- a/lib/trino-metastore/src/main/java/io/trino/metastore/tracing/TracingHiveMetastore.java +++ b/lib/trino-metastore/src/main/java/io/trino/metastore/tracing/TracingHiveMetastore.java @@ -236,13 +236,13 @@ public void dropTable(String databaseName, String tableName, boolean deleteData) } @Override - public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges) + public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Map environmentContext) { Span span = tracer.spanBuilder("HiveMetastore.replaceTable") .setAttribute(SCHEMA, databaseName) .setAttribute(TABLE, tableName) .startSpan(); - withTracing(span, () -> delegate.replaceTable(databaseName, tableName, newTable, principalPrivileges)); + withTracing(span, () -> delegate.replaceTable(databaseName, tableName, newTable, principalPrivileges, environmentContext)); } @Override diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/HiveMetastoreBackedDeltaLakeMetastore.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/HiveMetastoreBackedDeltaLakeMetastore.java index 42aef31456ff..86b583185b8d 100644 --- a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/HiveMetastoreBackedDeltaLakeMetastore.java +++ b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/HiveMetastoreBackedDeltaLakeMetastore.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.deltalake.metastore; +import com.google.common.collect.ImmutableMap; import io.trino.metastore.Database; import io.trino.metastore.HiveMetastore; import io.trino.metastore.PrincipalPrivileges; @@ -108,7 +109,7 @@ public void createTable(Table table, PrincipalPrivileges principalPrivileges) @Override public void replaceTable(Table table, PrincipalPrivileges principalPrivileges) { - delegate.replaceTable(table.getDatabaseName(), table.getTableName(), table, principalPrivileges); + delegate.replaceTable(table.getDatabaseName(), table.getTableName(), table, principalPrivileges, ImmutableMap.of()); } @Override diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/file/DeltaLakeFileMetastoreTableOperations.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/file/DeltaLakeFileMetastoreTableOperations.java index 69019cf22133..01583d1529b0 100644 --- a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/file/DeltaLakeFileMetastoreTableOperations.java +++ b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/file/DeltaLakeFileMetastoreTableOperations.java @@ -47,6 +47,6 @@ public void commitToExistingTable(SchemaTableName schemaTableName, long version, .putAll(tableMetadataParameters(version, schemaString, tableComment)) .buildKeepingLast(); Table updatedTable = currentTable.withParameters(parameters); - metastore.replaceTable(currentTable.getDatabaseName(), currentTable.getTableName(), updatedTable, buildInitialPrivilegeSet(currentTable.getOwner().orElseThrow())); + metastore.replaceTable(currentTable.getDatabaseName(), currentTable.getTableName(), updatedTable, buildInitialPrivilegeSet(currentTable.getOwner().orElseThrow()), ImmutableMap.of()); } } diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/thrift/DeltaLakeThriftMetastoreTableOperations.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/thrift/DeltaLakeThriftMetastoreTableOperations.java index 055b3c0d7b81..428233beea64 100644 --- a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/thrift/DeltaLakeThriftMetastoreTableOperations.java +++ b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/thrift/DeltaLakeThriftMetastoreTableOperations.java @@ -66,7 +66,7 @@ public void commitToExistingTable(SchemaTableName schemaTableName, long version, .buildKeepingLast(); Table updatedTable = currentTable.withParameters(parameters); - metastore.replaceTable(currentTable.getDatabaseName(), currentTable.getTableName(), updatedTable, buildInitialPrivilegeSet(currentTable.getOwner().orElseThrow())); + metastore.replaceTable(currentTable.getDatabaseName(), currentTable.getTableName(), updatedTable, buildInitialPrivilegeSet(currentTable.getOwner().orElseThrow()), ImmutableMap.of()); } finally { thriftMetastore.releaseTableLock(lockId); diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeFileOperations.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeFileOperations.java index 08c613b385aa..3315a33a3884 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeFileOperations.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeFileOperations.java @@ -14,6 +14,7 @@ package io.trino.plugin.deltalake; import com.google.common.collect.HashMultiset; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultiset; import com.google.common.collect.Multiset; import com.google.common.io.Resources; @@ -1265,6 +1266,6 @@ private void removeLastTransactionVersionFromMetastore(String schemaName, String Table newMetastoreTable = Table.builder(table) .setParameters(filterKeys(table.getParameters(), key -> !key.equals("trino_last_transaction_version"))) .build(); - metastore.replaceTable(table.getDatabaseName(), table.getTableName(), newMetastoreTable, buildInitialPrivilegeSet(table.getOwner().orElseThrow())); + metastore.replaceTable(table.getDatabaseName(), table.getTableName(), newMetastoreTable, buildInitialPrivilegeSet(table.getOwner().orElseThrow()), ImmutableMap.of()); } } diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/TestDeltaLakeMetastoreAccessOperations.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/TestDeltaLakeMetastoreAccessOperations.java index 5211e96c5490..bd9f4a711020 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/TestDeltaLakeMetastoreAccessOperations.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/TestDeltaLakeMetastoreAccessOperations.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.deltalake.metastore; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultiset; import com.google.common.collect.Maps; import com.google.common.collect.Multiset; @@ -574,7 +575,7 @@ private void removeMetadataCachingPropertiesFromMetastore(String tableName) Table newMetastoreTable = Table.builder(table) .setParameters(Maps.filterKeys(table.getParameters(), key -> !key.equals("trino_last_transaction_version"))) .build(); - metastore.replaceTable(table.getDatabaseName(), table.getTableName(), newMetastoreTable, buildInitialPrivilegeSet(table.getOwner().orElseThrow())); + metastore.replaceTable(table.getDatabaseName(), table.getTableName(), newMetastoreTable, buildInitialPrivilegeSet(table.getOwner().orElseThrow()), ImmutableMap.of()); } private Session sessionWithStoreTableMetadata(boolean storeTableMetadata) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/TrinoViewHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/TrinoViewHiveMetastore.java index 1e485066bdba..1ceeb04f7ee8 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/TrinoViewHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/TrinoViewHiveMetastore.java @@ -92,7 +92,7 @@ public void createView(ConnectorSession session, SchemaTableName schemaViewName, throw new ViewAlreadyExistsException(schemaViewName); } - metastore.replaceTable(schemaViewName.getSchemaName(), schemaViewName.getTableName(), table, principalPrivileges); + metastore.replaceTable(schemaViewName.getSchemaName(), schemaViewName.getTableName(), table, principalPrivileges, ImmutableMap.of()); return; } @@ -225,6 +225,6 @@ private void replaceView(ConnectorSession session, SchemaTableName viewName, Tab PrincipalPrivileges principalPrivileges = isUsingSystemSecurity ? NO_PRIVILEGES : buildInitialPrivilegeSet(session.getUser()); - metastore.replaceTable(viewName.getSchemaName(), viewName.getTableName(), viewBuilder.build(), principalPrivileges); + metastore.replaceTable(viewName.getSchemaName(), viewName.getTableName(), viewBuilder.build(), principalPrivileges, ImmutableMap.of()); } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java index 2b9ec5e6fe77..f8f29e900ae9 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java @@ -594,7 +594,7 @@ public synchronized void dropTable(ConnectorSession session, String databaseName public synchronized void replaceTable(String databaseName, String tableName, Table table, PrincipalPrivileges principalPrivileges) { - setExclusive(delegate -> delegate.replaceTable(databaseName, tableName, table, principalPrivileges)); + setExclusive(delegate -> delegate.replaceTable(databaseName, tableName, table, principalPrivileges, ImmutableMap.of())); } public synchronized void renameTable(String databaseName, String tableName, String newDatabaseName, String newTableName) @@ -3063,7 +3063,7 @@ public void run(HiveMetastore metastore, AcidTransaction transaction) metastore.alterTransactionalTable(newTable, transaction.getAcidTransactionId(), transaction.getWriteId(), principalPrivileges); } else { - metastore.replaceTable(newTable.getDatabaseName(), newTable.getTableName(), newTable, principalPrivileges); + metastore.replaceTable(newTable.getDatabaseName(), newTable.getTableName(), newTable, principalPrivileges, ImmutableMap.of()); } } @@ -3077,7 +3077,7 @@ public void undo(HiveMetastore metastore, AcidTransaction transaction) metastore.alterTransactionalTable(oldTable, transaction.getAcidTransactionId(), transaction.getWriteId(), principalPrivileges); } else { - metastore.replaceTable(oldTable.getDatabaseName(), oldTable.getTableName(), oldTable, principalPrivileges); + metastore.replaceTable(oldTable.getDatabaseName(), oldTable.getTableName(), oldTable, principalPrivileges, ImmutableMap.of()); } } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java index 91cabc99d878..1a2db8e2424f 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java @@ -586,7 +586,7 @@ public synchronized void dropTable(String databaseName, String tableName, boolea } @Override - public synchronized void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges) + public synchronized void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Map environmentContext) { Table table = getRequiredTable(databaseName, tableName); if (!table.getDatabaseName().equals(databaseName) || !table.getTableName().equals(tableName)) { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java index 73ce14153834..1d0980e07629 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java @@ -606,7 +606,7 @@ private void deleteDir(Location path) } @Override - public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges) + public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Map environmentContext) { if (!tableName.equals(newTable.getTableName()) || !databaseName.equals(newTable.getDatabaseName())) { throw new TrinoException(NOT_SUPPORTED, "Table rename is not yet supported by Glue service"); diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/BridgingHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/BridgingHiveMetastore.java index e66c5fdf76e1..95d02b9a3e84 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/BridgingHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/BridgingHiveMetastore.java @@ -247,9 +247,9 @@ public void dropTable(String databaseName, String tableName, boolean deleteData) } @Override - public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges) + public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Map environmentContext) { - alterTable(databaseName, tableName, toMetastoreApiTable(newTable, principalPrivileges)); + alterTable(databaseName, tableName, toMetastoreApiTable(newTable, principalPrivileges), environmentContext); } @Override @@ -292,7 +292,7 @@ public void setTableOwner(String databaseName, String tableName, HivePrincipal p .setOwner(Optional.of(principal.getName())) .build(); - delegate.alterTable(databaseName, tableName, toMetastoreApiTable(newTable)); + delegate.alterTable(databaseName, tableName, toMetastoreApiTable(newTable), ImmutableMap.of()); } @Override @@ -360,7 +360,12 @@ public void dropColumn(String databaseName, String tableName, String columnName) private void alterTable(String databaseName, String tableName, io.trino.hive.thrift.metastore.Table table) { - delegate.alterTable(databaseName, tableName, table); + delegate.alterTable(databaseName, tableName, table, ImmutableMap.of()); + } + + private void alterTable(String databaseName, String tableName, io.trino.hive.thrift.metastore.Table table, Map context) + { + delegate.alterTable(databaseName, tableName, table, context); } @Override diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftHiveMetastore.java index 41fdbffd42ad..5bacf3ef45ce 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftHiveMetastore.java @@ -434,7 +434,7 @@ public void updateTableStatistics(String databaseName, String tableName, Optiona if (acidWriteId.isPresent()) { modifiedTable.setWriteId(acidWriteId.getAsLong()); } - alterTable(databaseName, tableName, modifiedTable); + alterTable(databaseName, tableName, modifiedTable, ImmutableMap.of()); io.trino.metastore.Table table = fromMetastoreApiTable(modifiedTable); List metastoreColumnStatistics = updatedStatistics.columnStatistics().entrySet().stream() @@ -962,7 +962,7 @@ private static boolean isManagedTable(Table table) } @Override - public void alterTable(String databaseName, String tableName, Table table) + public void alterTable(String databaseName, String tableName, Table table, Map environmentContext) { if (!Objects.equals(databaseName, table.getDbName())) { validateObjectName(table.getDbName()); @@ -980,7 +980,10 @@ public void alterTable(String databaseName, String tableName, Table table) // This prevents Hive 3.x from collecting basic table stats at table creation time. // These stats are not useful by themselves and can take a very long time to collect when creating an // external table over a large data set. - context.setProperties(ImmutableMap.of("DO_NOT_UPDATE_STATS", "true")); + context.setProperties(ImmutableMap.builder() + .put("DO_NOT_UPDATE_STATS", "true") + .putAll(environmentContext) + .buildOrThrow()); client.alterTableWithEnvironmentContext(databaseName, tableName, table, context); } return null; diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastore.java index 6f9cf7a04869..9760ecd9cf63 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastore.java @@ -57,7 +57,7 @@ public sealed interface ThriftMetastore void dropTable(String databaseName, String tableName, boolean deleteData); - void alterTable(String databaseName, String tableName, Table table); + void alterTable(String databaseName, String tableName, Table table, Map environmentContext); void alterTransactionalTable(Table table, long transactionId, long writeId); diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveMetadataListing.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveMetadataListing.java index af734b91ec22..1799cf8b47e5 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveMetadataListing.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveMetadataListing.java @@ -340,7 +340,7 @@ public void dropTable(String databaseName, String tableName, boolean deleteData) } @Override - public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges) + public void replaceTable(String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Map environmentContext) { throw new UnsupportedOperationException(); } diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/file/FileMetastoreTableOperations.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/file/FileMetastoreTableOperations.java index 751372cbcfc6..5c784badb4dd 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/file/FileMetastoreTableOperations.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/file/FileMetastoreTableOperations.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.iceberg.catalog.file; +import com.google.common.collect.ImmutableMap; import io.trino.annotation.NotThreadSafe; import io.trino.metastore.PrincipalPrivileges; import io.trino.metastore.Table; @@ -87,7 +88,7 @@ private void commitTableUpdate(Table table, TableMetadata metadata, BiFunction tableUpdateFunction) { String newMetadataLocation = writeNewMetadata(metadata, version.orElseThrow() + 1); - long lockId = thriftMetastore.acquireTableExclusiveLock( - new AcidTransactionOwner(session.getUser()), - session.getQueryId(), - table.getDatabaseName(), - table.getTableName()); + + boolean lockingEnabled = parseBoolean(table.getParameters().getOrDefault(HIVE_LOCK_ENABLED, Boolean.toString(this.lockingEnabled))); + HiveLock hiveLock = lockingEnabled ? new ThriftMetastoreLock(table) : new NoLock(); + hiveLock.acquire(); + try { Table currentTable = fromMetastoreApiTable(thriftMetastore.getTable(database, table.getTableName()) .orElseThrow(() -> new TableNotFoundException(getSchemaTableName()))); @@ -105,7 +112,7 @@ private void commitTableUpdate(Table table, TableMetadata metadata, BiFunction environmentContext(String metadataLocation) + { + if (metadataLocation == null) { + return ImmutableMap.of(); + } + return ImmutableMap.builder() + .put("expected_parameter_key", "metadata_location") + .put("expected_parameter_value", metadataLocation) + .buildOrThrow(); + } + + private class ThriftMetastoreLock + implements HiveLock + { + private long lockId; + + private final Table table; + + public ThriftMetastoreLock(Table table) + { + this.table = requireNonNull(table, "table is null"); + } + + @Override + public void acquire() + { + lockId = thriftMetastore.acquireTableExclusiveLock( + new AcidTransactionOwner(session.getUser()), + session.getQueryId(), + table.getDatabaseName(), + table.getTableName()); + } + + @Override + public void release() + { try { thriftMetastore.releaseTableLock(lockId); } @@ -125,7 +174,23 @@ private void commitTableUpdate(Table table, TableMetadata metadata, BiFunction properties = ImmutableMap.builder() + .put("iceberg.hive-catalog.locking-enabled", "false") + .buildOrThrow(); + + IcebergHiveCatalogConfig expected = new IcebergHiveCatalogConfig() + .setLockingEnabled(false); + + assertFullMapping(properties, expected); + } +} diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestIcebergHiveCatalogWithoutLock.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestIcebergHiveCatalogWithoutLock.java new file mode 100644 index 000000000000..61743cf39608 --- /dev/null +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestIcebergHiveCatalogWithoutLock.java @@ -0,0 +1,82 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.iceberg.catalog.hms; + +import com.google.common.collect.ImmutableMap; +import io.trino.plugin.hive.containers.Hive4MinioDataLake; +import io.trino.plugin.iceberg.IcebergQueryRunner; +import io.trino.plugin.iceberg.SchemaInitializer; +import io.trino.testing.AbstractTestQueryFramework; +import io.trino.testing.QueryRunner; +import io.trino.testing.sql.TestTable; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +import java.util.List; +import java.util.Map; + +import static io.trino.testing.TestingNames.randomNameSuffix; +import static io.trino.testing.containers.Minio.MINIO_ACCESS_KEY; +import static io.trino.testing.containers.Minio.MINIO_REGION; +import static io.trino.testing.containers.Minio.MINIO_SECRET_KEY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; + +@TestInstance(PER_CLASS) +final class TestIcebergHiveCatalogWithoutLock + extends AbstractTestQueryFramework +{ + @Override + protected QueryRunner createQueryRunner() + throws Exception + { + String bucketName = "test-bucket" + randomNameSuffix(); + Hive4MinioDataLake hiveMinioDataLake = closeAfterClass(new Hive4MinioDataLake(bucketName)); + hiveMinioDataLake.start(); + + return IcebergQueryRunner.builder() + .setIcebergProperties( + ImmutableMap.builder() + .put("iceberg.catalog.type", "HIVE_METASTORE") + .put("hive.metastore.uri", hiveMinioDataLake.getHiveMetastoreEndpoint().toString()) + .put("iceberg.hive-catalog.locking-enabled", "false") + .put("fs.native-s3.enabled", "true") + .put("s3.aws-access-key", MINIO_ACCESS_KEY) + .put("s3.aws-secret-key", MINIO_SECRET_KEY) + .put("s3.region", MINIO_REGION) + .put("s3.endpoint", hiveMinioDataLake.getMinio().getMinioAddress()) + .put("s3.path-style-access", "true") + .buildOrThrow()) + .setSchemaInitializer( + SchemaInitializer.builder() + .withSchemaName("tpch") + .withSchemaProperties(Map.of("location", "'s3://%s/tpch'".formatted(bucketName))) + .build()) + .build(); + } + + @Test + void testCommitWithoutLock() + { + try (TestTable table = newTrinoTable("test_lock", "(x int)", List.of("1", "2", "3"))) { + assertUpdate("DELETE FROM " + table.getName() + " WHERE x = 1", 1); + assertThat(query("SELECT * FROM " + table.getName())) + .matches("VALUES 2, 3"); + + assertUpdate("DELETE FROM " + table.getName() + " WHERE x = 2", 1); + assertThat(query("SELECT * FROM " + table.getName())) + .matches("VALUES 3"); + } + } +} diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java index b76fc39f16e0..fdb8c3944273 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java @@ -145,21 +145,24 @@ protected TrinoCatalog createTrinoCatalog(boolean useUniqueTableLocations) new TrinoViewHiveMetastore(metastore, false, "trino-version", "Test"), fileSystemFactory, new TestingTypeManager(), - new HiveMetastoreTableOperationsProvider(fileSystemFactory, new ThriftMetastoreFactory() - { - @Override - public boolean isImpersonationEnabled() - { - verify(new ThriftMetastoreConfig().isImpersonationEnabled(), "This test wants to test the default behavior and assumes it's off"); - return false; - } + new HiveMetastoreTableOperationsProvider( + fileSystemFactory, + new ThriftMetastoreFactory() + { + @Override + public boolean isImpersonationEnabled() + { + verify(new ThriftMetastoreConfig().isImpersonationEnabled(), "This test wants to test the default behavior and assumes it's off"); + return false; + } - @Override - public ThriftMetastore createMetastore(Optional identity) - { - return thriftMetastore; - } - }), + @Override + public ThriftMetastore createMetastore(Optional identity) + { + return thriftMetastore; + } + }, + new IcebergHiveCatalogConfig()), useUniqueTableLocations, false, false, @@ -270,7 +273,8 @@ private Optional createTableWithTableType(TrinoCatalog catalog, Table.builder(metastoreTable) .setParameter(TABLE_TYPE_PROP, tableType) .build(), - NO_PRIVILEGES); + NO_PRIVILEGES, + ImmutableMap.of()); closer.register(() -> metastore.dropTable(namespace, tableName, true)); return Optional.of(lowerCaseTableTypeTable); }