Skip to content

Commit 9f93352

Browse files
alexjo2144Alex Jo
alexjo2144
authored and
Alex Jo
committed
Fix Iceberg materialized view storage table cleanup
1 parent 76c311c commit 9f93352

File tree

3 files changed

+72
-19
lines changed

3 files changed

+72
-19
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import io.trino.plugin.iceberg.IcebergMaterializedViewDefinition;
2626
import io.trino.plugin.iceberg.IcebergUtil;
2727
import io.trino.plugin.iceberg.PartitionTransforms.ColumnTransform;
28+
import io.trino.plugin.iceberg.fileio.ForwardingFileIo;
2829
import io.trino.plugin.iceberg.fileio.ForwardingOutputFile;
2930
import io.trino.spi.TrinoException;
3031
import io.trino.spi.catalog.CatalogName;
@@ -336,6 +337,14 @@ protected Location createMaterializedViewStorage(
336337
return metadataFileLocation;
337338
}
338339

340+
protected void dropMaterializedViewStorage(TrinoFileSystem fileSystem, String storageMetadataLocation)
341+
throws IOException
342+
{
343+
TableMetadata metadata = TableMetadataParser.read(new ForwardingFileIo(fileSystem), storageMetadataLocation);
344+
String storageLocation = metadata.location();
345+
fileSystem.deleteDirectory(Location.of(storageLocation));
346+
}
347+
339348
protected SchemaTableName createMaterializedViewStorageTable(
340349
ConnectorSession session,
341350
SchemaTableName viewName,

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

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
import org.apache.iceberg.exceptions.NotFoundException;
9090
import org.apache.iceberg.io.FileIO;
9191

92+
import java.io.IOException;
9293
import java.time.Duration;
9394
import java.util.HashMap;
9495
import java.util.Iterator;
@@ -1129,12 +1130,28 @@ public void createMaterializedView(
11291130
encodeMaterializedViewData(fromConnectorMaterializedViewDefinition(definition)),
11301131
isUsingSystemSecurity ? null : session.getUser(),
11311132
createMaterializedViewProperties(session, storageMetadataLocation));
1132-
if (existing.isPresent()) {
1133-
updateTable(viewName.getSchemaName(), materializedViewTableInput);
1133+
try {
1134+
if (existing.isPresent()) {
1135+
updateTable(viewName.getSchemaName(), materializedViewTableInput);
1136+
}
1137+
else {
1138+
createTable(viewName.getSchemaName(), materializedViewTableInput);
1139+
}
11341140
}
1135-
else {
1136-
createTable(viewName.getSchemaName(), materializedViewTableInput);
1141+
catch (RuntimeException e) {
1142+
try {
1143+
dropMaterializedViewStorage(fileSystemFactory.create(session), storageMetadataLocation.toString());
1144+
}
1145+
catch (Exception suppressed) {
1146+
LOG.warn(suppressed, "Failed to clean up metadata '%s' for materialized view '%s'", storageMetadataLocation, viewName);
1147+
if (e != suppressed) {
1148+
e.addSuppressed(suppressed);
1149+
}
1150+
}
1151+
throw e;
11371152
}
1153+
1154+
existing.ifPresent(existingView -> dropMaterializedViewStorage(session, existingView));
11381155
}
11391156
else {
11401157
createMaterializedViewWithStorageTable(session, viewName, definition, materializedViewProperties, existing);
@@ -1173,7 +1190,7 @@ private void createMaterializedViewWithStorageTable(
11731190
}
11741191
}
11751192
}
1176-
dropStorageTable(session, existing.get());
1193+
dropMaterializedViewStorage(session, existing.get());
11771194
}
11781195
else {
11791196
createTable(viewName.getSchemaName(), materializedViewTableInput);
@@ -1227,11 +1244,11 @@ public void dropMaterializedView(ConnectorSession session, SchemaTableName viewN
12271244
throw new TrinoException(UNSUPPORTED_TABLE_TYPE, "Not a Materialized View: " + view.getDatabaseName() + "." + view.getName());
12281245
}
12291246
materializedViewCache.invalidate(viewName);
1230-
dropStorageTable(session, view);
1247+
dropMaterializedViewStorage(session, view);
12311248
deleteTable(view.getDatabaseName(), view.getName());
12321249
}
12331250

1234-
private void dropStorageTable(ConnectorSession session, com.amazonaws.services.glue.model.Table view)
1251+
private void dropMaterializedViewStorage(ConnectorSession session, com.amazonaws.services.glue.model.Table view)
12351252
{
12361253
Map<String, String> parameters = getTableParameters(view);
12371254
String storageTableName = parameters.get(STORAGE_TABLE);
@@ -1245,6 +1262,16 @@ private void dropStorageTable(ConnectorSession session, com.amazonaws.services.g
12451262
LOG.warn(e, "Failed to drop storage table '%s.%s' for materialized view '%s'", storageSchema, storageTableName, view.getName());
12461263
}
12471264
}
1265+
else {
1266+
String storageMetadataLocation = parameters.get(METADATA_LOCATION_PROP);
1267+
checkState(storageMetadataLocation != null, "Storage location missing in definition of materialized view " + view.getName());
1268+
try {
1269+
dropMaterializedViewStorage(fileSystemFactory.create(session), storageMetadataLocation);
1270+
}
1271+
catch (IOException e) {
1272+
LOG.warn(e, "Failed to delete storage table metadata '%s' for materialized view '%s'", storageMetadataLocation, view.getName());
1273+
}
1274+
}
12481275
}
12491276

12501277
@Override

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

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -566,12 +566,28 @@ public void createMaterializedView(
566566
io.trino.plugin.hive.metastore.Table table = tableBuilder.build();
567567
PrincipalPrivileges principalPrivileges = isUsingSystemSecurity ? NO_PRIVILEGES : buildInitialPrivilegeSet(session.getUser());
568568

569-
if (existing.isPresent()) {
570-
metastore.replaceTable(viewName.getSchemaName(), viewName.getTableName(), table, principalPrivileges);
569+
try {
570+
if (existing.isPresent()) {
571+
metastore.replaceTable(viewName.getSchemaName(), viewName.getTableName(), table, principalPrivileges);
572+
}
573+
else {
574+
metastore.createTable(table, principalPrivileges);
575+
}
571576
}
572-
else {
573-
metastore.createTable(table, principalPrivileges);
577+
catch (RuntimeException e) {
578+
try {
579+
dropMaterializedViewStorage(fileSystemFactory.create(session), storageMetadataLocation.toString());
580+
}
581+
catch (Exception suppressed) {
582+
log.warn(suppressed, "Failed to clean up metadata '%s' for materialized view '%s'", storageMetadataLocation, viewName);
583+
if (e != suppressed) {
584+
e.addSuppressed(suppressed);
585+
}
586+
}
587+
throw e;
574588
}
589+
590+
existing.ifPresent(existingView -> dropMaterializedViewStorage(session, existingView));
575591
}
576592
else {
577593
createMaterializedViewWithStorageTable(session, viewName, definition, materializedViewProperties, existing);
@@ -673,6 +689,13 @@ public void dropMaterializedView(ConnectorSession session, SchemaTableName viewN
673689
throw new TrinoException(UNSUPPORTED_TABLE_TYPE, "Not a Materialized View: " + viewName);
674690
}
675691

692+
dropMaterializedViewStorage(session, view);
693+
metastore.dropTable(viewName.getSchemaName(), viewName.getTableName(), true);
694+
}
695+
696+
private void dropMaterializedViewStorage(ConnectorSession session, io.trino.plugin.hive.metastore.Table view)
697+
{
698+
SchemaTableName viewName = view.getSchemaTableName();
676699
String storageTableName = view.getParameters().get(STORAGE_TABLE);
677700
if (storageTableName != null) {
678701
String storageSchema = Optional.ofNullable(view.getParameters().get(STORAGE_SCHEMA))
@@ -687,19 +710,13 @@ public void dropMaterializedView(ConnectorSession session, SchemaTableName viewN
687710
else {
688711
String storageMetadataLocation = view.getParameters().get(METADATA_LOCATION_PROP);
689712
checkState(storageMetadataLocation != null, "Storage location missing in definition of materialized view " + viewName);
690-
691-
TrinoFileSystem fileSystem = fileSystemFactory.create(session);
692-
TableMetadata metadata = TableMetadataParser.read(new ForwardingFileIo(fileSystem), storageMetadataLocation);
693-
String storageLocation = metadata.location();
694713
try {
695-
fileSystem.deleteDirectory(Location.of(storageLocation));
714+
dropMaterializedViewStorage(fileSystemFactory.create(session), storageMetadataLocation);
696715
}
697716
catch (IOException e) {
698-
log.warn(e, "Failed to delete storage location '%s' for materialized view '%s'", storageLocation, viewName);
717+
log.warn(e, "Failed to delete storage table metadata '%s' for materialized view '%s'", storageMetadataLocation, viewName);
699718
}
700719
}
701-
702-
metastore.dropTable(viewName.getSchemaName(), viewName.getTableName(), true);
703720
}
704721

705722
@Override

0 commit comments

Comments
 (0)