diff --git a/core/trino-main/src/main/java/io/trino/connector/system/AbstractPropertiesSystemTable.java b/core/trino-main/src/main/java/io/trino/connector/system/AbstractPropertiesSystemTable.java index 0937996f156e..671a4515dd29 100644 --- a/core/trino-main/src/main/java/io/trino/connector/system/AbstractPropertiesSystemTable.java +++ b/core/trino-main/src/main/java/io/trino/connector/system/AbstractPropertiesSystemTable.java @@ -13,7 +13,6 @@ */ package io.trino.connector.system; -import com.google.common.collect.ImmutableMap; import io.trino.connector.CatalogName; import io.trino.spi.connector.ConnectorSession; import io.trino.spi.connector.ConnectorTableMetadata; @@ -27,13 +26,13 @@ import io.trino.transaction.TransactionId; import io.trino.transaction.TransactionManager; -import java.util.Map; -import java.util.Map.Entry; -import java.util.TreeMap; -import java.util.function.Supplier; -import java.util.stream.Collectors; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.function.Function; import static com.google.common.base.MoreObjects.firstNonNull; +import static com.google.common.collect.ImmutableList.toImmutableList; import static io.trino.metadata.MetadataUtil.TableMetadataBuilder.tableMetadataBuilder; import static io.trino.spi.connector.SystemTable.Distribution.SINGLE_COORDINATOR; import static io.trino.spi.type.VarcharType.createUnboundedVarcharType; @@ -44,9 +43,9 @@ abstract class AbstractPropertiesSystemTable { private final ConnectorTableMetadata tableMetadata; private final TransactionManager transactionManager; - private final Supplier>>> propertySupplier; + private final Function>> catalogProperties; - protected AbstractPropertiesSystemTable(String tableName, TransactionManager transactionManager, Supplier>>> propertySupplier) + protected AbstractPropertiesSystemTable(String tableName, TransactionManager transactionManager, Function>> catalogProperties) { this.tableMetadata = tableMetadataBuilder(new SchemaTableName("metadata", tableName)) .column("catalog_name", createUnboundedVarcharType()) @@ -56,7 +55,7 @@ protected AbstractPropertiesSystemTable(String tableName, TransactionManager tra .column("description", createUnboundedVarcharType()) .build(); this.transactionManager = requireNonNull(transactionManager, "transactionManager is null"); - this.propertySupplier = requireNonNull(propertySupplier, "propertySupplier is null"); + this.catalogProperties = requireNonNull(catalogProperties, "catalogProperties is null"); } @Override @@ -77,23 +76,22 @@ public final RecordCursor cursor(ConnectorTransactionHandle transactionHandle, C TransactionId transactionId = ((GlobalSystemTransactionHandle) transactionHandle).getTransactionId(); InMemoryRecordSet.Builder table = InMemoryRecordSet.builder(tableMetadata); - Map>> connectorProperties = propertySupplier.get(); - Map catalogNames = transactionManager.getCatalogs(transactionId).entrySet() - .stream() - .collect(Collectors.toMap( - Entry::getKey, - entry -> entry.getValue().getConnectorCatalogName())); - for (Entry entry : new TreeMap<>(catalogNames).entrySet()) { - String catalog = entry.getKey(); - Map> properties = new TreeMap<>(connectorProperties.getOrDefault(entry.getValue(), ImmutableMap.of())); - for (PropertyMetadata propertyMetadata : properties.values()) { - table.addRow( - catalog, - propertyMetadata.getName(), - firstNonNull(propertyMetadata.getDefaultValue(), "").toString(), - propertyMetadata.getSqlType().toString(), - propertyMetadata.getDescription()); - } + + List catalogNames = transactionManager.getCatalogs(transactionId).keySet().stream() + .sorted() + .map(CatalogName::new) + .collect(toImmutableList()); + + for (CatalogName catalogName : catalogNames) { + catalogProperties.apply(catalogName).stream() + .sorted(Comparator.comparing(PropertyMetadata::getName)) + .forEach(propertyMetadata -> + table.addRow( + catalogName.toString(), + propertyMetadata.getName(), + firstNonNull(propertyMetadata.getDefaultValue(), "").toString(), + propertyMetadata.getSqlType().toString(), + propertyMetadata.getDescription())); } return table.build().cursor(); } diff --git a/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java b/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java index c22436058c44..e8789c36e160 100644 --- a/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java @@ -118,12 +118,12 @@ public ListenableFuture execute( } Map columnProperties = columnPropertyManager.getProperties( catalogName, - tableName.getCatalogName(), element.getProperties(), session, plannerContext, accessControl, - parameterExtractor(statement, parameters)); + parameterExtractor(statement, parameters), + true); ColumnMetadata column = ColumnMetadata.builder() .setName(element.getName().getValue()) diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java index fbc310a6539b..47b59dd50775 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java @@ -104,12 +104,12 @@ public ListenableFuture execute( Map properties = materializedViewPropertyManager.getProperties( catalogName, - name.getCatalogName(), statement.getProperties(), session, plannerContext, accessControl, - parameterLookup); + parameterLookup, + true); MaterializedViewDefinition definition = new MaterializedViewDefinition( sql, diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java index a4f79284333f..acaf2bbcee88 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java @@ -103,12 +103,12 @@ static ListenableFuture internalExecute( Map properties = schemaPropertyManager.getProperties( catalogName, - schema.getCatalogName(), statement.getProperties(), session, plannerContext, accessControl, - parameterExtractor(statement, parameters)); + parameterExtractor(statement, parameters), + true); TrinoPrincipal principal = getCreatePrincipal(statement, session, plannerContext.getMetadata(), catalogName.getCatalogName()); try { diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java index 5d8b549dc234..73fb10c14af2 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java @@ -164,12 +164,12 @@ ListenableFuture internalExecute(CreateTable statement, Session session, L } Map columnProperties = columnPropertyManager.getProperties( catalogName, - tableName.getCatalogName(), column.getProperties(), session, plannerContext, accessControl, - parameterLookup); + parameterLookup, + true); columns.put(name, ColumnMetadata.builder() .setName(name) @@ -245,12 +245,12 @@ else if (element instanceof LikeClause) { } Map properties = tablePropertyManager.getProperties( catalogName, - tableName.getCatalogName(), statement.getProperties(), session, plannerContext, accessControl, - parameterLookup); + parameterLookup, + true); if (!disableSetPropertiesSecurityCheckForCreateDdl) { accessControl.checkCanCreateTable(session.toSecurityContext(), tableName, properties); diff --git a/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java b/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java index a494a4c1edc2..ba6da232bbec 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java @@ -17,7 +17,6 @@ import io.trino.Session; import io.trino.execution.warnings.WarningCollector; import io.trino.metadata.MaterializedViewPropertyManager; -import io.trino.metadata.Properties; import io.trino.metadata.QualifiedObjectName; import io.trino.metadata.TableHandle; import io.trino.metadata.TablePropertyManager; @@ -29,8 +28,13 @@ import javax.inject.Inject; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; +import java.util.Set; +import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; @@ -77,25 +81,25 @@ public ListenableFuture execute( QualifiedObjectName objectName = createQualifiedObjectName(session, statement, statement.getName()); if (statement.getType() == TABLE) { - Properties properties = tablePropertyManager.getOnlySpecifiedProperties( + Map> properties = tablePropertyManager.getNullableProperties( getRequiredCatalogHandle(plannerContext.getMetadata(), session, statement, objectName.getCatalogName()), - objectName.getCatalogName(), statement.getProperties(), session, plannerContext, accessControl, - parameterExtractor(statement, parameters)); + parameterExtractor(statement, parameters), + false); setTableProperties(statement, objectName, session, properties); } else if (statement.getType() == MATERIALIZED_VIEW) { - Properties properties = materializedViewPropertyManager.getOnlySpecifiedProperties( + Map> properties = materializedViewPropertyManager.getNullableProperties( getRequiredCatalogHandle(plannerContext.getMetadata(), session, statement, objectName.getCatalogName()), - objectName.getCatalogName(), statement.getProperties(), session, plannerContext, accessControl, - parameterExtractor(statement, parameters)); + parameterExtractor(statement, parameters), + false); setMaterializedViewProperties(statement, objectName, session, properties); } else { @@ -105,7 +109,7 @@ else if (statement.getType() == MATERIALIZED_VIEW) { return immediateVoidFuture(); } - private void setTableProperties(SetProperties statement, QualifiedObjectName tableName, Session session, Properties properties) + private void setTableProperties(SetProperties statement, QualifiedObjectName tableName, Session session, Map> properties) { if (plannerContext.getMetadata().isMaterializedView(session, tableName)) { throw semanticException(NOT_SUPPORTED, statement, "Cannot set properties to a materialized view in ALTER TABLE"); @@ -120,16 +124,16 @@ private void setTableProperties(SetProperties statement, QualifiedObjectName tab throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: %s", tableName); } - accessControl.checkCanSetTableProperties(session.toSecurityContext(), tableName, properties.getNonNullProperties(), properties.getNullPropertyNames()); + accessControl.checkCanSetTableProperties(session.toSecurityContext(), tableName, getNonNullProperties(properties), getNullProperties(properties)); - plannerContext.getMetadata().setTableProperties(session, tableHandle.get(), properties.getNonNullProperties(), properties.getNullPropertyNames()); + plannerContext.getMetadata().setTableProperties(session, tableHandle.get(), getNonNullProperties(properties), getNullProperties(properties)); } private void setMaterializedViewProperties( SetProperties statement, QualifiedObjectName materializedViewName, Session session, - Properties properties) + Map> properties) { if (plannerContext.getMetadata().getMaterializedView(session, materializedViewName).isEmpty()) { String exceptionMessage = format("Materialized View '%s' does not exist", materializedViewName); @@ -141,7 +145,22 @@ else if (plannerContext.getMetadata().getTableHandle(session, materializedViewNa } throw semanticException(TABLE_NOT_FOUND, statement, exceptionMessage); } - accessControl.checkCanSetMaterializedViewProperties(session.toSecurityContext(), materializedViewName, properties.getNonNullProperties(), properties.getNullPropertyNames()); - plannerContext.getMetadata().setMaterializedViewProperties(session, materializedViewName, properties.getNonNullProperties(), properties.getNullPropertyNames()); + accessControl.checkCanSetMaterializedViewProperties(session.toSecurityContext(), materializedViewName, getNonNullProperties(properties), getNullProperties(properties)); + plannerContext.getMetadata().setMaterializedViewProperties(session, materializedViewName, getNonNullProperties(properties), getNullProperties(properties)); + } + + private static Map getNonNullProperties(Map> propertyValues) + { + return propertyValues.entrySet().stream() + .filter(entry -> entry.getValue().isPresent()) + .collect(toImmutableMap(Entry::getKey, entry -> entry.getValue().orElseThrow())); + } + + private static Set getNullProperties(Map> propertyValues) + { + return propertyValues.entrySet().stream() + .filter(entry -> entry.getValue().isEmpty()) + .map(Entry::getKey) + .collect(toImmutableSet()); } } diff --git a/core/trino-main/src/main/java/io/trino/metadata/AbstractCatalogPropertyManager.java b/core/trino-main/src/main/java/io/trino/metadata/AbstractCatalogPropertyManager.java index a31b7dbdad86..0df88a99275f 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/AbstractCatalogPropertyManager.java +++ b/core/trino-main/src/main/java/io/trino/metadata/AbstractCatalogPropertyManager.java @@ -13,10 +13,13 @@ */ package io.trino.metadata; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Maps; import io.trino.Session; import io.trino.connector.CatalogName; import io.trino.security.AccessControl; import io.trino.spi.ErrorCodeSupplier; +import io.trino.spi.TrinoException; import io.trino.spi.session.PropertyMetadata; import io.trino.sql.PlannerContext; import io.trino.sql.tree.Expression; @@ -24,88 +27,100 @@ import io.trino.sql.tree.Parameter; import io.trino.sql.tree.Property; +import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Map.Entry; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static io.trino.metadata.PropertyUtil.evaluateProperties; +import static io.trino.spi.StandardErrorCode.NOT_FOUND; +import static java.lang.String.format; +import static java.util.Objects.requireNonNull; abstract class AbstractCatalogPropertyManager - extends AbstractPropertyManager { + private final ConcurrentMap>> connectorProperties = new ConcurrentHashMap<>(); + private final String propertyType; + private final ErrorCodeSupplier propertyError; + protected AbstractCatalogPropertyManager(String propertyType, ErrorCodeSupplier propertyError) { - super(propertyType, propertyError); + this.propertyType = propertyType; + this.propertyError = requireNonNull(propertyError, "propertyError is null"); } public void addProperties(CatalogName catalogName, List> properties) { - doAddProperties(catalogName, properties); + requireNonNull(catalogName, "catalogName is null"); + requireNonNull(properties, "properties is null"); + + Map> propertiesByName = Maps.uniqueIndex(properties, PropertyMetadata::getName); + + checkState(connectorProperties.putIfAbsent(catalogName, propertiesByName) == null, "Properties for key %s are already registered", catalogName); } public void removeProperties(CatalogName catalogName) { - doRemoveProperties(catalogName); + connectorProperties.remove(catalogName); } - /** - * Evaluate {@code properties}. The returned {@code Map<String, Object>} contains a supported property iff its value is - * (implicitly or explictly) set to a non-{@code null} value. Thus, - *
    - *
  • If a property does not appear in the result, then its value is {@code null}
    • - *
  • If a {@code Property} in {@code properties} is set to DEFAULT, then it will appear in the result iff the default value is not - * {@code null}.
    • - *
  • If a property does not appear in {@code properties} at all, then it will appear in the result iff its default value is not - * {@code null}. (Thus, specifying a property to have the DEFAULT value is the same as not specifying it at all.)
    • - *
- */ public Map getProperties( CatalogName catalog, - String catalogNameForDiagnostics, Iterable properties, Session session, PlannerContext plannerContext, AccessControl accessControl, - Map, Expression> parameters) + Map, Expression> parameters, + boolean includeAllProperties) { - return doGetProperties( + Map> nullableValues = getNullableProperties( catalog, - catalogNameForDiagnostics, properties, session, plannerContext, accessControl, - parameters); + parameters, + includeAllProperties); + return nullableValues.entrySet().stream() + .filter(entry -> entry.getValue().isPresent()) + .collect(toImmutableMap(Entry::getKey, entry -> entry.getValue().orElseThrow())); } - /** - * Evaluate {@code properties}. Unlike {@link #getProperties}, a property appears in the returned result iff it is specified in - * {@code properties}. - */ - public Properties getOnlySpecifiedProperties( + public Map> getNullableProperties( CatalogName catalog, - String catalogNameForDiagnostics, Iterable properties, Session session, PlannerContext plannerContext, AccessControl accessControl, - Map, Expression> parameters) + Map, Expression> parameters, + boolean includeAllProperties) { - return doGetOnlySpecifiedProperties( - catalog, - catalogNameForDiagnostics, + Map> propertyMetadata = connectorProperties.get(catalog); + if (propertyMetadata == null) { + throw new TrinoException(NOT_FOUND, format("Catalog '%s' %s property not found", catalog, propertyType)); + } + + return evaluateProperties( properties, session, plannerContext, accessControl, - parameters); - } - - public Map>> getAllProperties() - { - return doGetAllProperties(); + parameters, + includeAllProperties, + propertyMetadata, + propertyError, + format("catalog '%s' %s property", catalog, propertyType)); } - @Override - protected String formatPropertiesKeyForMessage(String catalogName, CatalogName ignored) + public Collection> getAllProperties(CatalogName catalogName) { - return "Catalog '" + catalogName + "'"; + return Optional.ofNullable(connectorProperties.get(catalogName)) + .map(Map::values) + .orElse(ImmutableList.of()); } } diff --git a/core/trino-main/src/main/java/io/trino/metadata/AbstractPropertyManager.java b/core/trino-main/src/main/java/io/trino/metadata/AbstractPropertyManager.java deleted file mode 100644 index 3812083fe1d8..000000000000 --- a/core/trino-main/src/main/java/io/trino/metadata/AbstractPropertyManager.java +++ /dev/null @@ -1,355 +0,0 @@ -/* - * 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.metadata; - -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Maps; -import io.trino.Session; -import io.trino.security.AccessControl; -import io.trino.spi.ErrorCodeSupplier; -import io.trino.spi.TrinoException; -import io.trino.spi.block.BlockBuilder; -import io.trino.spi.session.PropertyMetadata; -import io.trino.spi.type.Type; -import io.trino.sql.PlannerContext; -import io.trino.sql.planner.ParameterRewriter; -import io.trino.sql.tree.Expression; -import io.trino.sql.tree.ExpressionTreeRewriter; -import io.trino.sql.tree.NodeRef; -import io.trino.sql.tree.Parameter; -import io.trino.sql.tree.Property; - -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; - -import static com.google.common.base.Preconditions.checkState; -import static io.trino.spi.StandardErrorCode.DUPLICATE_PROPERTY; -import static io.trino.spi.StandardErrorCode.NOT_FOUND; -import static io.trino.spi.type.TypeUtils.writeNativeValue; -import static io.trino.sql.planner.ExpressionInterpreter.evaluateConstantExpression; -import static java.lang.String.format; -import static java.util.Locale.ENGLISH; -import static java.util.Objects.requireNonNull; - -abstract class AbstractPropertyManager -{ - protected final ConcurrentMap>> connectorProperties = new ConcurrentHashMap<>(); - private final String propertyType; - private final ErrorCodeSupplier propertyError; - - protected AbstractPropertyManager(String propertyType, ErrorCodeSupplier propertyError) - { - requireNonNull(propertyType, "propertyType is null"); - this.propertyType = propertyType; - this.propertyError = requireNonNull(propertyError, "propertyError is null"); - } - - protected final void doAddProperties(K propertiesKey, List> properties) - { - requireNonNull(propertiesKey, "propertiesKey is null"); - requireNonNull(properties, "properties is null"); - - Map> propertiesByName = Maps.uniqueIndex(properties, PropertyMetadata::getName); - - checkState(connectorProperties.putIfAbsent(propertiesKey, propertiesByName) == null, "Properties for key %s are already registered", propertiesKey); - } - - protected final void doRemoveProperties(K propertiesKey) - { - connectorProperties.remove(propertiesKey); - } - - protected final Properties doGetOnlySpecifiedProperties( - K propertiesKey, - String catalogNameForDiagnostics, - Iterable properties, - Session session, - PlannerContext plannerContext, - AccessControl accessControl, - Map, Expression> parameters) - { - Map> supportedPropertiesMetadata = getSupportedPropertiesMetadata(propertiesKey, catalogNameForDiagnostics); - - SeparatedProperties separatedProperties = separateProperties(propertiesKey, catalogNameForDiagnostics, properties); - - // builder for properties of which the values (after being evaluated or looked up) are non-null - ImmutableMap.Builder nonNullPropertiesBuilder; - - // evaluate the value expressions of the properties that are not set to DEFAULT (the results are necessarily non-null) - nonNullPropertiesBuilder = - evaluateSqlProperties( - propertiesKey, - supportedPropertiesMetadata, - catalogNameForDiagnostics, - separatedProperties.nonDefaultProperties, - session, - plannerContext, - accessControl, - parameters); - - // builder for names of properties of which the values are null - ImmutableSet.Builder nullPropertyNamesBuilder = ImmutableSet.builder(); - - // for each property set to DEFAULT, look up its default value; if the default value is non-null, then put the property into - // nonNullPropertiesBuilder, otherwise put its name into nullPropertyNamesBuilder - for (String name : separatedProperties.defaultPropertyNames) { - PropertyMetadata propertyMetadata = supportedPropertiesMetadata.get(name); - if (propertyMetadata == null) { - throw new TrinoException( - propertyError, - format("%s does not support %s property '%s'", - formatPropertiesKeyForMessage(catalogNameForDiagnostics, propertiesKey), - propertyType, - name)); - } - Object value = propertyMetadata.getDefaultValue(); - if (value != null) { - nonNullPropertiesBuilder.put(name, value); - } - else { - nullPropertyNamesBuilder.add(name); - } - } - - return new Properties(nonNullPropertiesBuilder.build(), nullPropertyNamesBuilder.build()); - } - - protected final Map doGetProperties( - K propertiesKey, - String catalogNameForDiagnostics, - Iterable properties, - Session session, - PlannerContext plannerContext, - AccessControl accessControl, - Map, Expression> parameters) - { - Map> supportedPropertiesMetadata = getSupportedPropertiesMetadata(propertiesKey, catalogNameForDiagnostics); - SeparatedProperties separatedProperties = separateProperties(propertiesKey, catalogNameForDiagnostics, properties); - - // check that all the properties that are set to DEFAULT are supported - for (String name : separatedProperties.defaultPropertyNames) { - if (!supportedPropertiesMetadata.containsKey(name)) { - throw new TrinoException( - propertyError, - format("%s does not support %s property '%s'", - formatPropertiesKeyForMessage(catalogNameForDiagnostics, propertiesKey), - propertyType, - name)); - } - } - - // call the version of doGetProperties that takes properties as a Map to evaluate the properties that are not - // set to DEFAULT (the properties that are set to DEFAULT (and any properties that are not specified) will be filled in if their - // default values are not null) - return doGetProperties( - propertiesKey, - catalogNameForDiagnostics, - separatedProperties.nonDefaultProperties, - session, - plannerContext, - accessControl, - parameters); - } - - protected final Map doGetProperties( - K propertiesKey, - String catalogNameForDiagnostics, - Map sqlProperties, - Session session, - PlannerContext plannerContext, - AccessControl accessControl, - Map, Expression> parameters) - { - Map> supportedPropertiesMetadata = getSupportedPropertiesMetadata(propertiesKey, catalogNameForDiagnostics); - ImmutableMap.Builder propertiesBuilder = - evaluateSqlProperties( - propertiesKey, - supportedPropertiesMetadata, - catalogNameForDiagnostics, - sqlProperties, - session, - plannerContext, - accessControl, - parameters); - - Map specifiedProperties = propertiesBuilder.build(); - - // Fill in the remaining properties with non-null defaults - for (PropertyMetadata propertyMetadata : supportedPropertiesMetadata.values()) { - if (!specifiedProperties.containsKey(propertyMetadata.getName())) { - Object value = propertyMetadata.getDefaultValue(); - if (value != null) { - propertiesBuilder.put(propertyMetadata.getName(), value); - } - } - } - - return propertiesBuilder.build(); - } - - protected final Map>> doGetAllProperties() - { - return ImmutableMap.copyOf(connectorProperties); - } - - /** - * Separates the {@link Property}'s in {@code properties} based on whether they are set to DEFAULT or not - */ - private SeparatedProperties separateProperties( - K propertiesKey, - String catalogNameForDiagnostics, - Iterable properties) - { - Map nonDefaultProperties = new HashMap<>(); - Set defaultPropertyNames = new HashSet<>(); - for (Property property : properties) { - String name = property.getName().getValue().toLowerCase(ENGLISH); // property names are case-insensitive and normalized to lower case - if (nonDefaultProperties.containsKey(name) || defaultPropertyNames.contains(name)) { - throw new TrinoException( - DUPLICATE_PROPERTY, - format("%s %s property '%s' is specified more than once", - formatPropertiesKeyForMessage(catalogNameForDiagnostics, propertiesKey), - propertyType, - name)); - } - if (!property.isSetToDefault()) { - nonDefaultProperties.put(name, property.getNonDefaultValue()); - } - else { - defaultPropertyNames.add(name); - } - } - return new SeparatedProperties(nonDefaultProperties, defaultPropertyNames); - } - - /** - * Represents the result of separating a collection of {@link Property}'s based on whether they are set to DEFAULT or not. This is the - * return type of {@link #separateProperties}. - */ - private static class SeparatedProperties - { - final Map nonDefaultProperties; - final Set defaultPropertyNames; - - SeparatedProperties(Map nonDefaultProperties, Set defaultPropertyNames) - { - this.nonDefaultProperties = ImmutableMap.copyOf(requireNonNull(nonDefaultProperties, "nonDefaultProperties is null")); - this.defaultPropertyNames = ImmutableSet.copyOf(requireNonNull(defaultPropertyNames, "defaultPropertyNames is null")); - } - } - - private Map> getSupportedPropertiesMetadata(K propertiesKey, String catalogNameForDiagnostics) - { - Map> supportedPropertiesMetadata = connectorProperties.get(propertiesKey); - if (supportedPropertiesMetadata == null) { - throw new TrinoException(NOT_FOUND, formatPropertiesKeyForMessage(catalogNameForDiagnostics, propertiesKey) + " not found"); - } - return supportedPropertiesMetadata; - } - - /** - * Evaluates {@code sqlProperties}. The results are decoded Java objects and are put into an {@code ImmutableMap.Builder} - * (as opposed to an {@code ImmutableMap} so that a caller has the option of adding more entries to the {@code Builder} - * before building an {@code ImmutableMap}). - */ - private ImmutableMap.Builder evaluateSqlProperties( - K propertiesKey, - Map> supportedPropertiesMetadata, - String catalogNameForDiagnostics, - Map sqlProperties, - Session session, - PlannerContext plannerContext, - AccessControl accessControl, - Map, Expression> parameters) - { - ImmutableMap.Builder propertiesBuilder = ImmutableMap.builder(); - for (Map.Entry sqlProperty : sqlProperties.entrySet()) { - String propertyName = sqlProperty.getKey().toLowerCase(ENGLISH); // property names are case-insensitive and normalized to lower case - PropertyMetadata propertyMetadata = supportedPropertiesMetadata.get(propertyName); - if (propertyMetadata == null) { - throw new TrinoException( - propertyError, - format("%s does not support %s property '%s'", - formatPropertiesKeyForMessage(catalogNameForDiagnostics, propertiesKey), - propertyType, - propertyName)); - } - - Object sqlObjectValue; - try { - sqlObjectValue = evaluatePropertyValue(sqlProperty.getValue(), propertyMetadata.getSqlType(), session, plannerContext, accessControl, parameters); - } - catch (TrinoException e) { - throw new TrinoException( - propertyError, - format("Invalid value for %s property '%s': Cannot convert [%s] to %s", - propertyType, - propertyMetadata.getName(), - sqlProperty.getValue(), - propertyMetadata.getSqlType()), - e); - } - - Object value; - try { - value = propertyMetadata.decode(sqlObjectValue); - } - catch (Exception e) { - throw new TrinoException( - propertyError, - format( - "Unable to set %s property '%s' to [%s]: %s", - propertyType, - propertyMetadata.getName(), - sqlProperty.getValue(), - e.getMessage()), - e); - } - - propertiesBuilder.put(propertyMetadata.getName(), value); - } - - return propertiesBuilder; - } - - private Object evaluatePropertyValue( - Expression expression, - Type expectedType, - Session session, - PlannerContext plannerContext, - AccessControl accessControl, - Map, Expression> parameters) - { - Expression rewritten = ExpressionTreeRewriter.rewriteWith(new ParameterRewriter(parameters), expression); - Object value = evaluateConstantExpression(rewritten, expectedType, plannerContext, session, accessControl, parameters); - - // convert to object value type of SQL type - BlockBuilder blockBuilder = expectedType.createBlockBuilder(null, 1); - writeNativeValue(expectedType, blockBuilder, value); - Object objectValue = expectedType.getObjectValue(session.toConnectorSession(), blockBuilder, 0); - - if (objectValue == null) { - throw new TrinoException(propertyError, format("Invalid null value for %s property", propertyType)); - } - return objectValue; - } - - protected abstract String formatPropertiesKeyForMessage(String catalogName, K propertiesKey); -} diff --git a/core/trino-main/src/main/java/io/trino/metadata/Properties.java b/core/trino-main/src/main/java/io/trino/metadata/Properties.java deleted file mode 100644 index 2588ff4f986e..000000000000 --- a/core/trino-main/src/main/java/io/trino/metadata/Properties.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * 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.metadata; - -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; - -import java.util.Map; -import java.util.Set; - -import static java.util.Objects.requireNonNull; - -public class Properties -{ - private final Map nonNullProperties; // properties whose values are non-null - private final Set nullPropertyNames; // names of properties whose values are null - - Properties(Map nonNullProperties, Set nullPropertyNames) - { - requireNonNull(nonNullProperties, "nonNullProperties is null"); - requireNonNull(nullPropertyNames, "nullPropertyNames is null"); - this.nonNullProperties = ImmutableMap.copyOf(nonNullProperties); - this.nullPropertyNames = ImmutableSet.copyOf(nullPropertyNames); - } - - public Map getNonNullProperties() - { - return nonNullProperties; - } - - public Set getNullPropertyNames() - { - return nullPropertyNames; - } -} diff --git a/core/trino-main/src/main/java/io/trino/metadata/PropertyUtil.java b/core/trino-main/src/main/java/io/trino/metadata/PropertyUtil.java new file mode 100644 index 000000000000..b75f067277ed --- /dev/null +++ b/core/trino-main/src/main/java/io/trino/metadata/PropertyUtil.java @@ -0,0 +1,159 @@ +/* + * 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.metadata; + +import com.google.common.collect.ImmutableMap; +import io.trino.Session; +import io.trino.security.AccessControl; +import io.trino.spi.ErrorCodeSupplier; +import io.trino.spi.TrinoException; +import io.trino.spi.block.BlockBuilder; +import io.trino.spi.session.PropertyMetadata; +import io.trino.spi.type.Type; +import io.trino.sql.PlannerContext; +import io.trino.sql.planner.ParameterRewriter; +import io.trino.sql.tree.Expression; +import io.trino.sql.tree.ExpressionTreeRewriter; +import io.trino.sql.tree.NodeRef; +import io.trino.sql.tree.Parameter; +import io.trino.sql.tree.Property; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Optional; + +import static io.trino.spi.type.TypeUtils.writeNativeValue; +import static io.trino.sql.planner.ExpressionInterpreter.evaluateConstantExpression; +import static java.lang.String.format; +import static java.util.Locale.ENGLISH; + +public final class PropertyUtil +{ + private PropertyUtil() {} + + public static Map> evaluateProperties( + Iterable setProperties, + Session session, + PlannerContext plannerContext, + AccessControl accessControl, + Map, Expression> parameters, + boolean includeAllProperties, + Map> metadata, + ErrorCodeSupplier errorCode, + String propertyTypeDescription) + { + Map> propertyValues = new LinkedHashMap<>(); + + // Fill in user-specified properties + for (Property property : setProperties) { + // property names are case-insensitive and normalized to lower case + String propertyName = property.getName().getValue().toLowerCase(ENGLISH); + PropertyMetadata propertyMetadata = metadata.get(propertyName); + if (propertyMetadata == null) { + throw new TrinoException(errorCode, format("%s '%s' does not exist", capitalize(propertyTypeDescription), propertyName)); + } + + Optional value; + if (property.isSetToDefault()) { + value = Optional.ofNullable(propertyMetadata.getDefaultValue()); + } + else { + value = Optional.of(evaluateProperty( + property.getNonDefaultValue(), + propertyMetadata, + session, + plannerContext, + accessControl, + parameters, + errorCode, + propertyTypeDescription)); + } + + propertyValues.put(propertyMetadata.getName(), value); + } + + if (includeAllProperties) { + for (PropertyMetadata propertyMetadata : metadata.values()) { + if (!propertyValues.containsKey(propertyMetadata.getName())) { + propertyValues.put(propertyMetadata.getName(), Optional.ofNullable(propertyMetadata.getDefaultValue())); + } + } + } + return ImmutableMap.copyOf(propertyValues); + } + + private static Object evaluateProperty( + Expression expression, + PropertyMetadata property, + Session session, + PlannerContext plannerContext, + AccessControl accessControl, + Map, Expression> parameters, + ErrorCodeSupplier errorCode, + String propertyTypeDescription) + { + Object sqlObjectValue; + try { + Type expectedType = property.getSqlType(); + Expression rewritten = ExpressionTreeRewriter.rewriteWith(new ParameterRewriter(parameters), expression); + Object value = evaluateConstantExpression(rewritten, expectedType, plannerContext, session, accessControl, parameters); + + // convert to object value type of SQL type + BlockBuilder blockBuilder = expectedType.createBlockBuilder(null, 1); + writeNativeValue(expectedType, blockBuilder, value); + sqlObjectValue = expectedType.getObjectValue(session.toConnectorSession(), blockBuilder, 0); + } + catch (TrinoException e) { + throw new TrinoException( + errorCode, + format( + "Invalid value for %s '%s': Cannot convert [%s] to %s", + propertyTypeDescription, + property.getName(), + expression, + property.getSqlType()), + e); + } + + if (sqlObjectValue == null) { + throw new TrinoException( + errorCode, + format( + "Invalid null value for %s '%s' from [%s]", + propertyTypeDescription, + property.getName(), + expression)); + } + + try { + return property.decode(sqlObjectValue); + } + catch (Exception e) { + throw new TrinoException( + errorCode, + format( + "Unable to set %s '%s' to [%s]: %s", + propertyTypeDescription, + property.getName(), + expression, + e.getMessage()), + e); + } + } + + private static String capitalize(String value) + { + return Character.toUpperCase(value.charAt(0)) + value.substring(1); + } +} diff --git a/core/trino-main/src/main/java/io/trino/metadata/TableProceduresPropertyManager.java b/core/trino-main/src/main/java/io/trino/metadata/TableProceduresPropertyManager.java index 0006ebcd1348..7174b1c0e8ae 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/TableProceduresPropertyManager.java +++ b/core/trino-main/src/main/java/io/trino/metadata/TableProceduresPropertyManager.java @@ -13,36 +13,52 @@ */ package io.trino.metadata; +import com.google.common.collect.Maps; import io.trino.Session; import io.trino.connector.CatalogName; import io.trino.security.AccessControl; +import io.trino.spi.TrinoException; import io.trino.spi.session.PropertyMetadata; import io.trino.sql.PlannerContext; import io.trino.sql.tree.Expression; +import io.trino.sql.tree.Identifier; import io.trino.sql.tree.NodeRef; import io.trino.sql.tree.Parameter; +import io.trino.sql.tree.Property; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; +import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static io.trino.metadata.PropertyUtil.evaluateProperties; import static io.trino.spi.StandardErrorCode.INVALID_PROCEDURE_ARGUMENT; +import static io.trino.spi.StandardErrorCode.NOT_FOUND; import static java.lang.String.format; import static java.util.Objects.requireNonNull; public class TableProceduresPropertyManager - extends AbstractPropertyManager { - public TableProceduresPropertyManager() - { - super("procedure", INVALID_PROCEDURE_ARGUMENT); - } + private final ConcurrentMap>> connectorProperties = new ConcurrentHashMap<>(); public void addProperties(CatalogName catalogName, String procedureName, List> properties) { - doAddProperties(new Key(catalogName, procedureName), properties); + requireNonNull(catalogName, "catalogName is null"); + requireNonNull(procedureName, "procedureName is null"); + requireNonNull(catalogName, "catalogName is null"); + + Map> propertiesByName = Maps.uniqueIndex(properties, PropertyMetadata::getName); + + Key propertiesKey = new Key(catalogName, procedureName); + checkState(connectorProperties.putIfAbsent(propertiesKey, propertiesByName) == null, "Properties for key %s are already registered", propertiesKey); } public void removeProperties(CatalogName catalogName) @@ -51,39 +67,39 @@ public void removeProperties(CatalogName catalogName) .filter(key -> catalogName.equals(key.getCatalogName())) .collect(toImmutableSet()); for (Key key : keysToRemove) { - doRemoveProperties(key); + connectorProperties.remove(key); } } public Map getProperties( CatalogName catalog, String procedureName, - String catalogNameForDiagnostics, Map sqlPropertyValues, Session session, PlannerContext plannerContext, AccessControl accessControl, Map, Expression> parameters) { - return doGetProperties( - new Key(catalog, procedureName), - catalogNameForDiagnostics, - sqlPropertyValues, + Map> supportedProperties = connectorProperties.get(new Key(catalog, procedureName)); + if (supportedProperties == null) { + throw new TrinoException(NOT_FOUND, format("Catalog '%s' table procedure '%s' property not found", catalog, procedureName)); + } + + Map> propertyValues = evaluateProperties( + sqlPropertyValues.entrySet().stream() + .map(entry -> new Property(new Identifier(entry.getKey()), entry.getValue())) + .collect(toImmutableList()), session, plannerContext, accessControl, - parameters); - } - - public Map>> getAllProperties() - { - return doGetAllProperties(); - } - - @Override - protected String formatPropertiesKeyForMessage(String catalogName, Key propertiesKey) - { - return format("Catalog %s table procedure %s", catalogName, propertiesKey.procedureName); + parameters, + true, + supportedProperties, + INVALID_PROCEDURE_ARGUMENT, + format("catalog '%s' table procedure '%s' property", catalog, procedureName)); + return propertyValues.entrySet().stream() + .filter(entry -> entry.getValue().isPresent()) + .collect(toImmutableMap(Entry::getKey, entry -> entry.getValue().orElseThrow())); } static final class Key diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java index 2b2e3f6df732..e240ead0469b 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java @@ -753,12 +753,12 @@ protected Scope visitAnalyze(Analyze node, Optional scope) Map analyzeProperties = analyzePropertyManager.getProperties( catalogName, - catalogName.getCatalogName(), node.getProperties(), session, plannerContext, accessControl, - analysis.getParameters()); + analysis.getParameters(), + true); TableHandle tableHandle = metadata.getTableHandleForStatisticsCollection(session, tableName, analyzeProperties) .orElseThrow(() -> semanticException(TABLE_NOT_FOUND, node, "Table '%s' does not exist", tableName)); @@ -842,13 +842,12 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional properties = tablePropertyManager.getProperties( catalogName, - targetTable.getCatalogName(), node.getProperties(), session, plannerContext, accessControl, - analysis.getParameters()); - + analysis.getParameters(), + true); ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(targetTable.asSchemaTableName(), columns.build(), properties, node.getComment()); // analyze target table layout @@ -1084,7 +1083,6 @@ protected Scope visitTableExecute(TableExecute node, Optional scope) Map tableProperties = tableProceduresPropertyManager.getProperties( catalogName, procedureName, - catalogName.getCatalogName(), propertiesMap, session, plannerContext, diff --git a/core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java b/core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java index a6cdfe2db3d4..3021163521c7 100644 --- a/core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java +++ b/core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Lists; +import com.google.common.collect.Maps; import com.google.common.primitives.Primitives; import io.trino.Session; import io.trino.connector.CatalogName; @@ -91,6 +92,7 @@ import javax.inject.Inject; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -591,9 +593,7 @@ protected Node visitShowCreate(ShowCreate node, Void context) accessControl.checkCanShowCreateTable(session.toSecurityContext(), new QualifiedObjectName(catalogName.getValue(), schemaName.getValue(), tableName.getValue())); Map properties = viewDefinition.get().getProperties(); - Map> allMaterializedViewProperties = materializedViewPropertyManager - .getAllProperties() - .get(new CatalogName(catalogName.getValue())); + Collection> allMaterializedViewProperties = materializedViewPropertyManager.getAllProperties(new CatalogName(catalogName.getValue())); List propertyNodes = buildProperties(objectName, Optional.empty(), INVALID_MATERIALIZED_VIEW_PROPERTY, properties, allMaterializedViewProperties); String sql = formatSql(new CreateMaterializedView(Optional.empty(), QualifiedName.of(ImmutableList.of(catalogName, schemaName, tableName)), @@ -657,7 +657,7 @@ protected Node visitShowCreate(ShowCreate node, Void context) accessControl.checkCanShowCreateTable(session.toSecurityContext(), targetTableName); ConnectorTableMetadata connectorTableMetadata = metadata.getTableMetadata(session, tableHandle.get()).getMetadata(); - Map> allColumnProperties = columnPropertyManager.getAllProperties().get(tableHandle.get().getCatalogName()); + Collection> allColumnProperties = columnPropertyManager.getAllProperties(tableHandle.get().getCatalogName()); List columns = connectorTableMetadata.getColumns().stream() .filter(column -> !column.isHidden()) @@ -673,7 +673,7 @@ protected Node visitShowCreate(ShowCreate node, Void context) .collect(toImmutableList()); Map properties = connectorTableMetadata.getProperties(); - Map> allTableProperties = tablePropertyManager.getAllProperties().get(tableHandle.get().getCatalogName()); + Collection> allTableProperties = tablePropertyManager.getAllProperties(tableHandle.get().getCatalogName()); List propertyNodes = buildProperties(targetTableName, Optional.empty(), INVALID_TABLE_PROPERTY, properties, allTableProperties); CreateTable createTable = new CreateTable( @@ -695,7 +695,7 @@ protected Node visitShowCreate(ShowCreate node, Void context) accessControl.checkCanShowCreateSchema(session.toSecurityContext(), schemaName); Map properties = metadata.getSchemaProperties(session, schemaName); - Map> allTableProperties = schemaPropertyManager.getAllProperties().get(new CatalogName(schemaName.getCatalogName())); + Collection> allTableProperties = schemaPropertyManager.getAllProperties(new CatalogName(schemaName.getCatalogName())); QualifiedName qualifiedSchemaName = QualifiedName.of(schemaName.getCatalogName(), schemaName.getSchemaName()); List propertyNodes = buildProperties(qualifiedSchemaName, Optional.empty(), INVALID_SCHEMA_PROPERTY, properties, allTableProperties); @@ -717,12 +717,13 @@ private static List buildProperties( Optional columnName, StandardErrorCode errorCode, Map properties, - Map> allProperties) + Collection> allProperties) { if (properties.isEmpty()) { return Collections.emptyList(); } + Map> indexedPropertyMetadata = Maps.uniqueIndex(allProperties, PropertyMetadata::getName); ImmutableSortedMap.Builder sqlProperties = ImmutableSortedMap.naturalOrder(); for (Map.Entry propertyEntry : properties.entrySet()) { @@ -732,7 +733,7 @@ private static List buildProperties( throw new TrinoException(errorCode, format("Property %s for %s cannot have a null value", propertyName, toQualifiedName(objectName, columnName))); } - PropertyMetadata property = allProperties.get(propertyName); + PropertyMetadata property = indexedPropertyMetadata.get(propertyName); if (property == null) { throw new TrinoException(errorCode, "No PropertyMetadata for property: " + propertyName); } diff --git a/core/trino-main/src/test/java/io/trino/execution/TestCreateMaterializedViewTask.java b/core/trino-main/src/test/java/io/trino/execution/TestCreateMaterializedViewTask.java index 7c4ce26a562e..24c1dc799979 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestCreateMaterializedViewTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestCreateMaterializedViewTask.java @@ -192,7 +192,7 @@ public void testCreateMaterializedViewWithInvalidProperty() assertTrinoExceptionThrownBy(() -> getFutureValue(new CreateMaterializedViewTask(plannerContext, new AllowAllAccessControl(), parser, analyzerFactory, materializedViewPropertyManager, new FeaturesConfig()) .execute(statement, queryStateMachine, ImmutableList.of(), WarningCollector.NOOP))) .hasErrorCode(INVALID_MATERIALIZED_VIEW_PROPERTY) - .hasMessage("Catalog 'catalog' does not support materialized view property 'baz'"); + .hasMessage("Catalog 'catalog' materialized view property 'baz' does not exist"); assertEquals(metadata.getCreateMaterializedViewCallCount(), 0); } diff --git a/core/trino-main/src/test/java/io/trino/execution/TestCreateTableTask.java b/core/trino-main/src/test/java/io/trino/execution/TestCreateTableTask.java index 16cd2b6fc796..89e815843cae 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestCreateTableTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestCreateTableTask.java @@ -172,7 +172,7 @@ public void testCreateTableWithMaterializedViewPropertyFails() CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager, new FeaturesConfig()); assertTrinoExceptionThrownBy(() -> getFutureValue(createTableTask.internalExecute(statement, testSession, emptyList(), output -> {}))) .hasErrorCode(INVALID_TABLE_PROPERTY) - .hasMessage("Catalog 'catalog' does not support table property 'foo'"); + .hasMessage("Catalog 'catalog' table property 'foo' does not exist"); assertEquals(metadata.getCreateTableCallCount(), 0); } diff --git a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java index db5fc48fd677..b1f31a75147c 100644 --- a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java +++ b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java @@ -259,7 +259,8 @@ public void testDifferentEngine() assertUpdate("DROP TABLE " + tableName); //NOT support engine - assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'bad_engine')", "Unable to set table property 'engine' to.*"); + assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'bad_engine')", + "Unable to set catalog 'clickhouse' table property 'engine' to.*"); } /** @@ -318,15 +319,15 @@ public void testTableProperty() // Primary key must be a prefix of the sorting key, assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR NOT NULL, y VARCHAR NOT NULL) WITH (engine = 'MergeTree', order_by = ARRAY['id'], sample_by = ARRAY['x', 'y'])", - "Invalid value for table property 'sample_by': .*"); + "Invalid value for catalog 'clickhouse' table property 'sample_by': .*"); // wrong property type assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL) WITH (engine = 'MergeTree', order_by = 'id')", - "Invalid value for table property 'order_by': .*"); + "Invalid value for catalog 'clickhouse' table property 'order_by': .*"); assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL) WITH (engine = 'MergeTree', order_by = ARRAY['id'], primary_key = 'id')", - "Invalid value for table property 'primary_key': .*"); + "Invalid value for catalog 'clickhouse' table property 'primary_key': .*"); assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL) WITH (engine = 'MergeTree', order_by = ARRAY['id'], primary_key = ARRAY['id'], partition_by = 'id')", - "Invalid value for table property 'partition_by': .*"); + "Invalid value for catalog 'clickhouse' table property 'partition_by': .*"); } @Test @@ -365,7 +366,7 @@ public void testAlterInvalidTableProperties() "(p1 int NOT NULL, p2 int NOT NULL, x VARCHAR) WITH (engine = 'MergeTree', order_by = ARRAY['p1', 'p2'], primary_key = ARRAY['p1', 'p2'])")) { assertQueryFails( "ALTER TABLE " + table.getName() + " SET PROPERTIES invalid_property = 'p2'", - "Catalog 'clickhouse' does not support table property 'invalid_property'"); + "Catalog 'clickhouse' table property 'invalid_property' does not exist"); } } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java index 50623c9c2a83..7ae9e3b021ec 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java @@ -6426,9 +6426,9 @@ public void testInvalidAnalyzePartitionedTable() createPartitionedTableForAnalyzeTest(tableName); // Test invalid property - assertQueryFails(format("ANALYZE %s WITH (error = 1)", tableName), ".*'hive' does not support analyze property 'error'.*"); - assertQueryFails(format("ANALYZE %s WITH (partitions = 1)", tableName), "\\QInvalid value for analyze property 'partitions': Cannot convert [1] to array(array(varchar))\\E"); - assertQueryFails(format("ANALYZE %s WITH (partitions = NULL)", tableName), "\\QInvalid value for analyze property 'partitions': Cannot convert [null] to array(array(varchar))\\E"); + assertQueryFails(format("ANALYZE %s WITH (error = 1)", tableName), ".*'hive' analyze property 'error' does not exist.*"); + assertQueryFails(format("ANALYZE %s WITH (partitions = 1)", tableName), "\\QInvalid value for catalog 'hive' analyze property 'partitions': Cannot convert [1] to array(array(varchar))\\E"); + assertQueryFails(format("ANALYZE %s WITH (partitions = NULL)", tableName), "\\QInvalid null value for catalog 'hive' analyze property 'partitions' from [null]\\E"); assertQueryFails(format("ANALYZE %s WITH (partitions = ARRAY[NULL])", tableName), ".*Invalid null value in analyze partitions property.*"); // Test non-existed partition @@ -6941,7 +6941,7 @@ public void testInvalidColumnsAnalyzeTable() // Column names must be strings assertQueryFails( "ANALYZE " + tableName + " WITH (columns = ARRAY[42])", - "\\QInvalid value for analyze property 'columns': Cannot convert [ARRAY[42]] to array(varchar)\\E"); + "\\QInvalid value for catalog 'hive' analyze property 'columns': Cannot convert [ARRAY[42]] to array(varchar)\\E"); assertUpdate("DROP TABLE " + tableName); } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java index 407ea646a3df..caf4d6f29dd6 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java @@ -3279,10 +3279,10 @@ public void testOptimizeParameterValidation() "\\Qline 1:1: Table 'iceberg.tpch.no_such_table_exists' does not exist"); assertQueryFails( "ALTER TABLE nation EXECUTE OPTIMIZE (file_size_threshold => '33')", - "\\QUnable to set procedure property 'file_size_threshold' to ['33']: size is not a valid data size string: 33"); + "\\QUnable to set catalog 'iceberg' table procedure 'OPTIMIZE' property 'file_size_threshold' to ['33']: size is not a valid data size string: 33"); assertQueryFails( "ALTER TABLE nation EXECUTE OPTIMIZE (file_size_threshold => '33s')", - "\\QUnable to set procedure property 'file_size_threshold' to ['33s']: Unknown unit: s"); + "\\QUnable to set catalog 'iceberg' table procedure 'OPTIMIZE' property 'file_size_threshold' to ['33s']: Unknown unit: s"); } @Test diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMaterializedViews.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMaterializedViews.java index 5b9cab679b1b..3b90fc7bf7b6 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMaterializedViews.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMaterializedViews.java @@ -137,7 +137,7 @@ public void testCreateWithInvalidPropertyFails() assertThatThrownBy(() -> computeActual("CREATE MATERIALIZED VIEW materialized_view_with_property " + "WITH (invalid_property = ARRAY['_date']) AS " + "SELECT _bigint, _date FROM base_table1")) - .hasMessage("Catalog 'iceberg' does not support materialized view property 'invalid_property'"); + .hasMessage("Catalog 'iceberg' materialized view property 'invalid_property' does not exist"); } @Test diff --git a/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java b/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java index fad1fa6f4bff..8211e09a4bd2 100644 --- a/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java +++ b/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java @@ -207,7 +207,7 @@ public void testSchemaProperties() { assertUpdate("CREATE SCHEMA mock.test_schema WITH (boolean_schema_property = true)"); assertThatThrownBy(() -> assertUpdate("CREATE SCHEMA mock.test_schema WITH (unknown_property = true)")) - .hasMessage("Catalog 'mock' does not support schema property 'unknown_property'"); + .hasMessage("Catalog 'mock' schema property 'unknown_property' does not exist"); } @Test @@ -215,6 +215,6 @@ public void testTableProperties() { assertUpdate("CREATE TABLE mock.default.new_table (c int) WITH (integer_table_property = 1)"); assertThatThrownBy(() -> assertUpdate("CREATE TABLE mock.default.new_table (c int) WITH (unknown_property = 1)")) - .hasMessage("Catalog 'mock' does not support table property 'unknown_property'"); + .hasMessage("Catalog 'mock' table property 'unknown_property' does not exist"); } } diff --git a/testing/trino-tests/src/test/java/io/trino/tests/tpch/TestTpchConnectorTest.java b/testing/trino-tests/src/test/java/io/trino/tests/tpch/TestTpchConnectorTest.java index 3937febc03af..64d11e6ae66f 100644 --- a/testing/trino-tests/src/test/java/io/trino/tests/tpch/TestTpchConnectorTest.java +++ b/testing/trino-tests/src/test/java/io/trino/tests/tpch/TestTpchConnectorTest.java @@ -129,7 +129,7 @@ public void testAnalyzePropertiesSystemTable() public void testAnalyze() { assertUpdate("ANALYZE orders", 15000); - assertQueryFails("ANALYZE orders WITH (foo = 'bar')", ".* does not support analyze property 'foo'.*"); + assertQueryFails("ANALYZE orders WITH (foo = 'bar')", ".* analyze property 'foo' does not exist"); } @Test