Pass table properties to getSupportedType

The set of supported types may depend on table properties, e.g. based on storage engine selection.
jklamer · Oct 3, 2023 · e115bd5 · e115bd5
1 parent ece8933
commit e115bd5
Show file tree

Hide file tree

Showing 14 changed files with 37 additions and 30 deletions.
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
@@ -157,6 +157,16 @@ ListenableFuture<Void> internalExecute(CreateTable statement, Session session, L
         String catalogName = tableName.getCatalogName();
         CatalogHandle catalogHandle = getRequiredCatalogHandle(plannerContext.getMetadata(), session, statement, catalogName);
 
+        Map<String, Object> properties = tablePropertyManager.getProperties(
+                catalogName,
+                catalogHandle,
+                statement.getProperties(),
+                session,
+                plannerContext,
+                accessControl,
+                parameterLookup,
+                true);
+
         LinkedHashMap<String, ColumnMetadata> columns = new LinkedHashMap<>();
         Map<String, Object> inheritedProperties = ImmutableMap.of();
         boolean includingProperties = false;
@@ -194,7 +204,7 @@ ListenableFuture<Void> internalExecute(CreateTable statement, Session session, L
 
                 columns.put(name.getValue().toLowerCase(ENGLISH), ColumnMetadata.builder()
                         .setName(name.getValue().toLowerCase(ENGLISH))
-                        .setType(getSupportedType(session, catalogHandle, type))
+                        .setType(getSupportedType(session, catalogHandle, properties, type))
                         .setNullable(column.isNullable())
                         .setComment(column.getComment())
                         .setProperties(columnProperties)
@@ -266,23 +276,14 @@ else if (element instanceof LikeClause likeClause) {
                             columns.put(
                                     column.getName().toLowerCase(Locale.ENGLISH),
                                     ColumnMetadata.builderFrom(column)
-                                            .setType(getSupportedType(session, catalogHandle, column.getType()))
+                                            .setType(getSupportedType(session, catalogHandle, properties, column.getType()))
                                             .build());
                         });
             }
             else {
                 throw new TrinoException(GENERIC_INTERNAL_ERROR, "Invalid TableElement: " + element.getClass().getName());
             }
         }
-        Map<String, Object> properties = tablePropertyManager.getProperties(
-                catalogName,
-                catalogHandle,
-                statement.getProperties(),
-                session,
-                plannerContext,
-                accessControl,
-                parameterLookup,
-                true);
 
         Set<String> specifiedPropertyKeys = statement.getProperties().stream()
                 // property names are case-insensitive and normalized to lower case
@@ -316,10 +317,10 @@ else if (element instanceof LikeClause likeClause) {
         return immediateVoidFuture();
     }
 
-    private Type getSupportedType(Session session, CatalogHandle catalogHandle, Type type)
+    private Type getSupportedType(Session session, CatalogHandle catalogHandle, Map<String, Object> tableProperties, Type type)
     {
         return plannerContext.getMetadata()
-                .getSupportedType(session, catalogHandle, type)
+                .getSupportedType(session, catalogHandle, tableProperties, type)
                 .orElse(type);
     }
 

diff --git a/core/trino-main/src/main/java/io/trino/metadata/Metadata.java b/core/trino-main/src/main/java/io/trino/metadata/Metadata.java
@@ -309,7 +309,7 @@ Optional<TableExecuteHandle> getTableHandleForExecute(
     /**
      * Return the effective {@link io.trino.spi.type.Type} that is supported by the connector for the given type, if {@link Optional#empty()} is returned, the type will be used as is during table creation which may or may not be supported by the connector.
      */
-    Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Type type);
+    Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Map<String, Object> tableProperties, Type type);
 
     /**
      * Begin the atomic creation of a table with data.

diff --git a/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java b/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java
@@ -1008,11 +1008,11 @@ public Optional<TableLayout> getNewTableLayout(Session session, String catalogNa
     }
 
     @Override
-    public Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Type type)
+    public Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Map<String, Object> tableProperties, Type type)
     {
         CatalogMetadata catalogMetadata = getCatalogMetadata(session, catalogHandle);
         ConnectorMetadata metadata = catalogMetadata.getMetadata(session);
-        return metadata.getSupportedType(session.toConnectorSession(catalogHandle), type)
+        return metadata.getSupportedType(session.toConnectorSession(catalogHandle), tableProperties, type)
                 .map(newType -> {
                     if (!typeCoercion.isCompatible(newType, type)) {
                         throw new TrinoException(FUNCTION_IMPLEMENTATION_ERROR, format("Type '%s' is not compatible with the supplied type '%s' in getSupportedType", type, newType));

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
@@ -946,15 +946,15 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional<Scop
                         throw semanticException(COLUMN_TYPE_UNKNOWN, node, "Column type is unknown at position %s", queryScope.getRelationType().indexOf(field) + 1);
                     }
                     String columnName = node.getColumnAliases().get().get(aliasPosition).getValue();
-                    columnsBuilder.add(new ColumnMetadata(columnName, metadata.getSupportedType(session, catalogHandle, field.getType()).orElse(field.getType())));
+                    columnsBuilder.add(new ColumnMetadata(columnName, metadata.getSupportedType(session, catalogHandle, properties, field.getType()).orElse(field.getType())));
                     outputColumns.add(new OutputColumn(new Column(columnName, field.getType().toString()), analysis.getSourceColumns(field)));
                     aliasPosition++;
                 }
             }
             else {
                 validateColumns(node, queryScope.getRelationType());
                 columnsBuilder.addAll(queryScope.getRelationType().getVisibleFields().stream()
-                        .map(field -> new ColumnMetadata(field.getName().orElseThrow(), metadata.getSupportedType(session, catalogHandle, field.getType()).orElse(field.getType())))
+                        .map(field -> new ColumnMetadata(field.getName().orElseThrow(), metadata.getSupportedType(session, catalogHandle, properties, field.getType()).orElse(field.getType())))
                         .collect(toImmutableList()));
                 queryScope.getRelationType().getVisibleFields().stream()
                         .map(this::createOutputColumn)

diff --git a/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java b/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java
@@ -547,11 +547,11 @@ public Optional<ConnectorTableLayout> getNewTableLayout(ConnectorSession session
     }
 
     @Override
-    public Optional<Type> getSupportedType(ConnectorSession session, Type type)
+    public Optional<Type> getSupportedType(ConnectorSession session, Map<String, Object> tableProperties, Type type)
     {
         Span span = startSpan("getSupportedType");
         try (var ignored = scopedSpan(span)) {
-            return delegate.getSupportedType(session, type);
+            return delegate.getSupportedType(session, tableProperties, type);
         }
     }
 

diff --git a/core/trino-main/src/main/java/io/trino/tracing/TracingMetadata.java b/core/trino-main/src/main/java/io/trino/tracing/TracingMetadata.java
@@ -559,11 +559,11 @@ public Optional<TableLayout> getNewTableLayout(Session session, String catalogNa
     }
 
     @Override
-    public Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Type type)
+    public Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Map<String, Object> tableProperties, Type type)
     {
         Span span = startSpan("getSupportedType", catalogHandle.getCatalogName());
         try (var ignored = scopedSpan(span)) {
-            return delegate.getSupportedType(session, catalogHandle, type);
+            return delegate.getSupportedType(session, catalogHandle, tableProperties, type);
         }
     }
 

diff --git a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java
@@ -774,7 +774,7 @@ public Optional<ConnectorTableLayout> getNewTableLayout(ConnectorSession session
         }
 
         @Override
-        public Optional<Type> getSupportedType(ConnectorSession session, Type type)
+        public Optional<Type> getSupportedType(ConnectorSession session, Map<String, Object> tableProperties, Type type)
         {
             return getSupportedType.apply(session, type);
         }

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
@@ -463,7 +463,7 @@ public Optional<TableHandle> getTableHandle(Session session, QualifiedObjectName
         }
 
         @Override
-        public Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Type type)
+        public Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Map<String, Object> tableProperties, Type type)
         {
             if (type instanceof TimestampType) {
                 return Optional.of(TIMESTAMP_MILLIS);

diff --git a/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java b/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java
@@ -393,7 +393,7 @@ public Optional<TableLayout> getNewTableLayout(Session session, String catalogNa
     }
 
     @Override
-    public Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Type type)
+    public Optional<Type> getSupportedType(Session session, CatalogHandle catalogHandle, Map<String, Object> tableProperties, Type type)
     {
         throw new UnsupportedOperationException();
     }

diff --git a/core/trino-spi/pom.xml b/core/trino-spi/pom.xml
@@ -211,6 +211,11 @@
                                 </item>
                                 <!-- Backwards incompatible changes since the previous release -->
                                 <!-- Any exclusions below can be deleted after each release -->
+                                <item>
+                                    <ignore>true</ignore>
+                                    <code>java.method.removed</code>
+                                    <old>method java.util.Optional&lt;io.trino.spi.type.Type&gt; io.trino.spi.connector.ConnectorMetadata::getSupportedType(io.trino.spi.connector.ConnectorSession, io.trino.spi.type.Type)</old>
+                                </item>
                             </differences>
                         </revapi.differences>
                     </analysisConfiguration>

diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java
@@ -662,7 +662,8 @@ default Optional<ConnectorTableLayout> getNewTableLayout(ConnectorSession sessio
      * If {@link Optional#empty()} is returned, the type will be used as is during table creation which may or may not be supported by the connector.
      * The effective type shall be a type that is cast-compatible with the input type.
      */
-    default Optional<Type> getSupportedType(ConnectorSession session, Type type)
+    @Experimental(eta = "2024-01-31")
+    default Optional<Type> getSupportedType(ConnectorSession session, Map<String, Object> tableProperties, Type type)
     {
         return Optional.empty();
     }

diff --git a/...lkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java b/...lkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java
@@ -133,10 +133,10 @@ public Optional<ConnectorTableLayout> getNewTableLayout(ConnectorSession session
     }
 
     @Override
-    public Optional<Type> getSupportedType(ConnectorSession session, Type type)
+    public Optional<Type> getSupportedType(ConnectorSession session, Map<String, Object> tableProperties, Type type)
     {
         try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) {
-            return delegate.getSupportedType(session, type);
+            return delegate.getSupportedType(session, tableProperties, type);
         }
     }
 

diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java
@@ -799,7 +799,7 @@ private void verifyRetryMode(ConnectorSession session, RetryMode retryMode)
     }
 
     @Override
-    public Optional<Type> getSupportedType(ConnectorSession session, Type type)
+    public Optional<Type> getSupportedType(ConnectorSession session, Map<String, Object> tableProperties, Type type)
     {
         return jdbcClient.getSupportedType(session, type);
     }

diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java
@@ -3416,7 +3416,7 @@ public Optional<ConnectorTableLayout> getNewTableLayout(ConnectorSession session
     }
 
     @Override
-    public Optional<Type> getSupportedType(ConnectorSession session, Type type)
+    public Optional<Type> getSupportedType(ConnectorSession session, Map<String, Object> tableProperties, Type type)
     {
         if (type instanceof VarcharType varcharType && !varcharType.isUnbounded() && varcharType.getBoundedLength() == 0) {
             return Optional.of(VarcharType.createVarcharType(1));