Remove HiveConfig from Iceberg module

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeModule.java

@@ -14,6 +14,7 @@
 package io.trino.plugin.deltalake;
 
 import com.google.inject.Binder;
+import com.google.inject.Key;
 import com.google.inject.Provider;
 import com.google.inject.Provides;
 import com.google.inject.Scopes;
@@ -55,6 +56,7 @@
 import io.trino.plugin.hive.metastore.HiveMetastore;
 import io.trino.plugin.hive.metastore.MetastoreConfig;
 import io.trino.plugin.hive.metastore.SemiTransactionalHiveMetastore;
+import io.trino.plugin.hive.metastore.thrift.TranslateHiveViews;
 import io.trino.plugin.hive.parquet.ParquetReaderConfig;
 import io.trino.plugin.hive.parquet.ParquetWriterConfig;
 import io.trino.plugin.hive.procedure.OptimizeTableProcedure;
@@ -92,6 +94,7 @@ public void setup(Binder binder)
         configBinder(binder).bindConfig(DeltaLakeConfig.class);
         configBinder(binder).bindConfig(HiveConfig.class);
         binder.bind(MetastoreConfig.class).toInstance(new MetastoreConfig()); // currently not configurable
+        binder.bind(Key.get(boolean.class, TranslateHiveViews.class)).toInstance(false);
         configBinder(binder).bindConfig(ParquetReaderConfig.class);
         configBinder(binder).bindConfig(ParquetWriterConfig.class);
 

plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveConfig.java

@@ -63,6 +63,7 @@
 public class HiveConfig
 {
     private static final Splitter SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings();
+    public static final String HIVE_VIEWS_ENABLED = "hive.hive-views.enabled";
 
     private boolean singleStatementWritesOnly;
 
@@ -771,7 +772,7 @@ public boolean isTranslateHiveViews()
     }
 
     @LegacyConfig({"hive.views-execution.enabled", "hive.translate-hive-views"})
-    @Config("hive.hive-views.enabled")
+    @Config(HIVE_VIEWS_ENABLED)
     @ConfigDescription("Experimental: Allow translation of Hive views into Trino views")
     public HiveConfig setTranslateHiveViews(boolean translateHiveViews)
     {

plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveModule.java

@@ -23,6 +23,7 @@
 import io.trino.plugin.base.CatalogName;
 import io.trino.plugin.hive.fs.CachingDirectoryLister;
 import io.trino.plugin.hive.metastore.MetastoreConfig;
+import io.trino.plugin.hive.metastore.thrift.TranslateHiveViews;
 import io.trino.plugin.hive.orc.OrcFileWriterFactory;
 import io.trino.plugin.hive.orc.OrcPageSourceFactory;
 import io.trino.plugin.hive.orc.OrcReaderConfig;
@@ -139,4 +140,12 @@ public ScheduledExecutorService createHiveTransactionHeartbeatExecutor(CatalogNa
                 hiveConfig.getHiveTransactionHeartbeatThreads(),
                 daemonThreadsNamed("hive-heartbeat-" + catalogName + "-%s"));
     }
+
+    @TranslateHiveViews
+    @Singleton
+    @Provides
+    public boolean translateHiveViews(HiveConfig hiveConfig)
+    {
+        return hiveConfig.isTranslateHiveViews();
+    }
 }

plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftHiveMetastore.java

@@ -25,7 +25,6 @@
 import io.trino.plugin.hive.HdfsEnvironment;
 import io.trino.plugin.hive.HdfsEnvironment.HdfsContext;
 import io.trino.plugin.hive.HiveBasicStatistics;
-import io.trino.plugin.hive.HiveConfig;
 import io.trino.plugin.hive.HivePartition;
 import io.trino.plugin.hive.HiveType;
 import io.trino.plugin.hive.PartitionNotFoundException;
@@ -197,25 +196,25 @@ public class ThriftHiveMetastore
     @Inject
     public ThriftHiveMetastore(
             MetastoreLocator metastoreLocator,
-            HiveConfig hiveConfig,
             MetastoreConfig metastoreConfig,
+            @TranslateHiveViews boolean translateHiveViews,
             ThriftMetastoreConfig thriftConfig,
             ThriftMetastoreAuthenticationConfig authenticationConfig,
             HdfsEnvironment hdfsEnvironment)
     {
         this(
                 metastoreLocator,
-                hiveConfig,
                 metastoreConfig,
+                translateHiveViews,
                 thriftConfig,
                 hdfsEnvironment,
                 authenticationConfig.getAuthenticationType() != ThriftMetastoreAuthenticationType.NONE);
     }
 
     public ThriftHiveMetastore(
             MetastoreLocator metastoreLocator,
-            HiveConfig hiveConfig,
             MetastoreConfig metastoreConfig,
+            boolean translateHiveViews,
             ThriftMetastoreConfig thriftConfig,
             HdfsEnvironment hdfsEnvironment,
             boolean authenticationEnabled)
@@ -230,8 +229,7 @@ public ThriftHiveMetastore(
         this.maxRetries = thriftConfig.getMaxRetries();
         this.impersonationEnabled = thriftConfig.isImpersonationEnabled();
         this.deleteFilesOnDrop = thriftConfig.isDeleteFilesOnDrop();
-        requireNonNull(hiveConfig, "hiveConfig is null");
-        this.translateHiveViews = hiveConfig.isTranslateHiveViews();
+        this.translateHiveViews = translateHiveViews;
         requireNonNull(metastoreConfig, "metastoreConfig is null");
         checkArgument(!metastoreConfig.isHideDeltaLakeTables(), "Hiding Delta Lake tables is not supported"); // TODO
         this.maxWaitForLock = thriftConfig.getMaxWaitForTransactionLock();

plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/TranslateHiveViews.java

@@ -0,0 +1,31 @@
+/*
+ * 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.hive.metastore.thrift;
+
+import javax.inject.Qualifier;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+@Retention(RUNTIME)
+@Target({FIELD, PARAMETER, METHOD})
+@Qualifier
+public @interface TranslateHiveViews
+{
+}

plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java

@@ -791,8 +791,8 @@ protected final void setup(String host, int port, String databaseName, String ti
         HiveMetastore metastore = cachingHiveMetastore(
                 new BridgingHiveMetastore(new ThriftHiveMetastore(
                         metastoreLocator,
-                        hiveConfig,
                         new MetastoreConfig(),
+                        new HiveConfig().isTranslateHiveViews(),
                         new ThriftMetastoreConfig(),
                         hdfsEnvironment,
                         false),

plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHiveFileSystem.java

@@ -190,8 +190,8 @@ protected void setup(String host, int port, String databaseName, boolean s3Selec
                 new BridgingHiveMetastore(
                         new ThriftHiveMetastore(
                                 metastoreLocator,
-                                new HiveConfig(),
                                 metastoreConfig,
+                                new HiveConfig().isTranslateHiveViews(),
                                 new ThriftMetastoreConfig(),
                                 hdfsEnvironment,
                                 false),

plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseTestHiveOnDataLake.java

@@ -80,8 +80,8 @@ protected QueryRunner createQueryRunner()
                 new TestingMetastoreLocator(
                         Optional.empty(),
                         this.dockerizedS3DataLake.getHiveHadoop().getHiveMetastoreEndpoint()),
-                new HiveConfig(),
                 new MetastoreConfig(),
+                new HiveConfig().isTranslateHiveViews(),
                 new ThriftMetastoreConfig(),
                 new HdfsEnvironment(new HiveHdfsConfiguration(
                         new HdfsConfigurationInitializer(

plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/cache/TestCachingHiveMetastore.java

@@ -154,7 +154,7 @@ private ThriftHiveMetastore createThriftHiveMetastore()
     private static ThriftHiveMetastore createThriftHiveMetastore(ThriftMetastoreClient client)
     {
         MetastoreLocator metastoreLocator = new MockMetastoreLocator(client);
-        return new ThriftHiveMetastore(metastoreLocator, new HiveConfig(), new MetastoreConfig(), new ThriftMetastoreConfig(), HDFS_ENVIRONMENT, false);
+        return new ThriftHiveMetastore(metastoreLocator, new MetastoreConfig(), new HiveConfig().isTranslateHiveViews(), new ThriftMetastoreConfig(), HDFS_ENVIRONMENT, false);
     }
 
     @Test

plugin/trino-hive/src/test/java/io/trino/plugin/hive/s3/S3HiveQueryRunner.java

@@ -156,8 +156,8 @@ public DistributedQueryRunner build()
                                             Optional.empty(),
                                             hiveMetastoreEndpoint,
                                             metastoreTimeout),
-                                    new HiveConfig(),
                                     new MetastoreConfig(),
+                                    new HiveConfig().isTranslateHiveViews(),
                                     new ThriftMetastoreConfig(),
                                     new HdfsEnvironment(new HiveHdfsConfiguration(
                                             new HdfsConfigurationInitializer(

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergConfig.java

@@ -15,6 +15,8 @@
 
 import io.airlift.configuration.Config;
 import io.airlift.configuration.ConfigDescription;
+import io.airlift.configuration.LegacyConfig;
+import io.airlift.units.DataSize;
 import io.airlift.units.Duration;
 import io.trino.plugin.hive.HiveCompressionCodec;
 
@@ -24,6 +26,7 @@
 
 import java.util.Optional;
 
+import static io.airlift.units.DataSize.Unit.GIGABYTE;
 import static io.trino.plugin.hive.HiveCompressionCodec.ZSTD;
 import static io.trino.plugin.iceberg.CatalogType.HIVE_METASTORE;
 import static io.trino.plugin.iceberg.IcebergFileFormat.ORC;
@@ -50,6 +53,11 @@ public class IcebergConfig
     private int formatVersion = FORMAT_VERSION_SUPPORT_MAX;
     private Duration expireSnapshotsMinRetention = new Duration(7, DAYS);
     private Duration removeOrphanFilesMinRetention = new Duration(7, DAYS);
+    private DataSize targetMaxFileSize = DataSize.of(1, GIGABYTE);
+    // This is meant to protect users who are misusing schema locations (by
+    // putting schemas in locations with extraneous files), so default to false
+    // to avoid deleting those files if Trino is unable to check.
+    private boolean deleteSchemaLocationsFallback;
 
     public CatalogType getCatalogType()
     {
@@ -235,4 +243,32 @@ public IcebergConfig setRemoveOrphanFilesMinRetention(Duration removeOrphanFiles
         this.removeOrphanFilesMinRetention = removeOrphanFilesMinRetention;
         return this;
     }
+
+    public DataSize getTargetMaxFileSize()
+    {
+        return targetMaxFileSize;
+    }
+
+    @LegacyConfig("hive.target-max-file-size")
+    @Config("iceberg.target-max-file-size")
+    @ConfigDescription("Target maximum size of written files; the actual size may be larger")
+    public IcebergConfig setTargetMaxFileSize(DataSize targetMaxFileSize)
+    {
+        this.targetMaxFileSize = targetMaxFileSize;
+        return this;
+    }
+
+    public boolean isDeleteSchemaLocationsFallback()
+    {
+        return this.deleteSchemaLocationsFallback;
+    }
+
+    @LegacyConfig("hive.delete-schema-locations-fallback")
+    @Config("iceberg.delete-schema-locations-fallback")
+    @ConfigDescription("Whether schema locations should be deleted when Trino can't determine whether they contain external files.")
+    public IcebergConfig setDeleteSchemaLocationsFallback(boolean deleteSchemaLocationsFallback)
+    {
+        this.deleteSchemaLocationsFallback = deleteSchemaLocationsFallback;
+        return this;
+    }
 }

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergModule.java

@@ -19,7 +19,6 @@
 import com.google.inject.multibindings.Multibinder;
 import io.trino.plugin.base.session.SessionPropertiesProvider;
 import io.trino.plugin.hive.FileFormatDataSourceStats;
-import io.trino.plugin.hive.HiveConfig;
 import io.trino.plugin.hive.metastore.MetastoreConfig;
 import io.trino.plugin.hive.orc.OrcReaderConfig;
 import io.trino.plugin.hive.orc.OrcWriterConfig;
@@ -48,8 +47,6 @@ public class IcebergModule
     public void configure(Binder binder)
     {
         binder.bind(IcebergTransactionManager.class).in(Scopes.SINGLETON);
-
-        configBinder(binder).bindConfig(HiveConfig.class);
         configBinder(binder).bindConfig(IcebergConfig.class);
         configBinder(binder).bindConfig(MetastoreConfig.class);
 

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSessionProperties.java

@@ -19,7 +19,6 @@
 import io.trino.orc.OrcWriteValidation.OrcWriteValidationMode;
 import io.trino.plugin.base.session.SessionPropertiesProvider;
 import io.trino.plugin.hive.HiveCompressionCodec;
-import io.trino.plugin.hive.HiveConfig;
 import io.trino.plugin.hive.orc.OrcReaderConfig;
 import io.trino.plugin.hive.orc.OrcWriterConfig;
 import io.trino.plugin.hive.parquet.ParquetReaderConfig;
@@ -85,8 +84,7 @@ public IcebergSessionProperties(
             OrcReaderConfig orcReaderConfig,
             OrcWriterConfig orcWriterConfig,
             ParquetReaderConfig parquetReaderConfig,
-            ParquetWriterConfig parquetWriterConfig,
-            HiveConfig hiveConfig)
+            ParquetWriterConfig parquetWriterConfig)
     {
         sessionProperties = ImmutableList.<PropertyMetadata<?>>builder()
                 .add(enumProperty(
@@ -222,7 +220,7 @@ public IcebergSessionProperties(
                 .add(dataSizeProperty(
                         TARGET_MAX_FILE_SIZE,
                         "Target maximum size of written files; the actual size may be larger",
-                        hiveConfig.getTargetMaxFileSize(),
+                        icebergConfig.getTargetMaxFileSize(),
                         false))
                 .add(stringProperty(
                         HIVE_CATALOG_NAME,

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/IcebergHiveMetastoreCatalogModule.java

@@ -14,10 +14,12 @@
 package io.trino.plugin.iceberg.catalog.hms;
 
 import com.google.inject.Binder;
+import com.google.inject.Key;
 import com.google.inject.Scopes;
 import io.airlift.configuration.AbstractConfigurationAwareModule;
 import io.trino.plugin.hive.metastore.DecoratedHiveMetastoreModule;
 import io.trino.plugin.hive.metastore.thrift.ThriftMetastoreModule;
+import io.trino.plugin.hive.metastore.thrift.TranslateHiveViews;
 import io.trino.plugin.iceberg.catalog.IcebergCatalogModule.MetastoreValidator;
 import io.trino.plugin.iceberg.catalog.IcebergTableOperationsProvider;
 import io.trino.plugin.iceberg.catalog.TrinoCatalogFactory;
@@ -32,6 +34,7 @@ protected void setup(Binder binder)
         binder.bind(IcebergTableOperationsProvider.class).to(HiveMetastoreTableOperationsProvider.class).in(Scopes.SINGLETON);
         binder.bind(TrinoCatalogFactory.class).to(TrinoHiveCatalogFactory.class).in(Scopes.SINGLETON);
         binder.bind(MetastoreValidator.class).asEagerSingleton();
+        binder.bind(Key.get(boolean.class, TranslateHiveViews.class)).toInstance(false);
         install(new DecoratedHiveMetastoreModule());
     }
 }

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalogFactory.java

@@ -15,7 +15,6 @@
 
 import io.trino.plugin.base.CatalogName;
 import io.trino.plugin.hive.HdfsEnvironment;
-import io.trino.plugin.hive.HiveConfig;
 import io.trino.plugin.hive.NodeVersion;
 import io.trino.plugin.hive.metastore.HiveMetastoreFactory;
 import io.trino.plugin.iceberg.IcebergConfig;
@@ -56,8 +55,7 @@ public TrinoHiveCatalogFactory(
             TypeManager typeManager,
             IcebergTableOperationsProvider tableOperationsProvider,
             NodeVersion nodeVersion,
-            IcebergSecurityConfig securityConfig,
-            HiveConfig hiveConfig)
+            IcebergSecurityConfig securityConfig)
     {
         this.catalogName = requireNonNull(catalogName, "catalogName is null");
         this.metastoreFactory = requireNonNull(metastoreFactory, "metastoreFactory is null");
@@ -69,8 +67,7 @@ public TrinoHiveCatalogFactory(
         this.isUniqueTableLocation = config.isUniqueTableLocation();
         requireNonNull(securityConfig, "securityConfig is null");
         this.isUsingSystemSecurity = securityConfig.getSecuritySystem() == SYSTEM;
-        requireNonNull(hiveConfig, "hiveConfig is null");
-        this.deleteSchemaLocationsFallback = hiveConfig.isDeleteSchemaLocationsFallback();
+        this.deleteSchemaLocationsFallback = config.isDeleteSchemaLocationsFallback();
     }
 
     @Override

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergConfig.java

@@ -14,6 +14,7 @@
 package io.trino.plugin.iceberg;
 
 import com.google.common.collect.ImmutableMap;
+import io.airlift.units.DataSize;
 import io.airlift.units.Duration;
 import io.trino.plugin.hive.HiveCompressionCodec;
 import org.testng.annotations.Test;
@@ -23,6 +24,8 @@
 import static io.airlift.configuration.testing.ConfigAssertions.assertFullMapping;
 import static io.airlift.configuration.testing.ConfigAssertions.assertRecordedDefaults;
 import static io.airlift.configuration.testing.ConfigAssertions.recordDefaults;
+import static io.airlift.units.DataSize.Unit.GIGABYTE;
+import static io.airlift.units.DataSize.Unit.MEGABYTE;
 import static io.trino.plugin.hive.HiveCompressionCodec.ZSTD;
 import static io.trino.plugin.iceberg.CatalogType.GLUE;
 import static io.trino.plugin.iceberg.CatalogType.HIVE_METASTORE;
@@ -50,7 +53,9 @@ public void testDefaults()
                 .setHiveCatalogName(null)
                 .setFormatVersion(2)
                 .setExpireSnapshotsMinRetention(new Duration(7, DAYS))
-                .setRemoveOrphanFilesMinRetention(new Duration(7, DAYS)));
+                .setRemoveOrphanFilesMinRetention(new Duration(7, DAYS))
+                .setDeleteSchemaLocationsFallback(false)
+                .setTargetMaxFileSize(DataSize.of(1, GIGABYTE)));
     }
 
     @Test
@@ -70,6 +75,8 @@ public void testExplicitPropertyMappings()
                 .put("iceberg.format-version", "1")
                 .put("iceberg.expire_snapshots.min-retention", "13h")
                 .put("iceberg.remove_orphan_files.min-retention", "14h")
+                .put("iceberg.delete-schema-locations-fallback", "true")
+                .put("iceberg.target-max-file-size", "1MB")
                 .buildOrThrow();
 
         IcebergConfig expected = new IcebergConfig()
@@ -85,7 +92,9 @@ public void testExplicitPropertyMappings()
                 .setHiveCatalogName("hive")
                 .setFormatVersion(1)
                 .setExpireSnapshotsMinRetention(new Duration(13, HOURS))
-                .setRemoveOrphanFilesMinRetention(new Duration(14, HOURS));
+                .setRemoveOrphanFilesMinRetention(new Duration(14, HOURS))
+                .setDeleteSchemaLocationsFallback(true)
+                .setTargetMaxFileSize(DataSize.of(1, MEGABYTE));
 
         assertFullMapping(properties, expected);
     }