Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Arbitrary table properties #9523

Closed
wants to merge 2 commits into from
Closed

[WIP] Arbitrary table properties #9523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b611f0d
Add support for dynamic table properties
dain Oct 6, 2021
d9d7060
Add support for arbitrary extra table properties to Hive
dain Oct 6, 2021
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: 8 additions & 7 deletions core/trino-main/src/main/java/io/trino/connector/ConnectorManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import io.trino.spi.connector.ConnectorPageSourceProvider;
import io.trino.spi.connector.ConnectorRecordSetProvider;
import io.trino.spi.connector.ConnectorSplitManager;
import io.trino.spi.connector.PropertyProvider;
import io.trino.spi.connector.SystemTable;
import io.trino.spi.eventlistener.EventListener;
import io.trino.spi.procedure.Procedure;
Expand Down Expand Up @@ -303,7 +304,7 @@ private synchronized void addConnectorInternal(MaterializedConnector connector)
connector.getAccessControl()
.ifPresent(accessControl -> accessControlManager.addCatalogAccessControl(catalogName, accessControl));

metadataManager.getTablePropertyManager().addProperties(catalogName, connector.getTableProperties());
metadataManager.getTablePropertyManager().addProperties(catalogName, connector.getTablePropertyProvider());
metadataManager.getMaterializedViewPropertyManager().addProperties(catalogName, connector.getMaterializedViewProperties());
metadataManager.getColumnPropertyManager().addProperties(catalogName, connector.getColumnProperties());
metadataManager.getSchemaPropertyManager().addProperties(catalogName, connector.getSchemaProperties());
Expand Down Expand Up @@ -409,7 +410,7 @@ private static class MaterializedConnector
private final Optional<ConnectorAccessControl> accessControl;
private final List<EventListener> eventListeners;
private final List<PropertyMetadata<?>> sessionProperties;
private final List<PropertyMetadata<?>> tableProperties;
private final PropertyProvider tablePropertyProvider;
private final List<PropertyMetadata<?>> materializedViewProperties;
private final List<PropertyMetadata<?>> schemaProperties;
private final List<PropertyMetadata<?>> columnProperties;
Expand Down Expand Up @@ -497,9 +498,9 @@ public MaterializedConnector(CatalogName catalogName, Connector connector)
requireNonNull(sessionProperties, format("Connector '%s' returned a null system properties set", catalogName));
this.sessionProperties = ImmutableList.copyOf(sessionProperties);

List<PropertyMetadata<?>> tableProperties = connector.getTableProperties();
requireNonNull(tableProperties, format("Connector '%s' returned a null table properties set", catalogName));
this.tableProperties = ImmutableList.copyOf(tableProperties);
PropertyProvider tablePropertyProvider = connector.getTablePropertyProvider();
requireNonNull(tablePropertyProvider, format("Connector '%s' returned a null table properties provider", catalogName));
this.tablePropertyProvider = tablePropertyProvider;

List<PropertyMetadata<?>> materializedViewProperties = connector.getMaterializedViewProperties();
requireNonNull(materializedViewProperties, format("Connector '%s' returned a null materialized view properties set", catalogName));
Expand Down Expand Up @@ -578,9 +579,9 @@ public List<PropertyMetadata<?>> getSessionProperties()
return sessionProperties;
}

public List<PropertyMetadata<?>> getTableProperties()
public PropertyProvider getTablePropertyProvider()
{
return tableProperties;
return tablePropertyProvider;
}

public List<PropertyMetadata<?>> getMaterializedViewProperties()
Expand Down
32 changes: 17 additions & 15 deletions core/trino-main/src/main/java/io/trino/connector/system/AbstractPropertiesSystemTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,22 @@
*/
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;
import io.trino.spi.connector.ConnectorTransactionHandle;
import io.trino.spi.connector.InMemoryRecordSet;
import io.trino.spi.connector.PropertyProvider;
import io.trino.spi.connector.RecordCursor;
import io.trino.spi.connector.SchemaTableName;
import io.trino.spi.connector.SystemTable;
import io.trino.spi.predicate.TupleDomain;
import io.trino.spi.session.PropertyMetadata;
import io.trino.transaction.TransactionId;
import io.trino.transaction.TransactionManager;

import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.TreeMap;
import java.util.function.Supplier;

Expand All @@ -43,9 +43,9 @@ abstract class AbstractPropertiesSystemTable
{
private final ConnectorTableMetadata tableMetadata;
private final TransactionManager transactionManager;
private final Supplier<Map<CatalogName, Map<String, PropertyMetadata<?>>>> propertySupplier;
private final Supplier<Map<CatalogName, PropertyProvider>> propertySupplier;

protected AbstractPropertiesSystemTable(String tableName, TransactionManager transactionManager, Supplier<Map<CatalogName, Map<String, PropertyMetadata<?>>>> propertySupplier)
protected AbstractPropertiesSystemTable(String tableName, TransactionManager transactionManager, Supplier<Map<CatalogName, PropertyProvider>> propertySupplier)
{
this.tableMetadata = tableMetadataBuilder(new SchemaTableName("metadata", tableName))
.column("catalog_name", createUnboundedVarcharType())
Expand Down Expand Up @@ -76,18 +76,20 @@ public final RecordCursor cursor(ConnectorTransactionHandle transactionHandle, C
TransactionId transactionId = ((GlobalSystemTransactionHandle) transactionHandle).getTransactionId();

InMemoryRecordSet.Builder table = InMemoryRecordSet.builder(tableMetadata);
Map<CatalogName, Map<String, PropertyMetadata<?>>> connectorProperties = propertySupplier.get();
Map<CatalogName, PropertyProvider> connectorProperties = propertySupplier.get();
for (Entry<String, CatalogName> entry : new TreeMap<>(transactionManager.getCatalogNames(transactionId)).entrySet()) {
String catalog = entry.getKey();
Map<String, PropertyMetadata<?>> 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());
}
PropertyProvider propertyProvider = connectorProperties.get(entry.getValue());
propertyProvider.getKnownPropertyNames().stream()
.sorted()
.map(propertyProvider::getProperty)
.filter(Optional::isPresent)
.map(Optional::get)
.forEach(propertyMetadata -> table.addRow(
entry.getKey(),
propertyMetadata.getName(),
firstNonNull(propertyMetadata.getDefaultValue(), "").toString(),
propertyMetadata.getSqlType().toString(),
propertyMetadata.getDescription()));
}
return table.build().cursor();
}
Expand Down
43 changes: 24 additions & 19 deletions core/trino-main/src/main/java/io/trino/metadata/AbstractPropertyManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
package io.trino.metadata;

import com.google.common.collect.ImmutableMap;
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.block.BlockBuilder;
import io.trino.spi.connector.FixedPropertyProvider;
import io.trino.spi.connector.PropertyProvider;
import io.trino.spi.session.PropertyMetadata;
import io.trino.spi.type.Type;
import io.trino.sql.planner.ParameterRewriter;
Expand All @@ -44,7 +45,7 @@

abstract class AbstractPropertyManager
{
private final ConcurrentMap<CatalogName, Map<String, PropertyMetadata<?>>> connectorProperties = new ConcurrentHashMap<>();
private final ConcurrentMap<CatalogName, PropertyProvider> connectorProperties = new ConcurrentHashMap<>();
private final String propertyType;
private final ErrorCodeSupplier propertyError;

Expand All @@ -57,12 +58,15 @@ protected AbstractPropertyManager(String propertyType, ErrorCodeSupplier propert

public final void addProperties(CatalogName catalogName, List<PropertyMetadata<?>> properties)
{
requireNonNull(catalogName, "catalogName is null");
requireNonNull(properties, "properties is null");
addProperties(catalogName, new FixedPropertyProvider(properties));
}

Map<String, PropertyMetadata<?>> propertiesByName = Maps.uniqueIndex(properties, PropertyMetadata::getName);
public final void addProperties(CatalogName catalogName, PropertyProvider propertyProvider)
{
requireNonNull(catalogName, "catalogName is null");
requireNonNull(propertyProvider, "propertyProvider is null");

checkState(connectorProperties.putIfAbsent(catalogName, propertiesByName) == null, "Properties for connector '%s' are already registered", catalogName);
checkState(connectorProperties.putIfAbsent(catalogName, propertyProvider) == null, "Properties for connector '%s' are already registered", catalogName);
}

public final void removeProperties(CatalogName catalogName)
Expand All @@ -79,8 +83,8 @@ public final Map<String, Object> getProperties(
AccessControl accessControl,
Map<NodeRef<Parameter>, Expression> parameters)
{
Map<String, PropertyMetadata<?>> supportedProperties = connectorProperties.get(catalogName);
if (supportedProperties == null) {
PropertyProvider propertyProvider = connectorProperties.get(catalogName);
if (propertyProvider == null) {
throw new TrinoException(NOT_FOUND, "Catalog not found: " + catalog);
}

Expand All @@ -89,15 +93,14 @@ public final Map<String, Object> getProperties(
// Fill in user-specified properties
for (Map.Entry<String, Expression> sqlProperty : sqlPropertyValues.entrySet()) {
String propertyName = sqlProperty.getKey().toLowerCase(ENGLISH);
PropertyMetadata<?> property = supportedProperties.get(propertyName);
if (property == null) {
throw new TrinoException(
propertyError,
format("Catalog '%s' does not support %s property '%s'",
catalog,
propertyType,
propertyName));
}
PropertyMetadata<?> property = propertyProvider.getProperty(propertyName)
.orElseThrow(() -> new TrinoException(
propertyError,
format(
"Catalog '%s' does not support %s property '%s'",
catalog,
propertyType,
propertyName)));

Object sqlObjectValue;
try {
Expand Down Expand Up @@ -135,7 +138,9 @@ public final Map<String, Object> getProperties(
Map<String, Object> userSpecifiedProperties = properties.build();

// Fill in the remaining properties with non-null defaults
for (PropertyMetadata<?> propertyMetadata : supportedProperties.values()) {
for (String propertyName : propertyProvider.getKnownPropertyNames()) {
PropertyMetadata<?> propertyMetadata = propertyProvider.getProperty(propertyName)
.orElseThrow(() -> new TrinoException(propertyError, "Could not get load default value property: " + propertyName));
if (!userSpecifiedProperties.containsKey(propertyMetadata.getName())) {
Object value = propertyMetadata.getDefaultValue();
if (value != null) {
Expand All @@ -146,7 +151,7 @@ public final Map<String, Object> getProperties(
return properties.build();
}

public Map<CatalogName, Map<String, PropertyMetadata<?>>> getAllProperties()
public Map<CatalogName, PropertyProvider> getAllProperties()
{
return ImmutableMap.copyOf(connectorProperties);
}
Expand Down
25 changes: 12 additions & 13 deletions core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import io.trino.spi.connector.ConnectorMaterializedViewDefinition;
import io.trino.spi.connector.ConnectorTableMetadata;
import io.trino.spi.connector.ConnectorViewDefinition;
import io.trino.spi.connector.PropertyProvider;
import io.trino.spi.connector.SchemaTableName;
import io.trino.spi.security.GroupProvider;
import io.trino.spi.security.PrincipalType;
Expand Down Expand Up @@ -521,10 +522,10 @@ protected Node visitShowCreate(ShowCreate node, Void context)
accessControl.checkCanShowCreateTable(session.toSecurityContext(), new QualifiedObjectName(catalogName.getValue(), schemaName.getValue(), tableName.getValue()));

Map<String, Object> properties = viewDefinition.get().getProperties();
Map<String, PropertyMetadata<?>> allMaterializedViewProperties = metadata.getMaterializedViewPropertyManager()
PropertyProvider materializedViewPropertyProvider = metadata.getMaterializedViewPropertyManager()
.getAllProperties()
.get(new CatalogName(catalogName.getValue()));
List<Property> propertyNodes = buildProperties(objectName, Optional.empty(), INVALID_MATERIALIZED_VIEW_PROPERTY, properties, allMaterializedViewProperties);
List<Property> propertyNodes = buildProperties(objectName, Optional.empty(), INVALID_MATERIALIZED_VIEW_PROPERTY, properties, materializedViewPropertyProvider);

String sql = formatSql(new CreateMaterializedView(Optional.empty(), QualifiedName.of(ImmutableList.of(catalogName, schemaName, tableName)),
query, false, false, propertyNodes, viewDefinition.get().getComment())).trim();
Expand Down Expand Up @@ -587,12 +588,12 @@ protected Node visitShowCreate(ShowCreate node, Void context)
accessControl.checkCanShowCreateTable(session.toSecurityContext(), targetTableName);
ConnectorTableMetadata connectorTableMetadata = metadata.getTableMetadata(session, tableHandle.get()).getMetadata();

Map<String, PropertyMetadata<?>> allColumnProperties = metadata.getColumnPropertyManager().getAllProperties().get(tableHandle.get().getCatalogName());
PropertyProvider columnPropertyProvider = metadata.getColumnPropertyManager().getAllProperties().get(tableHandle.get().getCatalogName());

List<TableElement> columns = connectorTableMetadata.getColumns().stream()
.filter(column -> !column.isHidden())
.map(column -> {
List<Property> propertyNodes = buildProperties(targetTableName, Optional.of(column.getName()), INVALID_COLUMN_PROPERTY, column.getProperties(), allColumnProperties);
List<Property> propertyNodes = buildProperties(targetTableName, Optional.of(column.getName()), INVALID_COLUMN_PROPERTY, column.getProperties(), columnPropertyProvider);
return new ColumnDefinition(
new Identifier(column.getName()),
toSqlType(column.getType()),
Expand All @@ -603,8 +604,8 @@ protected Node visitShowCreate(ShowCreate node, Void context)
.collect(toImmutableList());

Map<String, Object> properties = connectorTableMetadata.getProperties();
Map<String, PropertyMetadata<?>> allTableProperties = metadata.getTablePropertyManager().getAllProperties().get(tableHandle.get().getCatalogName());
List<Property> propertyNodes = buildProperties(targetTableName, Optional.empty(), INVALID_TABLE_PROPERTY, properties, allTableProperties);
PropertyProvider tablePropertyProvider = metadata.getTablePropertyManager().getAllProperties().get(tableHandle.get().getCatalogName());
List<Property> propertyNodes = buildProperties(targetTableName, Optional.empty(), INVALID_TABLE_PROPERTY, properties, tablePropertyProvider);

CreateTable createTable = new CreateTable(
QualifiedName.of(objectName.getCatalogName(), objectName.getSchemaName(), objectName.getObjectName()),
Expand All @@ -625,9 +626,9 @@ protected Node visitShowCreate(ShowCreate node, Void context)
accessControl.checkCanShowCreateSchema(session.toSecurityContext(), schemaName);

Map<String, Object> properties = metadata.getSchemaProperties(session, schemaName);
Map<String, PropertyMetadata<?>> allTableProperties = metadata.getSchemaPropertyManager().getAllProperties().get(new CatalogName(schemaName.getCatalogName()));
PropertyProvider tablePropertyProvider = metadata.getSchemaPropertyManager().getAllProperties().get(new CatalogName(schemaName.getCatalogName()));
QualifiedName qualifiedSchemaName = QualifiedName.of(schemaName.getCatalogName(), schemaName.getSchemaName());
List<Property> propertyNodes = buildProperties(qualifiedSchemaName, Optional.empty(), INVALID_SCHEMA_PROPERTY, properties, allTableProperties);
List<Property> propertyNodes = buildProperties(qualifiedSchemaName, Optional.empty(), INVALID_SCHEMA_PROPERTY, properties, tablePropertyProvider);

Optional<PrincipalSpecification> owner = metadata.getSchemaOwner(session, schemaName).map(MetadataUtil::createPrincipal);

Expand All @@ -647,7 +648,7 @@ private List<Property> buildProperties(
Optional<String> columnName,
StandardErrorCode errorCode,
Map<String, Object> properties,
Map<String, PropertyMetadata<?>> allProperties)
PropertyProvider propertyProvider)
{
if (properties.isEmpty()) {
return Collections.emptyList();
Expand All @@ -662,10 +663,8 @@ private List<Property> buildProperties(
throw new TrinoException(errorCode, format("Property %s for %s cannot have a null value", propertyName, toQualifiedName(objectName, columnName)));
}

PropertyMetadata<?> property = allProperties.get(propertyName);
if (property == null) {
throw new TrinoException(errorCode, "No PropertyMetadata for property: " + propertyName);
}
PropertyMetadata<?> property = propertyProvider.getProperty(propertyName)
.orElseThrow(() -> new TrinoException(errorCode, "No PropertyMetadata for property: " + propertyName));
if (!Primitives.wrap(property.getJavaType()).isInstance(value)) {
throw new TrinoException(errorCode, format(
"Property %s for %s should have value of type %s, not %s",
Expand Down
10 changes: 10 additions & 0 deletions core/trino-spi/src/main/java/io/trino/spi/connector/Connector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,22 @@ default List<PropertyMetadata<?>> getAnalyzeProperties()

/**
* @return the table properties for this connector
* @deprecated use getTablePropertyProvider
*/
@Deprecated
default List<PropertyMetadata<?>> getTableProperties()
{
return emptyList();
}

/**
* @return the table properties for this connector
*/
default PropertyProvider getTablePropertyProvider()
{
return new FixedPropertyProvider(getTableProperties());
}

/**
* @return the materialized view properties for this connector
*/
Expand Down
Loading