Skip to content

Support comments when building column definition in ClickHouse #11606

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

Merged
ebyhr merged 1 commit into from
Apr 5, 2022
Merged

Support comments when building column definition in ClickHouse #11606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,9 @@ protected String getColumnDefinitionSql(ConnectorSession session, ColumnMetadata
// By default, the clickhouse column is not allowed to be null
sb.append(toWriteMapping(session, column.getType()).getDataType());
}
if (column.getComment() != null) {
sb.append(format(" COMMENT '%s'", column.getComment()));
}
return sb.toString();
}

Expand All @@ -401,9 +404,6 @@ protected String renameSchemaSql(String remoteSchemaName, String newRemoteSchema
@Override
public void addColumn(ConnectorSession session, JdbcTableHandle handle, ColumnMetadata column)
{
if (column.getComment() != null) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support adding columns with comments");
}
try (Connection connection = connectionFactory.openConnection(session)) {
String remoteColumnName = getIdentifierMapping().toRemoteColumnName(connection, column.getName());
String sql = format(
Expand Down
21 changes: 20 additions & 1 deletion ...rino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,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;

public class TestClickHouseConnectorTest
Expand Down Expand Up @@ -178,7 +179,16 @@ public void testAddColumn()
@Override
public void testAddColumnWithComment()
{
throw new SkipException("Clickhouse connector does not support specifying a comment when adding a column");
// Override because the default storage type doesn't support adding columns
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_add_col_desc_", "(a_varchar varchar NOT NULL) WITH (engine = 'MergeTree', order_by = ARRAY['a_varchar'])")) {
String tableName = table.getName();

assertUpdate("ALTER TABLE " + tableName + " ADD COLUMN b_varchar varchar COMMENT 'test new column comment'");
assertThat(getColumnComment(tableName, "b_varchar")).isEqualTo("test new column comment");

assertUpdate("ALTER TABLE " + tableName + " ADD COLUMN empty_comment varchar COMMENT ''");
assertNull(getColumnComment(tableName, "empty_comment"));
}
}

@Test
Expand Down Expand Up @@ -233,6 +243,15 @@ protected TestTable createTableWithDefaultColumns()
"col_required2 Int64) ENGINE=Log");
}

@Test
public void testCreateTableWithColumnComment()
{
// TODO (https://github.com/trinodb/trino/issues/11162) Merge into BaseConnectorTest
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_column_comment", "(col integer COMMENT 'column comment')")) {
assertEquals(getColumnComment(table.getName(), "col"), "column comment");
}
}

@Override
public void testCharVarcharComparison()
{
Expand Down