Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions docs/src/main/sphinx/connector/iceberg.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don’t add this. We don’t want special code just for testing. We’re removing this pattern from the code base.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this shouldn't be here, and should eventually move to test code. I didn't see the urgency to do this within one PR. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think we should add more test-only annotations to production code. This is an anti-pattern and we don’t want people to see this and think it is ok.

We’re keeping legacy code to create these views, so it’s not clear how much this change is helping at this stage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I could save you some time on your next review round, if you gave me a hint how you prefer want to see this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we create the legacy view manually in the test code? Or maybe we start an old version of Trino in Testcontainers to write the legacy view?

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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");
Expand All @@ -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
Expand Down Expand Up @@ -1123,7 +1120,7 @@ public void createMaterializedView(
}
}

if (hideMaterializedViewStorageTable) {
if (!isSeparateStorageTable(materializedViewProperties)) {
Location storageMetadataLocation = createMaterializedViewStorage(session, viewName, definition, materializedViewProperties);
TableInput materializedViewTableInput = getMaterializedViewTableInput(
viewName.getTableName(),
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
Expand All @@ -100,7 +98,6 @@ public TrinoCatalog create(ConnectorIdentity identity)
stats,
isUsingSystemSecurity,
defaultSchemaLocation,
isUniqueTableLocation,
hideMaterializedViewStorageTable);
isUniqueTableLocation);
}
}
Loading