Unify ConnectorMetadata.getTableHandle invocations

If a connector implements table versioning, it's going to implement `getTableHandle` that accepts optional version specification. It shouldn't need to still implement the `getTableHandle` without versioning specification.
trinodb · Jun 14, 2022 · 2a466ed · 2a466ed
1 parent c51be50
commit 2a466ed
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 19 deletions.
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
@@ -265,21 +265,12 @@ public Optional<TableHandle> getTableHandle(Session session, QualifiedObjectName
 
             ConnectorSession connectorSession = session.toConnectorSession(catalogName);
 
-            // GetTableHandle with the optional version handle field will throw an error if it is not implemented, so only try calling it when we have a version
-            if (startVersion.isPresent() || endVersion.isPresent()) {
-                ConnectorTableHandle versionedTableHandle = metadata.getTableHandle(
-                        connectorSession,
-                        table.asSchemaTableName(),
-                        toConnectorVersion(startVersion),
-                        toConnectorVersion(endVersion));
-                return Optional.ofNullable(versionedTableHandle)
-                        .map(connectorTableHandle -> new TableHandle(
-                                catalogName,
-                                connectorTableHandle,
-                                catalogMetadata.getTransactionHandleFor(catalogName)));
-            }
-
-            return Optional.ofNullable(metadata.getTableHandle(connectorSession, table.asSchemaTableName()))
+            ConnectorTableHandle tableHandle = metadata.getTableHandle(
+                    connectorSession,
+                    table.asSchemaTableName(),
+                    toConnectorVersion(startVersion),
+                    toConnectorVersion(endVersion));
+            return Optional.ofNullable(tableHandle)
                     .map(connectorTableHandle -> new TableHandle(
                             catalogName,
                             connectorTableHandle,

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
@@ -82,7 +82,13 @@ default ConnectorTableHandle getTableHandle(ConnectorSession session, SchemaTabl
     }
 
     /**
-     * Returns a table handle representing a versioned table. A versioned table differs by having an additional specifier for version.
+     * Returns a table handle for the specified table name and version, or {@code null} if {@code tableName} relation does not exist
+     * or is not a table (e.g. is a view, or a materialized view).
+     *
+     * @throws TrinoException implementation can throw this exception when {@code tableName} refers to a table that
+     * cannot be queried.
+     * @see #getView(ConnectorSession, SchemaTableName)
+     * @see #getMaterializedView(ConnectorSession, SchemaTableName)
      */
     @Nullable
     default ConnectorTableHandle getTableHandle(
@@ -91,10 +97,14 @@ default ConnectorTableHandle getTableHandle(
             Optional<ConnectorTableVersion> startVersion,
             Optional<ConnectorTableVersion> endVersion)
     {
-        if (getTableHandle(session, tableName) == null) {
+        ConnectorTableHandle tableHandle = getTableHandle(session, tableName);
+        if (tableHandle == null) {
             // Not found
             return null;
         }
+        if (startVersion.isEmpty() && endVersion.isEmpty()) {
+            return tableHandle;
+        }
         throw new TrinoException(NOT_SUPPORTED, "This connector does not support versioned tables");
     }
 

diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
@@ -271,7 +271,7 @@ public Optional<TrinoPrincipal> getSchemaOwner(ConnectorSession session, Catalog
     @Override
     public IcebergTableHandle getTableHandle(ConnectorSession session, SchemaTableName tableName)
     {
-        return getTableHandle(session, tableName, Optional.empty(), Optional.empty());
+        throw new UnsupportedOperationException("This method is not supported because getTableHandle with versions is implemented instead");
     }
 
     @Override
@@ -2080,7 +2080,7 @@ else if (strings.size() != 2) {
             String schema = strings.get(0);
             String name = strings.get(1);
             SchemaTableName schemaTableName = new SchemaTableName(schema, name);
-            IcebergTableHandle tableHandle = getTableHandle(session, schemaTableName);
+            IcebergTableHandle tableHandle = getTableHandle(session, schemaTableName, Optional.empty(), Optional.empty());
 
             if (tableHandle == null) {
                 throw new MaterializedViewNotFoundException(materializedViewName);