diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcClient.java index 4667ebd7d2a2..62993cfd16e3 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcClient.java @@ -205,7 +205,7 @@ public Optional getTableHandle(ConnectorSession session, Schema try (ResultSet resultSet = getTables(connection, Optional.of(remoteSchema), Optional.of(remoteTable))) { List tableHandles = new ArrayList<>(); while (resultSet.next()) { - tableHandles.add(new JdbcTableHandle(schemaTableName, getRemoteTable(resultSet))); + tableHandles.add(new JdbcTableHandle(schemaTableName, getRemoteTable(resultSet), getTableComment(resultSet))); } if (tableHandles.isEmpty()) { return Optional.empty(); diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java index b0ca6b4ab4f5..7979f86440c6 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java @@ -377,6 +377,20 @@ public void renameSchema(ConnectorSession session, String schemaName, String new invalidateSchemasCache(); } + @Override + public Optional getTableComment(ResultSet resultSet) + throws SQLException + { + return delegate.getTableComment(resultSet); + } + + @Override + public void setTableComment(ConnectorSession session, JdbcTableHandle handle, Optional comment) + { + delegate.setTableComment(session, handle, comment); + invalidateTableCaches(handle.asPlainTable().getSchemaTableName()); + } + @Override public void setColumnComment(ConnectorSession session, JdbcTableHandle handle, JdbcColumnHandle column, Optional comment) { 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 index b55bbeb79d85..353b56ac51ab 100644 --- 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 @@ -592,7 +592,8 @@ public ConnectorTableMetadata getTableMetadata(ConnectorSession session, Connect jdbcClient.getColumns(session, handle).stream() .map(JdbcColumnHandle::getColumnMetadata) .collect(toImmutableList()), - jdbcClient.getTableProperties(session, handle)); + jdbcClient.getTableProperties(session, handle), + getTableComment(handle)); } public static SchemaTableName getSchemaTableName(JdbcTableHandle handle) @@ -603,6 +604,11 @@ public static SchemaTableName getSchemaTableName(JdbcTableHandle handle) : new SchemaTableName("_generated", "_generated_query"); } + public static Optional getTableComment(JdbcTableHandle handle) + { + return handle.isNamedRelation() ? handle.getRequiredNamedRelation().getComment() : Optional.empty(); + } + @Override public List listTables(ConnectorSession session, Optional schemaName) { @@ -743,6 +749,14 @@ public void truncateTable(ConnectorSession session, ConnectorTableHandle tableHa jdbcClient.truncateTable(session, (JdbcTableHandle) tableHandle); } + @Override + public void setTableComment(ConnectorSession session, ConnectorTableHandle table, Optional comment) + { + JdbcTableHandle tableHandle = (JdbcTableHandle) table; + verify(!tableHandle.isSynthetic(), "Not a table reference: %s", tableHandle); + jdbcClient.setTableComment(session, tableHandle, comment); + } + @Override public void setColumnComment(ConnectorSession session, ConnectorTableHandle table, ColumnHandle column, Optional comment) { diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java index aa01e74353c9..ac7488fbd11a 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java @@ -264,6 +264,19 @@ public boolean isLimitGuaranteed(ConnectorSession session) return delegate().isLimitGuaranteed(session); } + @Override + public Optional getTableComment(ResultSet resultSet) + throws SQLException + { + return delegate().getTableComment(resultSet); + } + + @Override + public void setTableComment(ConnectorSession session, JdbcTableHandle handle, Optional comment) + { + delegate().setTableComment(session, handle, comment); + } + @Override public void setColumnComment(ConnectorSession session, JdbcTableHandle handle, JdbcColumnHandle column, Optional comment) { diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcClient.java index d18cf05e0c4a..223fbcc1b283 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcClient.java @@ -123,6 +123,17 @@ Optional implementJoin( boolean isLimitGuaranteed(ConnectorSession session); + default Optional getTableComment(ResultSet resultSet) + throws SQLException + { + return Optional.ofNullable(resultSet.getString("REMARKS")); + } + + default void setTableComment(ConnectorSession session, JdbcTableHandle handle, Optional comment) + { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting table comments"); + } + default void setColumnComment(ConnectorSession session, JdbcTableHandle handle, JdbcColumnHandle column, Optional comment) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting column comments"); diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcNamedRelationHandle.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcNamedRelationHandle.java index 6ec7f82a8212..18cc3c31314c 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcNamedRelationHandle.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcNamedRelationHandle.java @@ -18,6 +18,7 @@ import io.trino.spi.connector.SchemaTableName; import java.util.Objects; +import java.util.Optional; import static java.lang.String.format; import static java.util.Objects.requireNonNull; @@ -27,14 +28,17 @@ public class JdbcNamedRelationHandle { private final SchemaTableName schemaTableName; private final RemoteTableName remoteTableName; + private final Optional comment; @JsonCreator public JdbcNamedRelationHandle( @JsonProperty("schemaTableName") SchemaTableName schemaTableName, - @JsonProperty("remoteTableName") RemoteTableName remoteTableName) + @JsonProperty("remoteTableName") RemoteTableName remoteTableName, + @JsonProperty("comment") Optional comment) { this.schemaTableName = requireNonNull(schemaTableName, "schemaTableName is null"); this.remoteTableName = requireNonNull(remoteTableName, "remoteTableName is null"); + this.comment = requireNonNull(comment, "comment is null"); } @JsonProperty @@ -49,6 +53,12 @@ public RemoteTableName getRemoteTableName() return remoteTableName; } + @JsonProperty + public Optional getComment() + { + return comment; + } + @Override public boolean equals(Object o) { diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcTableHandle.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcTableHandle.java index dfd38f051fd7..48d08c453c81 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcTableHandle.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcTableHandle.java @@ -63,13 +63,13 @@ public final class JdbcTableHandle @Deprecated public JdbcTableHandle(SchemaTableName schemaTableName, @Nullable String catalogName, @Nullable String schemaName, String tableName) { - this(schemaTableName, new RemoteTableName(Optional.ofNullable(catalogName), Optional.ofNullable(schemaName), tableName)); + this(schemaTableName, new RemoteTableName(Optional.ofNullable(catalogName), Optional.ofNullable(schemaName), tableName), Optional.empty()); } - public JdbcTableHandle(SchemaTableName schemaTableName, RemoteTableName remoteTableName) + public JdbcTableHandle(SchemaTableName schemaTableName, RemoteTableName remoteTableName, Optional comment) { this( - new JdbcNamedRelationHandle(schemaTableName, remoteTableName), + new JdbcNamedRelationHandle(schemaTableName, remoteTableName, comment), TupleDomain.all(), ImmutableList.of(), Optional.empty(), diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/JdbcClientStats.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/JdbcClientStats.java index bf837d3b094a..cfcca6faea65 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/JdbcClientStats.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/JdbcClientStats.java @@ -29,6 +29,8 @@ public final class JdbcClientStats private final JdbcApiStats commitCreateTable = new JdbcApiStats(); private final JdbcApiStats createSchema = new JdbcApiStats(); private final JdbcApiStats createTable = new JdbcApiStats(); + private final JdbcApiStats getTableComment = new JdbcApiStats(); + private final JdbcApiStats setTableComment = new JdbcApiStats(); private final JdbcApiStats setColumnComment = new JdbcApiStats(); private final JdbcApiStats dropColumn = new JdbcApiStats(); private final JdbcApiStats dropSchema = new JdbcApiStats(); @@ -135,6 +137,20 @@ public JdbcApiStats getCreateTable() return createTable; } + @Managed + @Nested + public JdbcApiStats getGetTableComment() + { + return getTableComment; + } + + @Managed + @Nested + public JdbcApiStats getSetTableComment() + { + return setTableComment; + } + @Managed @Nested public JdbcApiStats getSetColumnComment() diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/StatisticsAwareJdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/StatisticsAwareJdbcClient.java index 8aac1155378c..f6b58fde5e02 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/StatisticsAwareJdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/StatisticsAwareJdbcClient.java @@ -197,6 +197,19 @@ public Optional implementJoin(ConnectorSession session, return stats.getImplementJoin().wrap(() -> delegate().implementJoin(session, joinType, leftSource, rightSource, joinConditions, rightAssignments, leftAssignments, statistics)); } + @Override + public Optional getTableComment(ResultSet resultSet) + throws SQLException + { + return stats.getGetTableComment().wrap(() -> delegate().getTableComment(resultSet)); + } + + @Override + public void setTableComment(ConnectorSession session, JdbcTableHandle handle, Optional comment) + { + stats.getSetTableComment().wrap(() -> delegate().setTableComment(session, handle, comment)); + } + @Override public void setColumnComment(ConnectorSession session, JdbcTableHandle handle, JdbcColumnHandle column, Optional comment) { diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcQueryBuilder.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcQueryBuilder.java index e9a740403404..8b12e5ed3c12 100644 --- a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcQueryBuilder.java +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcQueryBuilder.java @@ -93,7 +93,8 @@ public class TestDefaultJdbcQueryBuilder { private static final JdbcNamedRelationHandle TEST_TABLE = new JdbcNamedRelationHandle(new SchemaTableName( "some_test_schema", "test_table"), - new RemoteTableName(Optional.empty(), Optional.empty(), "test_table")); + new RemoteTableName(Optional.empty(), Optional.empty(), "test_table"), + Optional.empty()); private static final ConnectorSession SESSION = TestingConnectorSession.builder() .setPropertyMetadata(new JdbcMetadataSessionProperties(new JdbcMetadataConfig(), Optional.empty()).getSessionProperties()) .build(); diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestJdbcTableHandle.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestJdbcTableHandle.java index 7d9d7233680d..3aff0b180109 100644 --- a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestJdbcTableHandle.java +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestJdbcTableHandle.java @@ -81,7 +81,8 @@ private JdbcTableHandle createNamedHandle() return new JdbcTableHandle( new JdbcNamedRelationHandle( new SchemaTableName("schema", "table"), - new RemoteTableName(Optional.of("catalog"), Optional.of("schema"), "table")), + new RemoteTableName(Optional.of("catalog"), Optional.of("schema"), "table"), + Optional.empty()), TupleDomain.all(), ImmutableList.of(), Optional.empty(), diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestingH2JdbcClient.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestingH2JdbcClient.java index 1d84240723ce..0a41962ea655 100644 --- a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestingH2JdbcClient.java +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestingH2JdbcClient.java @@ -34,6 +34,7 @@ import net.jodah.failsafe.RetryPolicy; import java.sql.Connection; +import java.sql.ResultSet; import java.sql.Types; import java.util.Collection; import java.util.List; @@ -101,6 +102,13 @@ public Collection listSchemas(Connection connection) .get(() -> super.listSchemas(connection)); } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override public boolean supportsAggregationPushdown(ConnectorSession session, JdbcTableHandle table, List aggregates, Map assignments, List> groupingSets) { diff --git a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java index 84bd6281f9b4..0415fcd918b6 100644 --- a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java +++ b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java @@ -264,6 +264,13 @@ protected void copyTableSchema(Connection connection, String catalogName, String execute(connection, sql); } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override protected String createTableSql(RemoteTableName remoteTableName, List columns, ConnectorTableMetadata tableMetadata) { diff --git a/plugin/trino-druid/src/main/java/io/trino/plugin/druid/DruidJdbcClient.java b/plugin/trino-druid/src/main/java/io/trino/plugin/druid/DruidJdbcClient.java index 6b7034d2fb01..220d224e7e3b 100644 --- a/plugin/trino-druid/src/main/java/io/trino/plugin/druid/DruidJdbcClient.java +++ b/plugin/trino-druid/src/main/java/io/trino/plugin/druid/DruidJdbcClient.java @@ -178,6 +178,13 @@ public ResultSet getTables(Connection connection, Optional schemaName, O null); } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override public Optional toColumnMapping(ConnectorSession session, Connection connection, JdbcTypeHandle typeHandle) { @@ -284,7 +291,8 @@ private JdbcTableHandle prepareTableHandleForQuery(JdbcTableHandle table) new RemoteTableName( Optional.empty(), table.getRequiredNamedRelation().getRemoteTableName().getSchemaName(), - table.getRequiredNamedRelation().getRemoteTableName().getTableName())), + table.getRequiredNamedRelation().getRemoteTableName().getTableName()), + Optional.empty()), table.getConstraint(), table.getConstraintExpressions(), table.getSortOrder(), diff --git a/plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java b/plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java index ba943368b434..7c6e9d322180 100644 --- a/plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java +++ b/plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java @@ -30,6 +30,7 @@ import io.trino.plugin.jdbc.JdbcTypeHandle; import io.trino.plugin.jdbc.PreparedQuery; import io.trino.plugin.jdbc.QueryBuilder; +import io.trino.plugin.jdbc.RemoteTableName; import io.trino.plugin.jdbc.WriteMapping; import io.trino.plugin.jdbc.aggregation.ImplementAvgDecimal; import io.trino.plugin.jdbc.aggregation.ImplementAvgFloatingPoint; @@ -78,6 +79,8 @@ import java.util.function.BiFunction; import java.util.stream.Stream; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Strings.emptyToNull; import static com.google.common.base.Verify.verify; import static com.google.common.util.concurrent.MoreExecutors.directExecutor; import static com.mysql.cj.exceptions.MysqlErrorNumbers.SQL_STATE_ER_TABLE_EXISTS_ERROR; @@ -142,6 +145,7 @@ import static java.lang.Math.max; import static java.lang.Math.min; import static java.lang.String.format; +import static java.lang.String.join; import static java.util.Locale.ENGLISH; import static java.util.stream.Collectors.joining; @@ -157,6 +161,9 @@ public class MySqlClient // MySQL driver returns width of time types instead of precision, same as the above timestamp type. private static final int ZERO_PRECISION_TIME_COLUMN_SIZE = 8; + // An empty character means that the table doesn't have a comment in MySQL + private static final String NO_COMMENT = ""; + private final Type jsonType; private final ConnectorExpressionRewriter connectorExpressionRewriter; private final AggregateFunctionRewriter aggregateFunctionRewriter; @@ -273,6 +280,24 @@ public ResultSet getTables(Connection connection, Optional schemaName, O getTableTypes().map(types -> types.toArray(String[]::new)).orElse(null)); } + @Override + public Optional getTableComment(ResultSet resultSet) + throws SQLException + { + // Empty remarks means that the table doesn't have a comment in MySQL + return Optional.ofNullable(emptyToNull(resultSet.getString("REMARKS"))); + } + + @Override + public void setTableComment(ConnectorSession session, JdbcTableHandle handle, Optional comment) + { + String sql = format( + "ALTER TABLE %s COMMENT = '%s'", + quoted(handle.asPlainTable().getRemoteTableName()), + comment.orElse(NO_COMMENT)); // An empty character removes the existing comment in MySQL + execute(session, sql); + } + @Override protected String getTableSchemaName(ResultSet resultSet) throws SQLException @@ -281,6 +306,13 @@ protected String getTableSchemaName(ResultSet resultSet) return resultSet.getString("TABLE_CAT"); } + @Override + protected String createTableSql(RemoteTableName remoteTableName, List columns, ConnectorTableMetadata tableMetadata) + { + checkArgument(tableMetadata.getProperties().isEmpty(), "Unsupported table properties: %s", tableMetadata.getProperties()); + return format("CREATE TABLE %s (%s) COMMENT '%s'", quoted(remoteTableName), join(", ", columns), tableMetadata.getComment().orElse(NO_COMMENT)); + } + @Override public Optional toColumnMapping(ConnectorSession session, Connection connection, JdbcTypeHandle typeHandle) { diff --git a/plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/BaseMySqlConnectorTest.java b/plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/BaseMySqlConnectorTest.java index 80bd165cce73..bba20330e2c0 100644 --- a/plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/BaseMySqlConnectorTest.java +++ b/plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/BaseMySqlConnectorTest.java @@ -62,7 +62,6 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) case SUPPORTS_JOIN_PUSHDOWN_WITH_DISTINCT_FROM: return false; - case SUPPORTS_COMMENT_ON_TABLE: case SUPPORTS_COMMENT_ON_COLUMN: return false; diff --git a/plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java b/plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java index c1eae8e38b59..6ca108fcbdee 100644 --- a/plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java +++ b/plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java @@ -264,6 +264,13 @@ public void renameSchema(ConnectorSession session, String schemaName, String new throw new TrinoException(NOT_SUPPORTED, "This connector does not support renaming schemas"); } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override public Optional toColumnMapping(ConnectorSession session, Connection connection, JdbcTypeHandle typeHandle) { diff --git a/plugin/trino-phoenix/src/main/java/io/trino/plugin/phoenix/PhoenixClient.java b/plugin/trino-phoenix/src/main/java/io/trino/plugin/phoenix/PhoenixClient.java index 779ed27148a7..be6abb7b03e0 100644 --- a/plugin/trino-phoenix/src/main/java/io/trino/plugin/phoenix/PhoenixClient.java +++ b/plugin/trino-phoenix/src/main/java/io/trino/plugin/phoenix/PhoenixClient.java @@ -562,6 +562,13 @@ public WriteMapping toWriteMapping(ConnectorSession session, Type type) throw new TrinoException(NOT_SUPPORTED, "Unsupported column type: " + type.getDisplayName()); } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override public JdbcOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata) { diff --git a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClient.java b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClient.java index 60a20693938a..a0f861e870f8 100644 --- a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClient.java +++ b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClient.java @@ -554,6 +554,13 @@ public WriteMapping toWriteMapping(ConnectorSession session, Type type) throw new TrinoException(NOT_SUPPORTED, "Unsupported column type: " + type.getDisplayName()); } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override public JdbcOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata) { diff --git a/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java b/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java index e284c83ec913..410134c357e4 100644 --- a/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java +++ b/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java @@ -324,6 +324,13 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe } } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override protected void renameTable(ConnectorSession session, String catalogName, String schemaName, String tableName, SchemaTableName newTable) { diff --git a/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java b/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java index f31d0e7dc800..1ee8521ff597 100644 --- a/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java +++ b/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java @@ -36,6 +36,7 @@ import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Types; import java.util.Optional; @@ -94,6 +95,13 @@ public RedshiftClient(BaseJdbcConfig config, ConnectionFactory connectionFactory super(config, "\"", connectionFactory, queryBuilder, identifierMapping); } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override protected void renameTable(ConnectorSession session, String catalogName, String schemaName, String tableName, SchemaTableName newTable) { diff --git a/plugin/trino-singlestore/src/main/java/io/trino/plugin/singlestore/SingleStoreClient.java b/plugin/trino-singlestore/src/main/java/io/trino/plugin/singlestore/SingleStoreClient.java index 028f19b3421d..b86b5641b302 100644 --- a/plugin/trino-singlestore/src/main/java/io/trino/plugin/singlestore/SingleStoreClient.java +++ b/plugin/trino-singlestore/src/main/java/io/trino/plugin/singlestore/SingleStoreClient.java @@ -194,6 +194,13 @@ protected boolean filterSchema(String schemaName) return super.filterSchema(schemaName); } + @Override + public Optional getTableComment(ResultSet resultSet) + { + // Don't return a comment until the connector supports creating tables with comment + return Optional.empty(); + } + @Override public Optional toColumnMapping(ConnectorSession session, Connection connection, JdbcTypeHandle typeHandle) { diff --git a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java index d4378d0009d9..124dd7f2b620 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java @@ -51,7 +51,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.function.Supplier; -import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.IntStream; import java.util.stream.Stream; @@ -115,6 +114,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; @@ -1796,6 +1796,7 @@ public void testCreateTable() assertUpdate("CREATE TABLE " + tableName + " (a bigint, b double, c varchar(50))"); assertTrue(getQueryRunner().tableExists(getSession(), tableName)); assertTableColumnNames(tableName, "a", "b", "c"); + assertNull(getTableComment(getSession().getCatalog().orElseThrow(), getSession().getSchema().orElseThrow(), tableName)); assertUpdate("DROP TABLE " + tableName); assertFalse(getQueryRunner().tableExists(getSession(), tableName)); @@ -1844,6 +1845,7 @@ public void testCreateTableAsSelect() } assertUpdate("CREATE TABLE IF NOT EXISTS " + tableName + " AS SELECT name, regionkey FROM nation", "SELECT count(*) FROM nation"); assertTableColumnNames(tableName, "name", "regionkey"); + assertNull(getTableComment(getSession().getCatalog().orElseThrow(), getSession().getSchema().orElseThrow(), tableName)); assertUpdate("DROP TABLE " + tableName); // Some connectors support CREATE TABLE AS but not the ordinary CREATE TABLE. Let's test CTAS IF NOT EXISTS with a table that is guaranteed to exist. @@ -2016,7 +2018,7 @@ public void testCommentTable() // comment set assertUpdate("COMMENT ON TABLE " + table.getName() + " IS 'new comment'"); assertThat((String) computeActual("SHOW CREATE TABLE " + table.getName()).getOnlyValue()).contains("COMMENT 'new comment'"); - assertThat(getTableComment(table.getName())).isEqualTo("new comment"); + assertThat(getTableComment(catalogName, schemaName, table.getName())).isEqualTo("new comment"); assertThat(query( "SELECT table_name, comment FROM system.metadata.table_comments " + "WHERE catalog_name = '" + catalogName + "' AND " + @@ -2026,39 +2028,34 @@ public void testCommentTable() // comment updated assertUpdate("COMMENT ON TABLE " + table.getName() + " IS 'updated comment'"); - assertThat(getTableComment(table.getName())).isEqualTo("updated comment"); + assertThat(getTableComment(catalogName, schemaName, table.getName())).isEqualTo("updated comment"); // comment set to empty or deleted assertUpdate("COMMENT ON TABLE " + table.getName() + " IS ''"); - assertThat(getTableComment(table.getName())).isIn("", null); // Some storages do not preserve empty comment + assertThat(getTableComment(catalogName, schemaName, table.getName())).isIn("", null); // Some storages do not preserve empty comment // comment deleted assertUpdate("COMMENT ON TABLE " + table.getName() + " IS 'a comment'"); - assertThat(getTableComment(table.getName())).isEqualTo("a comment"); + assertThat(getTableComment(catalogName, schemaName, table.getName())).isEqualTo("a comment"); assertUpdate("COMMENT ON TABLE " + table.getName() + " IS NULL"); - assertThat(getTableComment(table.getName())).isEqualTo(null); + assertThat(getTableComment(catalogName, schemaName, table.getName())).isEqualTo(null); } String tableName = "test_comment_" + randomTableSuffix(); try { // comment set when creating a table assertUpdate("CREATE TABLE " + tableName + "(key integer) COMMENT 'new table comment'"); - assertThat(getTableComment(tableName)).isEqualTo("new table comment"); + assertThat(getTableComment(catalogName, schemaName, tableName)).isEqualTo("new table comment"); } finally { assertUpdate("DROP TABLE IF EXISTS " + tableName); } } - private String getTableComment(String tableName) + private String getTableComment(String catalogName, String schemaName, String tableName) { - // TODO use information_schema.tables.table_comment - String result = (String) computeActual("SHOW CREATE TABLE " + tableName).getOnlyValue(); - Matcher matcher = Pattern.compile("COMMENT '([^']*)'").matcher(result); - if (matcher.find()) { - return matcher.group(1); - } - return null; + String sql = format("SELECT comment FROM system.metadata.table_comments WHERE catalog_name = '%s' AND schema_name = '%s' AND table_name = '%s'", catalogName, schemaName, tableName); + return (String) computeActual(sql).getOnlyValue(); } @Test