From 209ac9d91a48f0be09b06dfb82aa66c966d22d9c Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 15 Mar 2023 18:39:56 +0900 Subject: [PATCH] Restore skip archive handler in Iceberg Glue catalog ce42133e1f broke a logic to skip archive in Iceberg Glue catalog. --- .../catalog/glue/GlueClientProvider.java | 55 ------------------- .../glue/IcebergGlueCatalogModule.java | 11 ++++ .../glue/TestingIcebergGlueCatalogModule.java | 28 +++++++++- 3 files changed, 37 insertions(+), 57 deletions(-) delete mode 100644 plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/GlueClientProvider.java diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/GlueClientProvider.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/GlueClientProvider.java deleted file mode 100644 index 5e4da1d8d2d7..000000000000 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/GlueClientProvider.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * 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.glue; - -import com.amazonaws.auth.AWSCredentialsProvider; -import com.amazonaws.services.glue.AWSGlueAsync; -import io.trino.plugin.hive.metastore.glue.GlueHiveMetastoreConfig; -import io.trino.plugin.hive.metastore.glue.GlueMetastoreStats; - -import javax.inject.Inject; -import javax.inject.Provider; - -import java.util.Optional; - -import static io.trino.plugin.hive.metastore.glue.GlueClientUtil.createAsyncGlueClient; -import static java.util.Objects.requireNonNull; - -public class GlueClientProvider - implements Provider -{ - private final GlueMetastoreStats stats; - private final AWSCredentialsProvider credentialsProvider; - private final GlueHiveMetastoreConfig glueConfig; // TODO do not keep mutable config instance on a field - private final boolean skipArchive; - - @Inject - public GlueClientProvider( - GlueMetastoreStats stats, - AWSCredentialsProvider credentialsProvider, - GlueHiveMetastoreConfig glueConfig, - IcebergGlueCatalogConfig icebergGlueConfig) - { - this.stats = requireNonNull(stats, "stats is null"); - this.credentialsProvider = requireNonNull(credentialsProvider, "credentialsProvider is null"); - this.glueConfig = glueConfig; - this.skipArchive = icebergGlueConfig.isSkipArchive(); - } - - @Override - public AWSGlueAsync get() - { - return createAsyncGlueClient(glueConfig, credentialsProvider, Optional.of(new SkipArchiveRequestHandler(skipArchive)), stats.newRequestMetricsCollector()); - } -} 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 38e9b4d0edd9..1df3e42ca5cd 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 @@ -14,10 +14,13 @@ package io.trino.plugin.iceberg.catalog.glue; import com.amazonaws.auth.AWSCredentialsProvider; +import com.amazonaws.handlers.RequestHandler2; import com.amazonaws.services.glue.model.Table; import com.google.inject.Binder; import com.google.inject.Key; +import com.google.inject.Provides; import com.google.inject.Scopes; +import com.google.inject.Singleton; import com.google.inject.TypeLiteral; import com.google.inject.multibindings.Multibinder; import io.airlift.configuration.AbstractConfigurationAwareModule; @@ -62,4 +65,12 @@ protected void setup(Binder binder) Multibinder procedures = newSetBinder(binder, Procedure.class); procedures.addBinding().toProvider(MigrateProcedure.class).in(Scopes.SINGLETON); } + + @Provides + @Singleton + @ForGlueHiveMetastore + public static RequestHandler2 createRequestHandler(IcebergGlueCatalogConfig config) + { + return new SkipArchiveRequestHandler(config.isSkipArchive()); + } } 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 5c0feb5d1ff4..8a49035e683b 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 @@ -14,16 +14,27 @@ package io.trino.plugin.iceberg.catalog.glue; import com.amazonaws.auth.AWSCredentialsProvider; -import com.amazonaws.services.glue.AWSGlueAsync; +import com.amazonaws.handlers.RequestHandler2; +import com.amazonaws.services.glue.model.Table; import com.google.inject.Binder; +import com.google.inject.Key; +import com.google.inject.Provides; import com.google.inject.Scopes; +import com.google.inject.Singleton; +import com.google.inject.TypeLiteral; import io.airlift.configuration.AbstractConfigurationAwareModule; +import io.trino.plugin.hive.HideDeltaLakeTables; +import io.trino.plugin.hive.metastore.glue.ForGlueHiveMetastore; import io.trino.plugin.hive.metastore.glue.GlueCredentialsProvider; import io.trino.plugin.hive.metastore.glue.GlueHiveMetastoreConfig; +import io.trino.plugin.hive.metastore.glue.GlueMetastoreModule; import io.trino.plugin.hive.metastore.glue.GlueMetastoreStats; import io.trino.plugin.iceberg.catalog.IcebergTableOperationsProvider; import io.trino.plugin.iceberg.catalog.TrinoCatalogFactory; +import java.util.function.Predicate; + +import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder; import static io.airlift.configuration.ConfigBinder.configBinder; import static java.util.Objects.requireNonNull; import static org.weakref.jmx.guice.ExportBinder.newExporter; @@ -45,11 +56,24 @@ protected void setup(Binder binder) configBinder(binder).bindConfig(IcebergGlueCatalogConfig.class); binder.bind(GlueMetastoreStats.class).in(Scopes.SINGLETON); newExporter(binder).export(GlueMetastoreStats.class).withGeneratedName(); - binder.bind(AWSGlueAsync.class).toProvider(GlueClientProvider.class).in(Scopes.SINGLETON); binder.bind(AWSCredentialsProvider.class).toProvider(GlueCredentialsProvider.class).in(Scopes.SINGLETON); binder.bind(IcebergTableOperationsProvider.class).to(TestingGlueIcebergTableOperationsProvider.class).in(Scopes.SINGLETON); binder.bind(TrinoCatalogFactory.class).to(TrinoGlueCatalogFactory.class).in(Scopes.SINGLETON); newExporter(binder).export(TrinoCatalogFactory.class).withGeneratedName(); binder.bind(AWSGlueAsyncAdapterProvider.class).toInstance(awsGlueAsyncAdapterProvider); + + // Required to inject HiveMetastoreFactory for migrate procedure + binder.bind(Key.get(boolean.class, HideDeltaLakeTables.class)).toInstance(false); + newOptionalBinder(binder, Key.get(new TypeLiteral>() {}, ForGlueHiveMetastore.class)) + .setBinding().toInstance(table -> true); + install(new GlueMetastoreModule()); + } + + @Provides + @Singleton + @ForGlueHiveMetastore + public static RequestHandler2 createRequestHandler(IcebergGlueCatalogConfig config) + { + return new SkipArchiveRequestHandler(config.isSkipArchive()); } }