Skip to content

Commit c52b655

Browse files
sopel39Karol Sobczak
sopel39
authored and
Karol Sobczak
committed
Add iceberg.default-new-tables-gc.enabled config property
iceberg.default-new-tables-gc.enabled will set gc.enabled flag for new tables created by Trino.
1 parent d3a7fd5 commit c52b655

File tree

15 files changed

+167
-19
lines changed

15 files changed

+167
-19
lines changed

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import static java.util.Locale.ENGLISH;
4646
import static java.util.concurrent.TimeUnit.DAYS;
4747
import static java.util.concurrent.TimeUnit.SECONDS;
48+
import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT;
4849

4950
@DefunctConfig({
5051
"iceberg.allow-legacy-snapshot-syntax",
@@ -95,6 +96,7 @@ public class IcebergConfig
9596
private int planningThreads = Math.min(Runtime.getRuntime().availableProcessors(), 16);
9697
private int fileDeleteThreads = Runtime.getRuntime().availableProcessors() * 2;
9798
private List<String> allowedExtraProperties = ImmutableList.of();
99+
private boolean defaultNewTablesGcEnabled = GC_ENABLED_DEFAULT;
98100
private boolean incrementalRefreshEnabled = true;
99101
private boolean metadataCacheEnabled = true;
100102
private boolean objectStoreLayoutEnabled;
@@ -567,6 +569,19 @@ public IcebergConfig setAllowedExtraProperties(List<String> allowedExtraProperti
567569
return this;
568570
}
569571

572+
public boolean isDefaultNewTablesGcEnabled()
573+
{
574+
return defaultNewTablesGcEnabled;
575+
}
576+
577+
@Config("iceberg.default-new-tables-gc.enabled")
578+
@ConfigDescription("Default value for Iceberg property gc.enabled when creating new tables")
579+
public IcebergConfig setDefaultNewTablesGcEnabled(boolean defaultNewTablesGcEnabled)
580+
{
581+
this.defaultNewTablesGcEnabled = defaultNewTablesGcEnabled;
582+
return this;
583+
}
584+
570585
public boolean isIncrementalRefreshEnabled()
571586
{
572587
return incrementalRefreshEnabled;

‎plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,7 @@ public class IcebergMetadata
473473
private final Executor metadataFetchingExecutor;
474474
private final ExecutorService icebergPlanningExecutor;
475475
private final ExecutorService icebergFileDeleteExecutor;
476+
private final boolean defaultNewTablesGcEnabled;
476477
private final Map<IcebergTableHandle, AtomicReference<TableStatistics>> tableStatisticsCache = new ConcurrentHashMap<>();
477478

478479
private Transaction transaction;
@@ -491,7 +492,8 @@ public IcebergMetadata(
491492
ExecutorService icebergScanExecutor,
492493
Executor metadataFetchingExecutor,
493494
ExecutorService icebergPlanningExecutor,
494-
ExecutorService icebergFileDeleteExecutor)
495+
ExecutorService icebergFileDeleteExecutor,
496+
boolean defaultNewTablesGcEnabled)
495497
{
496498
this.typeManager = requireNonNull(typeManager, "typeManager is null");
497499
this.commitTaskCodec = requireNonNull(commitTaskCodec, "commitTaskCodec is null");
@@ -506,6 +508,7 @@ public IcebergMetadata(
506508
this.metadataFetchingExecutor = requireNonNull(metadataFetchingExecutor, "metadataFetchingExecutor is null");
507509
this.icebergPlanningExecutor = requireNonNull(icebergPlanningExecutor, "icebergPlanningExecutor is null");
508510
this.icebergFileDeleteExecutor = requireNonNull(icebergFileDeleteExecutor, "icebergFileDeleteExecutor is null");
511+
this.defaultNewTablesGcEnabled = defaultNewTablesGcEnabled;
509512
}
510513

511514
@Override
@@ -1300,7 +1303,7 @@ public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, Con
13001303
tableLocation = getTableLocation(tableMetadata.getProperties())
13011304
.orElseGet(() -> catalog.defaultTableLocation(session, tableMetadata.getTable()));
13021305
}
1303-
transaction = newCreateTableTransaction(catalog, tableMetadata, session, replace, tableLocation, allowedExtraProperties);
1306+
transaction = newCreateTableTransaction(catalog, tableMetadata, session, replace, tableLocation, allowedExtraProperties, defaultNewTablesGcEnabled);
13041307
Location location = Location.of(transaction.table().location());
13051308
try {
13061309
// S3 Tables internally assigns a unique location for each table

‎plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataFactory.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public class IcebergMetadataFactory
4747
private final Executor metadataFetchingExecutor;
4848
private final ExecutorService icebergPlanningExecutor;
4949
private final ExecutorService icebergFileDeleteExecutor;
50+
private final boolean defaultNewTablesGcEnabled;
5051

5152
@Inject
5253
public IcebergMetadataFactory(
@@ -87,6 +88,7 @@ public IcebergMetadataFactory(
8788
}
8889
this.icebergPlanningExecutor = requireNonNull(icebergPlanningExecutor, "icebergPlanningExecutor is null");
8990
this.icebergFileDeleteExecutor = requireNonNull(icebergFileDeleteExecutor, "icebergFileDeleteExecutor is null");
91+
this.defaultNewTablesGcEnabled = config.isDefaultNewTablesGcEnabled();
9092
}
9193

9294
public IcebergMetadata create(ConnectorIdentity identity)
@@ -104,6 +106,7 @@ public IcebergMetadata create(ConnectorIdentity identity)
104106
icebergScanExecutor,
105107
metadataFetchingExecutor,
106108
icebergPlanningExecutor,
107-
icebergFileDeleteExecutor);
109+
icebergFileDeleteExecutor,
110+
defaultNewTablesGcEnabled);
108111
}
109112
}

‎plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,8 @@
188188
import static org.apache.iceberg.TableProperties.DEFAULT_FILE_FORMAT;
189189
import static org.apache.iceberg.TableProperties.DEFAULT_FILE_FORMAT_DEFAULT;
190190
import static org.apache.iceberg.TableProperties.FORMAT_VERSION;
191+
import static org.apache.iceberg.TableProperties.GC_ENABLED;
192+
import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT;
191193
import static org.apache.iceberg.TableProperties.OBJECT_STORE_ENABLED;
192194
import static org.apache.iceberg.TableProperties.OBJECT_STORE_ENABLED_DEFAULT;
193195
import static org.apache.iceberg.TableProperties.ORC_BLOOM_FILTER_COLUMNS;
@@ -869,7 +871,7 @@ public static List<ViewColumn> viewColumnsFromSchema(TypeManager typeManager, Sc
869871
.toList();
870872
}
871873

872-
public static Transaction newCreateTableTransaction(TrinoCatalog catalog, ConnectorTableMetadata tableMetadata, ConnectorSession session, boolean replace, String tableLocation, Predicate<String> allowedExtraProperties)
874+
public static Transaction newCreateTableTransaction(TrinoCatalog catalog, ConnectorTableMetadata tableMetadata, ConnectorSession session, boolean replace, String tableLocation, Predicate<String> allowedExtraProperties, boolean defaultNewTablesGcEnabled)
873875
{
874876
SchemaTableName schemaTableName = tableMetadata.getTable();
875877
Schema schema = schemaFromMetadata(tableMetadata.getColumns());
@@ -879,10 +881,10 @@ public static Transaction newCreateTableTransaction(TrinoCatalog catalog, Connec
879881
Transaction transaction;
880882

881883
if (replace) {
882-
transaction = catalog.newCreateOrReplaceTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, tableLocation, createTableProperties(tableMetadata, allowedExtraProperties));
884+
transaction = catalog.newCreateOrReplaceTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, tableLocation, createTableProperties(tableMetadata, allowedExtraProperties, defaultNewTablesGcEnabled));
883885
}
884886
else {
885-
transaction = catalog.newCreateTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, Optional.ofNullable(tableLocation), createTableProperties(tableMetadata, allowedExtraProperties));
887+
transaction = catalog.newCreateTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, Optional.ofNullable(tableLocation), createTableProperties(tableMetadata, allowedExtraProperties, defaultNewTablesGcEnabled));
886888
}
887889

888890
// 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
897899
return transaction;
898900
}
899901

900-
public static Map<String, String> createTableProperties(ConnectorTableMetadata tableMetadata, Predicate<String> allowedExtraProperties)
902+
public static Map<String, String> createTableProperties(ConnectorTableMetadata tableMetadata, Predicate<String> allowedExtraProperties, boolean defaultNewTablesGcEnabled)
901903
{
902904
ImmutableMap.Builder<String, String> propertiesBuilder = ImmutableMap.builder();
903905
IcebergFileFormat fileFormat = IcebergTableProperties.getFileFormat(tableMetadata.getProperties());
@@ -960,6 +962,11 @@ public static Map<String, String> createTableProperties(ConnectorTableMetadata t
960962

961963
verifyExtraProperties(baseProperties.keySet(), extraProperties, allowedExtraProperties);
962964

965+
// If user doesn't set gc.enabled, we need to set it to defaultNewTablesGcEnabled value
966+
if (!extraProperties.containsKey(GC_ENABLED) && defaultNewTablesGcEnabled != GC_ENABLED_DEFAULT) {
967+
baseProperties.put(GC_ENABLED, Boolean.toString(defaultNewTablesGcEnabled));
968+
}
969+
963970
return ImmutableMap.<String, String>builder()
964971
.putAll(baseProperties)
965972
.putAll(extraProperties)

‎plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
import static org.apache.iceberg.BaseMetastoreTableOperations.METADATA_LOCATION_PROP;
108108
import static org.apache.iceberg.TableMetadata.newTableMetadata;
109109
import static org.apache.iceberg.TableMetadataParser.getFileExtension;
110+
import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT;
110111
import static org.apache.iceberg.TableProperties.METADATA_COMPRESSION_DEFAULT;
111112
import static org.apache.iceberg.Transactions.createOrReplaceTableTransaction;
112113
import static org.apache.iceberg.Transactions.createTableTransaction;
@@ -312,7 +313,7 @@ protected Location createMaterializedViewStorage(
312313
Schema schema = schemaFromMetadata(columns);
313314
PartitionSpec partitionSpec = parsePartitionFields(schema, getPartitioning(materializedViewProperties));
314315
SortOrder sortOrder = parseSortFields(schema, getSortOrder(materializedViewProperties));
315-
Map<String, String> properties = createTableProperties(new ConnectorTableMetadata(storageTableName, columns, materializedViewProperties, Optional.empty()), _ -> false);
316+
Map<String, String> properties = createTableProperties(new ConnectorTableMetadata(storageTableName, columns, materializedViewProperties, Optional.empty()), _ -> false, GC_ENABLED_DEFAULT);
316317

317318
TableMetadata metadata = newTableMetadata(schema, partitionSpec, sortOrder, tableLocation, properties);
318319

@@ -350,7 +351,7 @@ protected SchemaTableName createMaterializedViewStorageTable(
350351
ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(storageTable, columns, materializedViewProperties, Optional.empty());
351352
String tableLocation = getTableLocation(tableMetadata.getProperties())
352353
.orElseGet(() -> defaultTableLocation(session, tableMetadata.getTable()));
353-
Transaction transaction = IcebergUtil.newCreateTableTransaction(this, tableMetadata, session, false, tableLocation, _ -> false);
354+
Transaction transaction = IcebergUtil.newCreateTableTransaction(this, tableMetadata, session, false, tableLocation, _ -> false, GC_ENABLED_DEFAULT);
354355
AppendFiles appendFiles = transaction.newAppend();
355356
commit(appendFiles, session);
356357
transaction.commitTransaction();

‎plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@
160160
import static java.util.Objects.requireNonNull;
161161
import static org.apache.iceberg.BaseMetastoreTableOperations.METADATA_LOCATION_PROP;
162162
import static org.apache.iceberg.CatalogUtil.dropTableData;
163+
import static org.apache.iceberg.TableProperties.GC_ENABLED;
164+
import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT;
165+
import static org.apache.iceberg.util.PropertyUtil.propertyAsBoolean;
163166

164167
public class TrinoGlueCatalog
165168
extends AbstractTrinoCatalog
@@ -707,7 +710,9 @@ public void dropTable(ConnectorSession session, SchemaTableName schemaTableName)
707710
// So log the exception and continue with deleting the table location
708711
LOG.warn(e, "Failed to delete table data referenced by metadata");
709712
}
710-
deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location());
713+
if (propertyAsBoolean(table.properties(), GC_ENABLED, GC_ENABLED_DEFAULT)) {
714+
deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location());
715+
}
711716
invalidateTableCache(schemaTableName);
712717
}
713718

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@
127127
import static org.apache.iceberg.BaseMetastoreTableOperations.METADATA_LOCATION_PROP;
128128
import static org.apache.iceberg.BaseMetastoreTableOperations.TABLE_TYPE_PROP;
129129
import static org.apache.iceberg.CatalogUtil.dropTableData;
130+
import static org.apache.iceberg.TableProperties.GC_ENABLED;
131+
import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT;
132+
import static org.apache.iceberg.util.PropertyUtil.propertyAsBoolean;
130133

131134
public class TrinoHiveCatalog
132135
extends AbstractTrinoCatalog
@@ -436,7 +439,9 @@ public void dropTable(ConnectorSession session, SchemaTableName schemaTableName)
436439
// So log the exception and continue with deleting the table location
437440
log.warn(e, "Failed to delete table data referenced by metadata");
438441
}
439-
deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, metastoreTable.getStorage().getLocation());
442+
if (propertyAsBoolean(metadata.properties(), GC_ENABLED, GC_ENABLED_DEFAULT)) {
443+
deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, metastoreTable.getStorage().getLocation());
444+
}
440445
invalidateTableCache(schemaTableName);
441446
}
442447

‎plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@
8787
import static java.util.Locale.ENGLISH;
8888
import static java.util.Objects.requireNonNull;
8989
import static org.apache.iceberg.CatalogUtil.dropTableData;
90+
import static org.apache.iceberg.TableProperties.GC_ENABLED;
91+
import static org.apache.iceberg.TableProperties.GC_ENABLED_DEFAULT;
92+
import static org.apache.iceberg.util.PropertyUtil.propertyAsBoolean;
9093
import static org.apache.iceberg.view.ViewProperties.COMMENT;
9194

9295
public class TrinoJdbcCatalog
@@ -348,7 +351,9 @@ public void dropTable(ConnectorSession session, SchemaTableName schemaTableName)
348351
// So log the exception and continue with deleting the table location
349352
LOG.warn(e, "Failed to delete table data referenced by metadata");
350353
}
351-
deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location());
354+
if (propertyAsBoolean(table.properties(), GC_ENABLED, GC_ENABLED_DEFAULT)) {
355+
deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location());
356+
}
352357
invalidateTableCache(schemaTableName);
353358
}
354359

0 commit comments

Comments
 (0)