diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergConfig.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergConfig.java index ba95f5f5625..95f99bbdbed 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergConfig.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergConfig.java @@ -45,6 +45,7 @@ import static java.util.Locale.ENGLISH; import static java.util.concurrent.TimeUnit.DAYS; import static java.util.concurrent.TimeUnit.SECONDS; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; @DefunctConfig({ "iceberg.allow-legacy-snapshot-syntax", @@ -95,6 +96,7 @@ public class IcebergConfig private int planningThreads = Math.min(Runtime.getRuntime().availableProcessors(), 16); private int fileDeleteThreads = Runtime.getRuntime().availableProcessors() * 2; private List allowedExtraProperties = ImmutableList.of(); + private boolean defaultNewTablesGcEnabled = GC_ENABLED_DEFAULT; private boolean incrementalRefreshEnabled = true; private boolean metadataCacheEnabled = true; private boolean objectStoreLayoutEnabled; @@ -567,6 +569,19 @@ public IcebergConfig setAllowedExtraProperties(List allowedExtraProperti return this; } + public boolean isDefaultNewTablesGcEnabled() + { + return defaultNewTablesGcEnabled; + } + + @Config("iceberg.default-new-tables-gc.enabled") + @ConfigDescription("Default value for Iceberg property gc.enabled when creating new tables") + public IcebergConfig setDefaultNewTablesGcEnabled(boolean defaultNewTablesGcEnabled) + { + this.defaultNewTablesGcEnabled = defaultNewTablesGcEnabled; + return this; + } + public boolean isIncrementalRefreshEnabled() { return incrementalRefreshEnabled; diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java index 298128734ff..2d88dc93c7a 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java @@ -473,6 +473,7 @@ public class IcebergMetadata private final Executor metadataFetchingExecutor; private final ExecutorService icebergPlanningExecutor; private final ExecutorService icebergFileDeleteExecutor; + private final boolean defaultNewTablesGcEnabled; private final Map> tableStatisticsCache = new ConcurrentHashMap<>(); private Transaction transaction; @@ -491,7 +492,8 @@ public IcebergMetadata( ExecutorService icebergScanExecutor, Executor metadataFetchingExecutor, ExecutorService icebergPlanningExecutor, - ExecutorService icebergFileDeleteExecutor) + ExecutorService icebergFileDeleteExecutor, + boolean defaultNewTablesGcEnabled) { this.typeManager = requireNonNull(typeManager, "typeManager is null"); this.commitTaskCodec = requireNonNull(commitTaskCodec, "commitTaskCodec is null"); @@ -506,6 +508,7 @@ public IcebergMetadata( this.metadataFetchingExecutor = requireNonNull(metadataFetchingExecutor, "metadataFetchingExecutor is null"); this.icebergPlanningExecutor = requireNonNull(icebergPlanningExecutor, "icebergPlanningExecutor is null"); this.icebergFileDeleteExecutor = requireNonNull(icebergFileDeleteExecutor, "icebergFileDeleteExecutor is null"); + this.defaultNewTablesGcEnabled = defaultNewTablesGcEnabled; } @Override @@ -1300,7 +1303,7 @@ public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, Con tableLocation = getTableLocation(tableMetadata.getProperties()) .orElseGet(() -> catalog.defaultTableLocation(session, tableMetadata.getTable())); } - transaction = newCreateTableTransaction(catalog, tableMetadata, session, replace, tableLocation, allowedExtraProperties); + transaction = newCreateTableTransaction(catalog, tableMetadata, session, replace, tableLocation, allowedExtraProperties, defaultNewTablesGcEnabled); Location location = Location.of(transaction.table().location()); try { // S3 Tables internally assigns a unique location for each table diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataFactory.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataFactory.java index 3260068eefd..5d3dfcb9c19 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataFactory.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataFactory.java @@ -47,6 +47,7 @@ public class IcebergMetadataFactory private final Executor metadataFetchingExecutor; private final ExecutorService icebergPlanningExecutor; private final ExecutorService icebergFileDeleteExecutor; + private final boolean defaultNewTablesGcEnabled; @Inject public IcebergMetadataFactory( @@ -87,6 +88,7 @@ public IcebergMetadataFactory( } this.icebergPlanningExecutor = requireNonNull(icebergPlanningExecutor, "icebergPlanningExecutor is null"); this.icebergFileDeleteExecutor = requireNonNull(icebergFileDeleteExecutor, "icebergFileDeleteExecutor is null"); + this.defaultNewTablesGcEnabled = config.isDefaultNewTablesGcEnabled(); } public IcebergMetadata create(ConnectorIdentity identity) @@ -104,6 +106,7 @@ public IcebergMetadata create(ConnectorIdentity identity) icebergScanExecutor, metadataFetchingExecutor, icebergPlanningExecutor, - icebergFileDeleteExecutor); + icebergFileDeleteExecutor, + defaultNewTablesGcEnabled); } } diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java index 0b309f98d27..1626199dcde 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java @@ -188,6 +188,8 @@ import static org.apache.iceberg.TableProperties.DEFAULT_FILE_FORMAT; import static org.apache.iceberg.TableProperties.DEFAULT_FILE_FORMAT_DEFAULT; import static org.apache.iceberg.TableProperties.FORMAT_VERSION; +import static org.apache.iceberg.TableProperties.GC_ENABLED; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; import static org.apache.iceberg.TableProperties.OBJECT_STORE_ENABLED; import static org.apache.iceberg.TableProperties.OBJECT_STORE_ENABLED_DEFAULT; import static org.apache.iceberg.TableProperties.ORC_BLOOM_FILTER_COLUMNS; @@ -869,7 +871,7 @@ public static List viewColumnsFromSchema(TypeManager typeManager, Sc .toList(); } - public static Transaction newCreateTableTransaction(TrinoCatalog catalog, ConnectorTableMetadata tableMetadata, ConnectorSession session, boolean replace, String tableLocation, Predicate allowedExtraProperties) + public static Transaction newCreateTableTransaction(TrinoCatalog catalog, ConnectorTableMetadata tableMetadata, ConnectorSession session, boolean replace, String tableLocation, Predicate allowedExtraProperties, boolean defaultNewTablesGcEnabled) { SchemaTableName schemaTableName = tableMetadata.getTable(); Schema schema = schemaFromMetadata(tableMetadata.getColumns()); @@ -879,10 +881,10 @@ public static Transaction newCreateTableTransaction(TrinoCatalog catalog, Connec Transaction transaction; if (replace) { - transaction = catalog.newCreateOrReplaceTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, tableLocation, createTableProperties(tableMetadata, allowedExtraProperties)); + transaction = catalog.newCreateOrReplaceTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, tableLocation, createTableProperties(tableMetadata, allowedExtraProperties, defaultNewTablesGcEnabled)); } else { - transaction = catalog.newCreateTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, Optional.ofNullable(tableLocation), createTableProperties(tableMetadata, allowedExtraProperties)); + transaction = catalog.newCreateTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, Optional.ofNullable(tableLocation), createTableProperties(tableMetadata, allowedExtraProperties, defaultNewTablesGcEnabled)); } // If user doesn't set compression-codec for parquet, we need to remove write.parquet.compression-codec property, @@ -897,7 +899,7 @@ public static Transaction newCreateTableTransaction(TrinoCatalog catalog, Connec return transaction; } - public static Map createTableProperties(ConnectorTableMetadata tableMetadata, Predicate allowedExtraProperties) + public static Map createTableProperties(ConnectorTableMetadata tableMetadata, Predicate allowedExtraProperties, boolean defaultNewTablesGcEnabled) { ImmutableMap.Builder propertiesBuilder = ImmutableMap.builder(); IcebergFileFormat fileFormat = IcebergTableProperties.getFileFormat(tableMetadata.getProperties()); @@ -955,10 +957,15 @@ public static Map createTableProperties(ConnectorTableMetadata t propertiesBuilder.put(TABLE_COMMENT, tableMetadata.getComment().get()); } - Map baseProperties = propertiesBuilder.buildOrThrow(); Map extraProperties = IcebergTableProperties.getExtraProperties(tableMetadata.getProperties()).orElseGet(ImmutableMap::of); + verifyExtraProperties(propertiesBuilder.buildOrThrow().keySet(), extraProperties, allowedExtraProperties); + + // If user doesn't set gc.enabled, we need to set it to defaultNewTablesGcEnabled value + if (!extraProperties.containsKey(GC_ENABLED) && defaultNewTablesGcEnabled != GC_ENABLED_DEFAULT) { + propertiesBuilder.put(GC_ENABLED, Boolean.toString(defaultNewTablesGcEnabled)); + } - verifyExtraProperties(baseProperties.keySet(), extraProperties, allowedExtraProperties); + Map baseProperties = propertiesBuilder.buildOrThrow(); return ImmutableMap.builder() .putAll(baseProperties) diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java index 4268acc934d..56a77047aac 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java @@ -107,6 +107,7 @@ import static org.apache.iceberg.BaseMetastoreTableOperations.METADATA_LOCATION_PROP; import static org.apache.iceberg.TableMetadata.newTableMetadata; import static org.apache.iceberg.TableMetadataParser.getFileExtension; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; import static org.apache.iceberg.TableProperties.METADATA_COMPRESSION_DEFAULT; import static org.apache.iceberg.Transactions.createOrReplaceTableTransaction; import static org.apache.iceberg.Transactions.createTableTransaction; @@ -312,7 +313,7 @@ protected Location createMaterializedViewStorage( Schema schema = schemaFromMetadata(columns); PartitionSpec partitionSpec = parsePartitionFields(schema, getPartitioning(materializedViewProperties)); SortOrder sortOrder = parseSortFields(schema, getSortOrder(materializedViewProperties)); - Map properties = createTableProperties(new ConnectorTableMetadata(storageTableName, columns, materializedViewProperties, Optional.empty()), _ -> false); + Map properties = createTableProperties(new ConnectorTableMetadata(storageTableName, columns, materializedViewProperties, Optional.empty()), _ -> false, GC_ENABLED_DEFAULT); TableMetadata metadata = newTableMetadata(schema, partitionSpec, sortOrder, tableLocation, properties); @@ -350,7 +351,7 @@ protected SchemaTableName createMaterializedViewStorageTable( ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(storageTable, columns, materializedViewProperties, Optional.empty()); String tableLocation = getTableLocation(tableMetadata.getProperties()) .orElseGet(() -> defaultTableLocation(session, tableMetadata.getTable())); - Transaction transaction = IcebergUtil.newCreateTableTransaction(this, tableMetadata, session, false, tableLocation, _ -> false); + Transaction transaction = IcebergUtil.newCreateTableTransaction(this, tableMetadata, session, false, tableLocation, _ -> false, GC_ENABLED_DEFAULT); AppendFiles appendFiles = transaction.newAppend(); commit(appendFiles, session); transaction.commitTransaction(); diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java index 85d761d1062..f4d67495e39 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java @@ -160,6 +160,9 @@ import static java.util.Objects.requireNonNull; import static org.apache.iceberg.BaseMetastoreTableOperations.METADATA_LOCATION_PROP; import static org.apache.iceberg.CatalogUtil.dropTableData; +import static org.apache.iceberg.TableProperties.GC_ENABLED; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; +import static org.apache.iceberg.util.PropertyUtil.propertyAsBoolean; public class TrinoGlueCatalog extends AbstractTrinoCatalog @@ -707,7 +710,9 @@ public void dropTable(ConnectorSession session, SchemaTableName schemaTableName) // So log the exception and continue with deleting the table location LOG.warn(e, "Failed to delete table data referenced by metadata"); } - deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location()); + if (propertyAsBoolean(table.properties(), GC_ENABLED, GC_ENABLED_DEFAULT)) { + deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location()); + } invalidateTableCache(schemaTableName); } diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java index 06460d638b7..9de418de898 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java @@ -127,6 +127,9 @@ import static org.apache.iceberg.BaseMetastoreTableOperations.METADATA_LOCATION_PROP; import static org.apache.iceberg.BaseMetastoreTableOperations.TABLE_TYPE_PROP; import static org.apache.iceberg.CatalogUtil.dropTableData; +import static org.apache.iceberg.TableProperties.GC_ENABLED; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; +import static org.apache.iceberg.util.PropertyUtil.propertyAsBoolean; public class TrinoHiveCatalog extends AbstractTrinoCatalog @@ -436,7 +439,9 @@ public void dropTable(ConnectorSession session, SchemaTableName schemaTableName) // So log the exception and continue with deleting the table location log.warn(e, "Failed to delete table data referenced by metadata"); } - deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, metastoreTable.getStorage().getLocation()); + if (propertyAsBoolean(metadata.properties(), GC_ENABLED, GC_ENABLED_DEFAULT)) { + deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, metastoreTable.getStorage().getLocation()); + } invalidateTableCache(schemaTableName); } diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java index 5e05f3abbba..6318d08ff36 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java @@ -87,6 +87,9 @@ import static java.util.Locale.ENGLISH; import static java.util.Objects.requireNonNull; import static org.apache.iceberg.CatalogUtil.dropTableData; +import static org.apache.iceberg.TableProperties.GC_ENABLED; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; +import static org.apache.iceberg.util.PropertyUtil.propertyAsBoolean; import static org.apache.iceberg.view.ViewProperties.COMMENT; public class TrinoJdbcCatalog @@ -348,7 +351,9 @@ public void dropTable(ConnectorSession session, SchemaTableName schemaTableName) // So log the exception and continue with deleting the table location LOG.warn(e, "Failed to delete table data referenced by metadata"); } - deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location()); + if (propertyAsBoolean(table.properties(), GC_ENABLED, GC_ENABLED_DEFAULT)) { + deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location()); + } invalidateTableCache(schemaTableName); } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java index d4222d8343c..c2859f0d7d7 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java @@ -219,7 +219,7 @@ protected IcebergQueryRunner.Builder createQueryRunnerBuilder() .setIcebergProperties(ImmutableMap.builder() .put("iceberg.file-format", format.name()) // Only allow some extra properties. Add "sorted_by" so that we can test that the property is disallowed by the connector explicitly. - .put("iceberg.allowed-extra-properties", "extra.property.one,extra.property.two,extra.property.three,sorted_by") + .put("iceberg.allowed-extra-properties", "extra.property.one,extra.property.two,extra.property.three,sorted_by,gc.enabled") // Allows testing the sorting writer flushing to the file system with smaller tables .put("iceberg.writer-sort-buffer-size", "1MB") .buildOrThrow()) @@ -5129,6 +5129,96 @@ public void testSplitPruningForFilterOnNonPartitionColumn() } } + @Test + public void testDefaultGcEnabledTablePropertyOnCreateAndCtas() throws IOException + { + // CREATE TABLE AS SELECT + String ctasTable = "test_gc_enabled_ctas_" + randomNameSuffix(); + assertUpdate("CREATE TABLE " + ctasTable + " AS SELECT 1 x", 1); + String ctasTableLocation = getTableLocation(ctasTable); + String ctasMetadataLocation = getLatestMetadataLocation(fileSystem, ctasTableLocation); + TableMetadata ctasMetadata = TableMetadataParser.read(FILE_IO_FACTORY.create(fileSystem), ctasMetadataLocation); + assertThat(ctasMetadata.properties().getOrDefault("gc.enabled", "")).isEqualTo(""); + assertThat(ctasMetadata.properties().getOrDefault("gc.enabled", "true")).isEqualTo("true"); + + // collect data files and verify they are removed on drop + var ctasDataFiles = getAllDataFilesFromTableDirectory(ctasTable); + assertThat(ctasDataFiles).isNotEmpty(); + assertUpdate("DROP TABLE " + ctasTable); + for (String path : ctasDataFiles) { + assertThat(fileSystem.newInputFile(Location.of(path)).exists()).isFalse(); + } + + // CREATE TABLE then INSERT + String createTable = "test_gc_enabled_create_" + randomNameSuffix(); + assertUpdate("CREATE TABLE " + createTable + " (a bigint)"); + String createTableLocation = getTableLocation(createTable); + String createMetadataLocation = getLatestMetadataLocation(fileSystem, createTableLocation); + TableMetadata createMetadata = TableMetadataParser.read(FILE_IO_FACTORY.create(fileSystem), createMetadataLocation); + assertThat(createMetadata.properties().getOrDefault("gc.enabled", "")).isEqualTo(""); + assertThat(createMetadata.properties().getOrDefault("gc.enabled", "true")).isEqualTo("true"); + assertUpdate("INSERT INTO " + createTable + " VALUES 2", 1); + var createDataFiles = getAllDataFilesFromTableDirectory(createTable); + assertThat(createDataFiles).isNotEmpty(); + assertUpdate("DROP TABLE " + createTable); + for (String path : createDataFiles) { + assertThat(fileSystem.newInputFile(Location.of(path)).exists()).isFalse(); + } + } + + @Test + public void testGcDisabledDoesNotDeleteDataFilesOnDrop() throws IOException + { + String table = "test_gc_disabled_keeps_files_" + randomNameSuffix(); + // Set gc.enabled=false via extra_properties + assertUpdate("CREATE TABLE " + table + " WITH (extra_properties = MAP(ARRAY['gc.enabled'], ARRAY['false'])) AS SELECT 1 x", 1); + + String tableLocation = getTableLocation(table); + String metadataLocation = getLatestMetadataLocation(fileSystem, tableLocation); + TableMetadata metadata = TableMetadataParser.read(FILE_IO_FACTORY.create(fileSystem), metadataLocation); + assertThat(metadata.properties().getOrDefault("gc.enabled", "")).isEqualTo("false"); + + var dataFiles = getAllDataFilesFromTableDirectory(table); + assertThat(dataFiles).isNotEmpty(); + + assertUpdate("DROP TABLE " + table); + + // When gc.enabled is false, data files should not be deleted. Verify the data files still exist + for (String path : dataFiles) { + assertThat(fileSystem.newInputFile(Location.of(path)).exists()).isTrue(); + } + } + + @Test + public void testGcDisabledAfterAlterDoesNotDeleteDataFilesOnDrop() throws IOException + { + String table = "test_gc_disabled_after_alter_keeps_files_" + randomNameSuffix(); + // CTAS to create table with default gc.enabled=true + assertUpdate("CREATE TABLE " + table + " AS SELECT 1 x", 1); + + String tableLocation = getTableLocation(table); + String metadataLocation = getLatestMetadataLocation(fileSystem, tableLocation); + TableMetadata metadata = TableMetadataParser.read(FILE_IO_FACTORY.create(fileSystem), metadataLocation); + assertThat(metadata.properties().getOrDefault("gc.enabled", "true")).isEqualTo("true"); + + var dataFiles = getAllDataFilesFromTableDirectory(table); + assertThat(dataFiles).isNotEmpty(); + + // Flip gc.enabled to false via ALTER TABLE ... SET PROPERTIES extra_properties + assertUpdate("ALTER TABLE " + table + " SET PROPERTIES extra_properties = MAP(ARRAY['gc.enabled'], ARRAY['false'])"); + + // Verify the property is now false in latest metadata + String newMetadataLocation = getLatestMetadataLocation(fileSystem, tableLocation); + TableMetadata newMetadata = TableMetadataParser.read(FILE_IO_FACTORY.create(fileSystem), newMetadataLocation); + assertThat(newMetadata.properties().getOrDefault("gc.enabled", "")).isEqualTo("false"); + + // Drop the table and verify data files still exist + assertUpdate("DROP TABLE " + table); + for (String path : dataFiles) { + assertThat(fileSystem.newInputFile(Location.of(path)).exists()).isTrue(); + } + } + @Test public void testGetIcebergTableWithLegacyOrcBloomFilterProperties() throws IOException diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergConfig.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergConfig.java index f7d9d02e94b..b2236b29646 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergConfig.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergConfig.java @@ -49,6 +49,7 @@ public void testDefaults() .setFileFormat(PARQUET) .setCompressionCodec(ZSTD) .setMaxCommitRetry(null) + .setDefaultNewTablesGcEnabled(true) .setUseFileSizeFromMetadata(true) .setMaxPartitionsPerWriter(100) .setUniqueTableLocation(true) @@ -94,6 +95,7 @@ public void testExplicitPropertyMappings() .put("iceberg.file-format", "ORC") .put("iceberg.compression-codec", "NONE") .put("iceberg.max-commit-retry", "100") + .put("iceberg.default-new-tables-gc.enabled", "false") .put("iceberg.use-file-size-from-metadata", "false") .put("iceberg.max-partitions-per-writer", "222") .put("iceberg.unique-table-location", "false") @@ -135,6 +137,7 @@ public void testExplicitPropertyMappings() .setFileFormat(ORC) .setCompressionCodec(HiveCompressionOption.NONE) .setMaxCommitRetry(100) + .setDefaultNewTablesGcEnabled(false) .setUseFileSizeFromMetadata(false) .setMaxPartitionsPerWriter(222) .setUniqueTableLocation(false) diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMinioOrcConnectorTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMinioOrcConnectorTest.java index 7f6a3929026..c7b37ed3b19 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMinioOrcConnectorTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMinioOrcConnectorTest.java @@ -75,7 +75,7 @@ protected QueryRunner createQueryRunner() .put("s3.streaming.part-size", "5MB") // minimize memory usage .put("s3.max-connections", "8") // verify no leaks .put("iceberg.register-table-procedure.enabled", "true") - .put("iceberg.allowed-extra-properties", "extra.property.one,extra.property.two,extra.property.three") + .put("iceberg.allowed-extra-properties", "extra.property.one,extra.property.two,extra.property.three,gc.enabled") // Allows testing the sorting writer flushing to the file system with smaller tables .put("iceberg.writer-sort-buffer-size", "1MB") .buildOrThrow()) diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java index 212d9c28239..42598b56905 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java @@ -75,6 +75,7 @@ import static io.trino.sql.planner.TestingPlannerContext.PLANNER_CONTEXT; import static io.trino.testing.TestingNames.randomNameSuffix; import static java.util.Locale.ENGLISH; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; import static org.assertj.core.api.Assertions.assertThat; public abstract class BaseTrinoCatalogTest @@ -153,7 +154,8 @@ public void testNonLowercaseNamespace() newDirectExecutorService(), directExecutor(), newDirectExecutorService(), - newDirectExecutorService()); + newDirectExecutorService(), + GC_ENABLED_DEFAULT); assertThat(icebergMetadata.schemaExists(SESSION, namespace)).as("icebergMetadata.schemaExists(namespace)") .isFalse(); assertThat(icebergMetadata.schemaExists(SESSION, schema)).as("icebergMetadata.schemaExists(schema)") @@ -191,7 +193,8 @@ public void testSchemaWithInvalidProperties() newDirectExecutorService(), directExecutor(), newDirectExecutorService(), - newDirectExecutorService()); + newDirectExecutorService(), + GC_ENABLED_DEFAULT); assertThat(icebergMetadata.getSchemaProperties(SESSION, namespace)) .doesNotContainKey("invalid_property"); diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java index 20595375564..c7d71929b7a 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java @@ -61,6 +61,7 @@ import static io.trino.testing.TestingNames.randomNameSuffix; import static io.trino.type.InternalTypeManager.TESTING_TYPE_MANAGER; import static java.util.Locale.ENGLISH; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; import static org.assertj.core.api.Assertions.assertThat; public class TestTrinoGlueCatalog @@ -152,7 +153,8 @@ public void testNonLowercaseGlueDatabase() newDirectExecutorService(), directExecutor(), newDirectExecutorService(), - newDirectExecutorService()); + newDirectExecutorService(), + GC_ENABLED_DEFAULT); assertThat(icebergMetadata.schemaExists(SESSION, databaseName)).as("icebergMetadata.schemaExists(databaseName)") .isFalse(); assertThat(icebergMetadata.schemaExists(SESSION, trinoSchemaName)).as("icebergMetadata.schemaExists(trinoSchemaName)") diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/nessie/TestTrinoNessieCatalog.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/nessie/TestTrinoNessieCatalog.java index a9f793998ee..daa03b9fec4 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/nessie/TestTrinoNessieCatalog.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/nessie/TestTrinoNessieCatalog.java @@ -57,6 +57,7 @@ import static io.trino.testing.TestingNames.randomNameSuffix; import static java.nio.file.Files.createTempDirectory; import static java.util.Locale.ENGLISH; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.fail; @@ -210,7 +211,8 @@ public void testNonLowercaseNamespace() newDirectExecutorService(), directExecutor(), newDirectExecutorService(), - newDirectExecutorService()); + newDirectExecutorService(), + GC_ENABLED_DEFAULT); assertThat(icebergMetadata.schemaExists(SESSION, namespace)).as("icebergMetadata.schemaExists(namespace)") .isTrue(); assertThat(icebergMetadata.schemaExists(SESSION, schema)).as("icebergMetadata.schemaExists(schema)") diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestTrinoRestCatalog.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestTrinoRestCatalog.java index ea86015a3bf..ccf7404a544 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestTrinoRestCatalog.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestTrinoRestCatalog.java @@ -50,6 +50,7 @@ import static io.trino.testing.TestingNames.randomNameSuffix; import static java.util.Locale.ENGLISH; import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -140,7 +141,8 @@ public void testNonLowercaseNamespace() newDirectExecutorService(), directExecutor(), newDirectExecutorService(), - newDirectExecutorService()); + newDirectExecutorService(), + GC_ENABLED_DEFAULT); assertThat(icebergMetadata.schemaExists(SESSION, namespace)).as("icebergMetadata.schemaExists(namespace)") .isTrue(); assertThat(icebergMetadata.schemaExists(SESSION, schema)).as("icebergMetadata.schemaExists(schema)") diff --git a/plugin/trino-iceberg/src/test/java/org/apache/iceberg/snowflake/TestTrinoSnowflakeCatalog.java b/plugin/trino-iceberg/src/test/java/org/apache/iceberg/snowflake/TestTrinoSnowflakeCatalog.java index 83e492561b7..39a4fbdf4c7 100644 --- a/plugin/trino-iceberg/src/test/java/org/apache/iceberg/snowflake/TestTrinoSnowflakeCatalog.java +++ b/plugin/trino-iceberg/src/test/java/org/apache/iceberg/snowflake/TestTrinoSnowflakeCatalog.java @@ -78,6 +78,7 @@ import static io.trino.testing.TestingNames.randomNameSuffix; import static io.trino.type.InternalTypeManager.TESTING_TYPE_MANAGER; import static java.util.Locale.ENGLISH; +import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT; import static org.apache.iceberg.snowflake.TrinoIcebergSnowflakeCatalogFactory.getSnowflakeDriverProperties; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -237,7 +238,8 @@ public void testNonLowercaseNamespace() newDirectExecutorService(), directExecutor(), newDirectExecutorService(), - newDirectExecutorService()); + newDirectExecutorService(), + GC_ENABLED_DEFAULT); assertThat(icebergMetadata.schemaExists(SESSION, namespace)).as("icebergMetadata.schemaExists(namespace)") .isTrue(); assertThat(icebergMetadata.schemaExists(SESSION, schema)).as("icebergMetadata.schemaExists(schema)")