From f7a47e364aee021ff34c392614ccda5e94be6ef2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Wa=C5=9B?= Date: Thu, 17 Oct 2024 12:59:31 +0200 Subject: [PATCH] Support hive.metastore.glue.skip-archive in v1 Glue --- .../metastore/glue/v1/GlueHiveMetastoreConfig.java | 14 ++++++++++++++ .../metastore/glue/v1/GlueMetastoreModule.java | 3 +++ .../glue/v1}/SkipArchiveRequestHandler.java | 2 +- .../glue/v1/TestGlueHiveMetastoreConfig.java | 3 +++ .../catalog/glue/IcebergGlueCatalogModule.java | 11 ++++++----- .../glue/TestIcebergGlueCatalogSkipArchive.java | 2 ++ .../glue/TestingIcebergGlueCatalogModule.java | 1 + 7 files changed, 30 insertions(+), 6 deletions(-) rename plugin/{trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue => trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1}/SkipArchiveRequestHandler.java (98%) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueHiveMetastoreConfig.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueHiveMetastoreConfig.java index 619c1d98f4459..ebae052277326 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueHiveMetastoreConfig.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueHiveMetastoreConfig.java @@ -48,6 +48,7 @@ public class GlueHiveMetastoreConfig private int readStatisticsThreads = 5; private int writeStatisticsThreads = 20; private boolean assumeCanonicalPartitionKeys; + private boolean skipArchive; public Optional getGlueRegion() { @@ -317,6 +318,19 @@ public GlueHiveMetastoreConfig setAssumeCanonicalPartitionKeys(boolean assumeCan return this; } + public boolean isSkipArchive() + { + return skipArchive; + } + + @Config("hive.metastore.glue.skip-archive") + @ConfigDescription("Skip archiving an old table version when creating a new version in a commit") + public GlueHiveMetastoreConfig setSkipArchive(boolean skipArchive) + { + this.skipArchive = skipArchive; + return this; + } + @PostConstruct public void validate() { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueMetastoreModule.java index 4e3450fecb27f..92bbf47a279e0 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueMetastoreModule.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueMetastoreModule.java @@ -63,6 +63,9 @@ protected void setup(Binder binder) glueConfig.getCatalogId().ifPresent(catalogId -> requestHandlers.addBinding().toInstance(new GlueCatalogIdRequestHandler(catalogId))); glueConfig.getGlueProxyApiId().ifPresent(glueProxyApiId -> requestHandlers.addBinding() .toInstance(new ProxyApiRequestHandler(glueProxyApiId))); + if (glueConfig.isSkipArchive()) { + requestHandlers.addBinding().toInstance(new SkipArchiveRequestHandler()); + } binder.bind(AWSCredentialsProvider.class).toProvider(GlueCredentialsProvider.class).in(Scopes.SINGLETON); newOptionalBinder(binder, Key.get(new TypeLiteral>() {}, ForGlueHiveMetastore.class)) diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/SkipArchiveRequestHandler.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/SkipArchiveRequestHandler.java similarity index 98% rename from plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/SkipArchiveRequestHandler.java rename to plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/SkipArchiveRequestHandler.java index 5bfa765acaa0d..d5ba1bc434f22 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/SkipArchiveRequestHandler.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/SkipArchiveRequestHandler.java @@ -11,7 +11,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.trino.plugin.iceberg.catalog.glue; +package io.trino.plugin.hive.metastore.glue.v1; import com.amazonaws.AmazonWebServiceRequest; import com.amazonaws.handlers.RequestHandler2; diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/v1/TestGlueHiveMetastoreConfig.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/v1/TestGlueHiveMetastoreConfig.java index 4e2ca96e03162..38c71002bbe49 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/v1/TestGlueHiveMetastoreConfig.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/v1/TestGlueHiveMetastoreConfig.java @@ -46,6 +46,7 @@ public void testDefaults() .setPartitionSegments(5) .setGetPartitionThreads(20) .setAssumeCanonicalPartitionKeys(false) + .setSkipArchive(false) .setReadStatisticsThreads(5) .setWriteStatisticsThreads(20)); } @@ -72,6 +73,7 @@ public void testExplicitPropertyMapping() .put("hive.metastore.glue.partitions-segments", "10") .put("hive.metastore.glue.get-partition-threads", "42") .put("hive.metastore.glue.assume-canonical-partition-keys", "true") + .put("hive.metastore.glue.skip-archive", "true") .put("hive.metastore.glue.read-statistics-threads", "42") .put("hive.metastore.glue.write-statistics-threads", "43") .buildOrThrow(); @@ -95,6 +97,7 @@ public void testExplicitPropertyMapping() .setPartitionSegments(10) .setGetPartitionThreads(42) .setAssumeCanonicalPartitionKeys(true) + .setSkipArchive(true) .setReadStatisticsThreads(42) .setWriteStatisticsThreads(43); diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/IcebergGlueCatalogModule.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/IcebergGlueCatalogModule.java index f86128a7da765..3e7e7edfa2618 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/IcebergGlueCatalogModule.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/IcebergGlueCatalogModule.java @@ -28,6 +28,7 @@ import io.trino.plugin.hive.metastore.glue.v1.GlueCredentialsProvider; import io.trino.plugin.hive.metastore.glue.v1.GlueHiveMetastoreConfig; import io.trino.plugin.hive.metastore.glue.v1.GlueMetastoreModule; +import io.trino.plugin.hive.metastore.glue.v1.SkipArchiveRequestHandler; import io.trino.plugin.iceberg.catalog.IcebergTableOperationsProvider; import io.trino.plugin.iceberg.catalog.TrinoCatalogFactory; import io.trino.plugin.iceberg.procedure.MigrateProcedure; @@ -37,7 +38,6 @@ import static com.google.inject.multibindings.Multibinder.newSetBinder; import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder; -import static io.airlift.configuration.ConditionalModule.conditionalModule; import static io.airlift.configuration.ConfigBinder.configBinder; import static org.weakref.jmx.guice.ExportBinder.newExporter; @@ -56,10 +56,11 @@ protected void setup(Binder binder) binder.bind(TrinoCatalogFactory.class).to(TrinoGlueCatalogFactory.class).in(Scopes.SINGLETON); newExporter(binder).export(TrinoCatalogFactory.class).withGeneratedName(); - install(conditionalModule( - IcebergGlueCatalogConfig.class, - IcebergGlueCatalogConfig::isSkipArchive, - internalBinder -> newSetBinder(internalBinder, RequestHandler2.class, ForGlueHiveMetastore.class).addBinding().toInstance(new SkipArchiveRequestHandler()))); + GlueHiveMetastoreConfig glueHiveConfig = buildConfigObject(GlueHiveMetastoreConfig.class); + IcebergGlueCatalogConfig glueIcebergConfig = buildConfigObject(IcebergGlueCatalogConfig.class); + if (glueIcebergConfig.isSkipArchive() || !glueHiveConfig.isSkipArchive()) { + newSetBinder(binder, RequestHandler2.class, ForGlueHiveMetastore.class).addBinding().toInstance(new SkipArchiveRequestHandler()); + } // Required to inject HiveMetastoreFactory for migrate procedure binder.bind(Key.get(boolean.class, HideDeltaLakeTables.class)).toInstance(false); diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java index 97529860099b1..585fc330f0023 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java @@ -82,6 +82,8 @@ protected QueryRunner createQueryRunner() .setIcebergProperties( ImmutableMap.builder() .put("iceberg.catalog.type", "glue") + // this is not required, since iceberg.glue.skip-archive is true by default, but verify that Guice modules load correctly + .put("hive.metastore.glue.skip-archive", "true") .put("hive.metastore.glue.default-warehouse-dir", schemaDirectory.getAbsolutePath()) .buildOrThrow()) .setSchemaInitializer( diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestingIcebergGlueCatalogModule.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestingIcebergGlueCatalogModule.java index 765081865a677..0c5ba631e634d 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestingIcebergGlueCatalogModule.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestingIcebergGlueCatalogModule.java @@ -27,6 +27,7 @@ import io.trino.plugin.hive.metastore.glue.v1.GlueCredentialsProvider; import io.trino.plugin.hive.metastore.glue.v1.GlueHiveMetastoreConfig; import io.trino.plugin.hive.metastore.glue.v1.GlueMetastoreModule; +import io.trino.plugin.hive.metastore.glue.v1.SkipArchiveRequestHandler; import io.trino.plugin.iceberg.catalog.IcebergTableOperationsProvider; import io.trino.plugin.iceberg.catalog.TrinoCatalogFactory;