diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java index 913c501b5795d..4d071a205089b 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java @@ -230,11 +230,11 @@ import static com.facebook.presto.spi.MaterializedViewStatus.MaterializedViewState.NOT_MATERIALIZED; import static com.facebook.presto.spi.MaterializedViewStatus.MaterializedViewState.PARTIALLY_MATERIALIZED; import static com.facebook.presto.spi.StandardErrorCode.ALREADY_EXISTS; +import static com.facebook.presto.spi.StandardErrorCode.INVALID_VIEW; import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; import static com.facebook.presto.spi.connector.RowChangeParadigm.DELETE_ROW_AND_INSERT_ROW; import static com.facebook.presto.spi.statistics.TableStatisticType.ROW_COUNT; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Verify.verify; import static com.google.common.collect.ImmutableList.toImmutableList; @@ -1658,32 +1658,44 @@ public void createMaterializedView( viewMetadata.getComment()); createTable(session, storageTableMetadata, false); - Map properties = new HashMap<>(); - properties.put(PRESTO_MATERIALIZED_VIEW_FORMAT_VERSION, CURRENT_MATERIALIZED_VIEW_FORMAT_VERSION + ""); - properties.put(PRESTO_MATERIALIZED_VIEW_ORIGINAL_SQL, viewDefinition.getOriginalSql()); - properties.put(PRESTO_MATERIALIZED_VIEW_STORAGE_SCHEMA, storageTableName.getSchemaName()); - properties.put(PRESTO_MATERIALIZED_VIEW_STORAGE_TABLE_NAME, storageTableName.getTableName()); - - String baseTablesStr = serializeSchemaTableNames(viewDefinition.getBaseTables()); - properties.put(PRESTO_MATERIALIZED_VIEW_BASE_TABLES, baseTablesStr); - properties.put(PRESTO_MATERIALIZED_VIEW_COLUMN_MAPPINGS, serializeColumnMappings(viewDefinition.getColumnMappings())); - checkState(viewDefinition.getOwner().isPresent(), "Materialized view owner is required"); - properties.put(PRESTO_MATERIALIZED_VIEW_OWNER, viewDefinition.getOwner().get()); - checkState(viewDefinition.getSecurityMode().isPresent(), "Materialized view security mode is required"); - properties.put(PRESTO_MATERIALIZED_VIEW_SECURITY_MODE, viewDefinition.getSecurityMode().get().name()); - - getStaleReadBehavior(materializedViewProperties) - .ifPresent(behavior -> properties.put(PRESTO_MATERIALIZED_VIEW_STALE_READ_BEHAVIOR, behavior.name())); - getStalenessWindow(materializedViewProperties) - .ifPresent(window -> properties.put(PRESTO_MATERIALIZED_VIEW_STALENESS_WINDOW, window.toString())); - MaterializedViewRefreshType refreshType = getRefreshType(materializedViewProperties); - properties.put(PRESTO_MATERIALIZED_VIEW_REFRESH_TYPE, refreshType.name()); - - for (SchemaTableName baseTable : viewDefinition.getBaseTables()) { - properties.put(getBaseTableViewPropertyName(baseTable), "0"); - } + try { + Map properties = new HashMap<>(); + properties.put(PRESTO_MATERIALIZED_VIEW_FORMAT_VERSION, CURRENT_MATERIALIZED_VIEW_FORMAT_VERSION + ""); + properties.put(PRESTO_MATERIALIZED_VIEW_ORIGINAL_SQL, viewDefinition.getOriginalSql()); + properties.put(PRESTO_MATERIALIZED_VIEW_STORAGE_SCHEMA, storageTableName.getSchemaName()); + properties.put(PRESTO_MATERIALIZED_VIEW_STORAGE_TABLE_NAME, storageTableName.getTableName()); + + String baseTablesStr = serializeSchemaTableNames(viewDefinition.getBaseTables()); + properties.put(PRESTO_MATERIALIZED_VIEW_BASE_TABLES, baseTablesStr); + properties.put(PRESTO_MATERIALIZED_VIEW_COLUMN_MAPPINGS, serializeColumnMappings(viewDefinition.getColumnMappings())); + properties.put(PRESTO_MATERIALIZED_VIEW_OWNER, viewDefinition.getOwner() + .orElseThrow(() -> new PrestoException(INVALID_VIEW, "Materialized view owner is required"))); + properties.put(PRESTO_MATERIALIZED_VIEW_SECURITY_MODE, viewDefinition.getSecurityMode() + .orElseThrow(() -> new PrestoException(INVALID_VIEW, "Materialized view security mode is required (set legacy_materialized_views=false)")) + .name()); + + getStaleReadBehavior(materializedViewProperties) + .ifPresent(behavior -> properties.put(PRESTO_MATERIALIZED_VIEW_STALE_READ_BEHAVIOR, behavior.name())); + getStalenessWindow(materializedViewProperties) + .ifPresent(window -> properties.put(PRESTO_MATERIALIZED_VIEW_STALENESS_WINDOW, window.toString())); + MaterializedViewRefreshType refreshType = getRefreshType(materializedViewProperties); + properties.put(PRESTO_MATERIALIZED_VIEW_REFRESH_TYPE, refreshType.name()); + + for (SchemaTableName baseTable : viewDefinition.getBaseTables()) { + properties.put(getBaseTableViewPropertyName(baseTable), "0"); + } - createIcebergView(session, viewName, viewMetadata.getColumns(), viewDefinition.getOriginalSql(), properties); + createIcebergView(session, viewName, viewMetadata.getColumns(), viewDefinition.getOriginalSql(), properties); + } + catch (Exception e) { + try { + dropStorageTable(session, storageTableName); + } + catch (Exception cleanupException) { + e.addSuppressed(cleanupException); + } + throw e; + } } catch (PrestoException e) { if (e.getErrorCode() == NOT_SUPPORTED.toErrorCode()) { @@ -1693,6 +1705,14 @@ public void createMaterializedView( } } + private void dropStorageTable(ConnectorSession session, SchemaTableName storageTableName) + { + ConnectorTableHandle storageTableHandle = getTableHandle(session, storageTableName); + if (storageTableHandle != null) { + dropTable(session, storageTableHandle); + } + } + @Override public List listMaterializedViews(ConnectorSession session, String schemaName) { diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergMaterializedViewMetadata.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergMaterializedViewMetadata.java index 45c1fad7c8740..c441132c7654a 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergMaterializedViewMetadata.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergMaterializedViewMetadata.java @@ -14,6 +14,7 @@ package com.facebook.presto.iceberg; import com.facebook.airlift.http.server.testing.TestingHttpServer; +import com.facebook.presto.Session; import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.tests.AbstractTestQueryFramework; import com.google.common.collect.ImmutableMap; @@ -98,7 +99,9 @@ protected QueryRunner createQueryRunner() .setDataDirectory(Optional.of(warehouseLocation.toPath())) .setSchemaName("test_schema") .setCreateTpchTables(false) - .setExtraProperties(ImmutableMap.of("experimental.legacy-materialized-views", "false")) + .setExtraProperties(ImmutableMap.of( + "experimental.legacy-materialized-views", "false", + "experimental.allow-legacy-materialized-views-toggle", "true")) .build().getQueryRunner(); } @@ -756,4 +759,44 @@ public void testStalenessPropertiesStoredInView() assertUpdate("DROP MATERIALIZED VIEW test_staleness_props_mv"); assertUpdate("DROP TABLE test_staleness_props_base"); } + + @Test + public void testNoOrphanStorageTableOnValidationFailure() + throws Exception + { + try (RESTCatalog catalog = new RESTCatalog()) { + assertUpdate("CREATE TABLE test_orphan_base (id BIGINT, value BIGINT)"); + assertUpdate("INSERT INTO test_orphan_base VALUES (1, 100)", 1); + + Session legacySession = Session.builder(getSession()) + .setSystemProperty("legacy_materialized_views", "true") + .build(); + + String mvName = "test_orphan_mv"; + String storageTableName = "__mv_storage__" + mvName; + + assertQueryFails( + legacySession, + "CREATE MATERIALIZED VIEW " + mvName + " AS SELECT id, value FROM test_orphan_base", + ".*Materialized view security mode is required.*"); + + assertQueryFails( + "SELECT COUNT(*) FROM \"" + storageTableName + "\"", + ".*(does not exist|not found).*"); + + Map catalogProps = new HashMap<>(); + catalogProps.put("uri", serverUri); + catalogProps.put("warehouse", warehouseLocation.getAbsolutePath()); + catalog.initialize("test_catalog", catalogProps); + + TableIdentifier storageTableId = TableIdentifier.of(Namespace.of("test_schema"), storageTableName); + boolean tableExists = catalog.tableExists(storageTableId); + assertFalse(tableExists, + "Storage table should not exist after failed MV creation. " + + "This would indicate validation happened after storage table creation."); + } + finally { + assertUpdate("DROP TABLE test_orphan_base"); + } + } }