Remove legacy Iceberg MV separate storage table support

Author: findepi

docs/src/main/sphinx/connector/iceberg.md

@@ -151,16 +151,6 @@ implementation is used:
 * - `iceberg.hive-catalog-name`
   - Catalog to redirect to when a Hive table is referenced.
   -
-* - `iceberg.materialized-views.storage-schema`
-  - Schema for creating materialized views storage tables. When this property is
-    not configured, storage tables are created in the same schema as the
-    materialized view definition. When the `storage_schema` materialized view
-    property is specified, it takes precedence over this catalog property.
-  - Empty
-* - `iceberg.materialized-views.hide-storage-table`
-  - Hide the information about the storage table backing the materialized view
-    in the metastore.
-  - `true`
 * - `iceberg.register-table-procedure.enabled`
   - Enable to allow user to call `register_table` procedure.
   - `false`
@@ -1492,11 +1482,6 @@ for the data files and partition the storage per day using the column
 WITH ( format = 'ORC', partitioning = ARRAY['event_date'] )
 ```
 
-By default, the storage table is created in the same schema as the materialized
-view definition. The `iceberg.materialized-views.storage-schema` catalog
-configuration property or `storage_schema` materialized view property can be
-used to specify the schema where the storage table is created.
-
 Creating a materialized view does not automatically populate it with data. You
 must run {doc}`/sql/refresh-materialized-view` to populate data in the
 materialized view.

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

@@ -0,0 +1,29 @@
+/*
+ * 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.iceberg;
+
+import com.google.inject.BindingAnnotation;
+
+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})
+@BindingAnnotation
+public @interface EnableMaterializedViewSeparateStorageTable {}

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

@@ -20,7 +20,6 @@
 import io.airlift.units.DataSize;
 import io.airlift.units.Duration;
 import io.trino.plugin.hive.HiveCompressionCodec;
-import jakarta.validation.constraints.AssertFalse;
 import jakarta.validation.constraints.DecimalMax;
 import jakarta.validation.constraints.DecimalMin;
 import jakarta.validation.constraints.Max;
@@ -74,8 +73,6 @@ public class IcebergConfig
     // to avoid deleting those files if Trino is unable to check.
     private boolean deleteSchemaLocationsFallback;
     private double minimumAssignedSplitWeight = 0.05;
-    private boolean hideMaterializedViewStorageTable = true;
-    private Optional<String> materializedViewsStorageSchema = Optional.empty();
     private boolean sortedWritingEnabled = true;
     private boolean queryPartitionFilterRequired;
     private int splitManagerThreads = Runtime.getRuntime().availableProcessors() * 2;
@@ -361,33 +358,6 @@ public double getMinimumAssignedSplitWeight()
         return minimumAssignedSplitWeight;
     }
 
-    public boolean isHideMaterializedViewStorageTable()
-    {
-        return hideMaterializedViewStorageTable;
-    }
-
-    @Config("iceberg.materialized-views.hide-storage-table")
-    @ConfigDescription("Hide materialized view storage tables in metastore")
-    public IcebergConfig setHideMaterializedViewStorageTable(boolean hideMaterializedViewStorageTable)
-    {
-        this.hideMaterializedViewStorageTable = hideMaterializedViewStorageTable;
-        return this;
-    }
-
-    @NotNull
-    public Optional<String> getMaterializedViewsStorageSchema()
-    {
-        return materializedViewsStorageSchema;
-    }
-
-    @Config("iceberg.materialized-views.storage-schema")
-    @ConfigDescription("Schema for creating materialized views storage tables")
-    public IcebergConfig setMaterializedViewsStorageSchema(String materializedViewsStorageSchema)
-    {
-        this.materializedViewsStorageSchema = Optional.ofNullable(materializedViewsStorageSchema);
-        return this;
-    }
-
     public boolean isSortedWritingEnabled()
     {
         return sortedWritingEnabled;
@@ -427,10 +397,4 @@ public IcebergConfig setSplitManagerThreads(int splitManagerThreads)
         this.splitManagerThreads = splitManagerThreads;
         return this;
     }
-
-    @AssertFalse(message = "iceberg.materialized-views.storage-schema may only be set when iceberg.materialized-views.hide-storage-table is set to false")
-    public boolean isStorageSchemaSetWhenHidingIsEnabled()
-    {
-        return hideMaterializedViewStorageTable && materializedViewsStorageSchema.isPresent();
-    }
 }

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

@@ -21,35 +21,45 @@
 import java.util.Map;
 import java.util.Optional;
 
-import static io.trino.spi.session.PropertyMetadata.stringProperty;
+import static io.trino.spi.session.PropertyMetadata.booleanProperty;
 
 public class IcebergMaterializedViewProperties
 {
-    public static final String STORAGE_SCHEMA = "storage_schema";
+    public static final String SEPARATE_STORAGE_TABLE = "separate_storage_table";
 
     private final List<PropertyMetadata<?>> materializedViewProperties;
 
     @Inject
-    public IcebergMaterializedViewProperties(IcebergConfig icebergConfig, IcebergTableProperties tableProperties)
+    public IcebergMaterializedViewProperties(
+            @EnableMaterializedViewSeparateStorageTable boolean enableMaterializedViewSeparateStorageTable, // for tests of legacy materialized views
+            IcebergTableProperties tableProperties)
     {
-        materializedViewProperties = ImmutableList.<PropertyMetadata<?>>builder()
-                .add(stringProperty(
-                        STORAGE_SCHEMA,
-                        "Schema for creating materialized view storage table",
-                        icebergConfig.getMaterializedViewsStorageSchema().orElse(null),
-                        false))
-                // Materialized view should allow configuring all the supported iceberg table properties for the storage table
-                .addAll(tableProperties.getTableProperties())
-                .build();
+        ImmutableList.Builder<PropertyMetadata<?>> materializedViewProperties = ImmutableList.builder();
+        if (enableMaterializedViewSeparateStorageTable) {
+            materializedViewProperties.add(booleanProperty(
+                    SEPARATE_STORAGE_TABLE,
+                    "Create separate storage table for materialized view",
+                    false,
+                    true));
+        }
+        // Materialized view should allow configuring all the supported iceberg table properties for the storage table
+        materializedViewProperties.addAll(tableProperties.getTableProperties());
+        this.materializedViewProperties = materializedViewProperties.build();
     }
 
     public List<PropertyMetadata<?>> getMaterializedViewProperties()
     {
         return materializedViewProperties;
     }
 
-    public static Optional<String> getStorageSchema(Map<String, Object> materializedViewProperties)
+    /**
+     * @deprecated TODO creation of legacy materialized views with separate storage table to test code
+     */
+    @Deprecated
+    public static boolean isSeparateStorageTable(Map<String, Object> materializedViewProperties)
     {
-        return Optional.ofNullable((String) materializedViewProperties.get(STORAGE_SCHEMA));
+        return Optional.ofNullable(materializedViewProperties.get(SEPARATE_STORAGE_TABLE))
+                .map(Boolean.class::cast)
+                .orElse(false);
     }
 }

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

@@ -76,6 +76,7 @@ public void configure(Binder binder)
         newSetBinder(binder, SessionPropertiesProvider.class).addBinding().to(IcebergSessionProperties.class).in(Scopes.SINGLETON);
         binder.bind(IcebergTableProperties.class).in(Scopes.SINGLETON);
         binder.bind(IcebergMaterializedViewProperties.class).in(Scopes.SINGLETON);
+        newOptionalBinder(binder, Key.get(boolean.class, EnableMaterializedViewSeparateStorageTable.class)).setDefault().toInstance(false);
         binder.bind(IcebergAnalyzeProperties.class).in(Scopes.SINGLETON);
 
         binder.bind(ConnectorSplitManager.class).to(IcebergSplitManager.class).in(Scopes.SINGLETON);

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

@@ -69,6 +69,7 @@
 import java.util.Set;
 import java.util.stream.Stream;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Throwables.throwIfUnchecked;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
@@ -79,10 +80,8 @@
 import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.mappedCopy;
 import static io.trino.plugin.hive.util.HiveUtil.escapeTableName;
 import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_FILESYSTEM_ERROR;
-import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_INVALID_METADATA;
 import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.decodeMaterializedViewData;
-import static io.trino.plugin.iceberg.IcebergMaterializedViewProperties.STORAGE_SCHEMA;
-import static io.trino.plugin.iceberg.IcebergMaterializedViewProperties.getStorageSchema;
+import static io.trino.plugin.iceberg.IcebergMaterializedViewProperties.isSeparateStorageTable;
 import static io.trino.plugin.iceberg.IcebergTableName.tableNameWithType;
 import static io.trino.plugin.iceberg.IcebergTableProperties.getPartitioning;
 import static io.trino.plugin.iceberg.IcebergTableProperties.getSortOrder;
@@ -97,7 +96,6 @@
 import static io.trino.plugin.iceberg.SortFieldUtils.parseSortFields;
 import static io.trino.plugin.iceberg.TableType.MATERIALIZED_VIEW_STORAGE;
 import static io.trino.plugin.iceberg.TypeConverter.toTrinoType;
-import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
 import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND;
 import static io.trino.spi.type.IntegerType.INTEGER;
 import static io.trino.spi.type.SmallintType.SMALLINT;
@@ -211,16 +209,9 @@ public Optional<ConnectorMaterializedViewDefinition> getMaterializedView(Connect
     @Override
     public Map<String, Object> getMaterializedViewProperties(ConnectorSession session, SchemaTableName viewName, ConnectorMaterializedViewDefinition definition)
     {
-        SchemaTableName storageTableName = definition.getStorageTable()
-                .orElseThrow(() -> new TrinoException(ICEBERG_INVALID_METADATA, "Materialized view definition is missing a storage table"))
-                .getSchemaTableName();
-
         try {
             Table storageTable = loadTable(session, definition.getStorageTable().orElseThrow().getSchemaTableName());
-            return ImmutableMap.<String, Object>builder()
-                    .putAll(getIcebergTableProperties(storageTable))
-                    .put(STORAGE_SCHEMA, storageTableName.getSchemaName())
-                    .buildOrThrow();
+            return getIcebergTableProperties(storageTable);
         }
         catch (RuntimeException e) {
             throw new TrinoException(ICEBERG_FILESYSTEM_ERROR, "Unable to load storage table metadata for materialized view: " + viewName);
@@ -313,9 +304,7 @@ protected Location createMaterializedViewStorage(
             ConnectorMaterializedViewDefinition definition,
             Map<String, Object> materializedViewProperties)
     {
-        if (getStorageSchema(materializedViewProperties).isPresent()) {
-            throw new TrinoException(NOT_SUPPORTED, "Materialized view property '%s' is not supported when hiding materialized view storage tables is enabled".formatted(STORAGE_SCHEMA));
-        }
+        checkArgument(!isSeparateStorageTable(materializedViewProperties), "Invalid request to create hidden materialized view storage table when separate storage table is requested");
         SchemaTableName storageTableName = new SchemaTableName(viewName.getSchemaName(), tableNameWithType(viewName.getTableName(), MATERIALIZED_VIEW_STORAGE));
         String tableLocation = getTableLocation(materializedViewProperties)
                 .orElseGet(() -> defaultTableLocation(session, viewName));
@@ -355,7 +344,7 @@ protected SchemaTableName createMaterializedViewStorageTable(
         // storage table as indicated in the materialized view definition.
         String storageTableName = "st_" + randomUUID().toString().replace("-", "");
 
-        String storageSchema = getStorageSchema(materializedViewProperties).orElse(viewName.getSchemaName());
+        String storageSchema = viewName.getSchemaName();
         SchemaTableName storageTable = new SchemaTableName(storageSchema, storageTableName);
         List<ColumnMetadata> columns = columnsForMaterializedView(definition, materializedViewProperties);
 
@@ -494,7 +483,6 @@ protected Map<String, String> createMaterializedViewProperties(ConnectorSession
     {
         return ImmutableMap.<String, String>builder()
                 .put(TRINO_QUERY_ID_NAME, session.getQueryId())
-                .put(STORAGE_SCHEMA, storageTableName.getSchemaName())
                 .put(STORAGE_TABLE, storageTableName.getTableName())
                 .put(PRESTO_VIEW_FLAG, "true")
                 .put(TRINO_CREATED_BY, TRINO_CREATED_BY_VALUE)

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

@@ -136,7 +136,7 @@
 import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.decodeMaterializedViewData;
 import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.encodeMaterializedViewData;
 import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.fromConnectorMaterializedViewDefinition;
-import static io.trino.plugin.iceberg.IcebergMaterializedViewProperties.STORAGE_SCHEMA;
+import static io.trino.plugin.iceberg.IcebergMaterializedViewProperties.isSeparateStorageTable;
 import static io.trino.plugin.iceberg.IcebergSchemaProperties.LOCATION_PROPERTY;
 import static io.trino.plugin.iceberg.IcebergTableName.tableNameWithType;
 import static io.trino.plugin.iceberg.IcebergUtil.COLUMN_TRINO_NOT_NULL_PROPERTY;
@@ -179,7 +179,6 @@ public class TrinoGlueCatalog
     private final Optional<String> defaultSchemaLocation;
     private final AWSGlueAsync glueClient;
     private final GlueMetastoreStats stats;
-    private final boolean hideMaterializedViewStorageTable;
     private final boolean isUsingSystemSecurity;
 
     private final Cache<SchemaTableName, com.amazonaws.services.glue.model.Table> glueTableCache = EvictableCacheBuilder.newBuilder()
@@ -208,8 +207,7 @@ public TrinoGlueCatalog(
             GlueMetastoreStats stats,
             boolean isUsingSystemSecurity,
             Optional<String> defaultSchemaLocation,
-            boolean useUniqueTableLocation,
-            boolean hideMaterializedViewStorageTable)
+            boolean useUniqueTableLocation)
     {
         super(catalogName, typeManager, tableOperationsProvider, fileSystemFactory, useUniqueTableLocation);
         this.trinoVersion = requireNonNull(trinoVersion, "trinoVersion is null");
@@ -220,7 +218,6 @@ public TrinoGlueCatalog(
         this.stats = requireNonNull(stats, "stats is null");
         this.isUsingSystemSecurity = isUsingSystemSecurity;
         this.defaultSchemaLocation = requireNonNull(defaultSchemaLocation, "defaultSchemaLocation is null");
-        this.hideMaterializedViewStorageTable = hideMaterializedViewStorageTable;
     }
 
     @Override
@@ -1123,7 +1120,7 @@ public void createMaterializedView(
             }
         }
 
-        if (hideMaterializedViewStorageTable) {
+        if (!isSeparateStorageTable(materializedViewProperties)) {
             Location storageMetadataLocation = createMaterializedViewStorage(session, viewName, definition, materializedViewProperties);
             TableInput materializedViewTableInput = getMaterializedViewTableInput(
                     viewName.getTableName(),
@@ -1253,8 +1250,7 @@ private void dropMaterializedViewStorage(ConnectorSession session, com.amazonaws
         Map<String, String> parameters = getTableParameters(view);
         String storageTableName = parameters.get(STORAGE_TABLE);
         if (storageTableName != null) {
-            String storageSchema = Optional.ofNullable(parameters.get(STORAGE_SCHEMA))
-                    .orElse(view.getDatabaseName());
+            String storageSchema = view.getDatabaseName();
             try {
                 dropTable(session, new SchemaTableName(storageSchema, storageTableName));
             }
@@ -1314,8 +1310,7 @@ private ConnectorMaterializedViewDefinition createMaterializedViewDefinition(
         }
 
         if (storageTable != null) {
-            String storageSchema = Optional.ofNullable(materializedViewParameters.get(STORAGE_SCHEMA))
-                    .orElse(viewName.getSchemaName());
+            String storageSchema = viewName.getSchemaName();
             SchemaTableName storageTableName = new SchemaTableName(storageSchema, storageTable);
 
             String viewOriginalText = table.getViewOriginalText();

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

@@ -47,7 +47,6 @@ public class TrinoGlueCatalogFactory
     private final Optional<String> defaultSchemaLocation;
     private final AWSGlueAsync glueClient;
     private final boolean isUniqueTableLocation;
-    private final boolean hideMaterializedViewStorageTable;
     private final GlueMetastoreStats stats;
     private final boolean isUsingSystemSecurity;
 
@@ -74,7 +73,6 @@ public TrinoGlueCatalogFactory(
         this.defaultSchemaLocation = glueConfig.getDefaultWarehouseDir();
         this.glueClient = requireNonNull(glueClient, "glueClient is null");
         this.isUniqueTableLocation = icebergConfig.isUniqueTableLocation();
-        this.hideMaterializedViewStorageTable = icebergConfig.isHideMaterializedViewStorageTable();
         this.stats = requireNonNull(stats, "stats is null");
         this.isUsingSystemSecurity = securityConfig.getSecuritySystem() == SYSTEM;
     }
@@ -100,7 +98,6 @@ public TrinoCatalog create(ConnectorIdentity identity)
                 stats,
                 isUsingSystemSecurity,
                 defaultSchemaLocation,
-                isUniqueTableLocation,
-                hideMaterializedViewStorageTable);
+                isUniqueTableLocation);
     }
 }