Skip to content

Commit

Permalink
Deny creating tables with column comment if unsupported
Browse files Browse the repository at this point in the history
  • Loading branch information
ebyhr committed May 28, 2022
1 parent 7f9057e commit 4911312
Show file tree
Hide file tree
Showing 29 changed files with 95 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import static io.trino.plugin.accumulo.AccumuloQueryRunner.createAccumuloQueryRunner;
import static io.trino.spi.type.VarcharType.VARCHAR;
import static io.trino.testing.MaterializedResult.resultBuilder;
import static io.trino.testing.sql.TestTable.randomTableSuffix;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.testng.Assert.assertEquals;
Expand Down Expand Up @@ -98,6 +99,22 @@ protected TestTable createTableWithDefaultColumns()
throw new SkipException("Accumulo connector does not support column default values");
}

@Override
public void testCreateTableWithColumnComment()
{
// TODO Avoid setting hard-coded column comment
// Accumulo connector ignores specified comment and sets column comments as
// "Accumulo row ID" for the first column when "row_id" table property isn't specified
// "Accumulo column %s:%s. Indexed: boolean" for other columns
String tableName = "test_create_" + randomTableSuffix();
assertUpdate("CREATE TABLE " + tableName + " (a bigint COMMENT 'test comment a', b bigint COMMENT 'test comment b')");

assertEquals(getColumnComment(tableName, "a"), "Accumulo row ID");
assertEquals(getColumnComment(tableName, "b"), "Accumulo column b:b. Indexed: false");

assertUpdate("DROP TABLE " + tableName);
}

@Override
public void testCreateTableAsSelect()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,9 @@ protected String createTableSql(RemoteTableName remoteTableName, List<String> co

protected String getColumnDefinitionSql(ConnectorSession session, ColumnMetadata column, String columnName)
{
if (column.getComment() != null) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}
StringBuilder sb = new StringBuilder()
.append(quoted(columnName))
.append(" ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return false;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_COMMENT_ON_TABLE:
case SUPPORTS_COMMENT_ON_COLUMN:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,9 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe
if (tableMetadata.getComment().isPresent()) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with table comment");
}
if (tableMetadata.getColumns().stream().anyMatch(column -> column.getComment() != null)) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}
try {
createTable(session, tableMetadata);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
case SUPPORTS_NOT_NULL_CONSTRAINT:
case SUPPORTS_CREATE_TABLE_WITH_DATA:
case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_DELETE:
case SUPPORTS_INSERT:
case SUPPORTS_ADD_COLUMN:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,9 @@ private CassandraOutputTableHandle createTable(ConnectorTableMetadata tableMetad
ImmutableList.Builder<ExtraColumnMetadata> columnExtra = ImmutableList.builder();
columnExtra.add(new ExtraColumnMetadata(ID_COLUMN_NAME, true));
for (ColumnMetadata column : tableMetadata.getColumns()) {
if (column.getComment() != null) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}
columnNames.add(column.getName());
columnTypes.add(column.getType());
columnExtra.add(new ExtraColumnMetadata(column.getName(), column.isHidden()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return false;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
return false;

case SUPPORTS_RENAME_TABLE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,10 @@ public void dropSchema(ConnectorSession session, String schemaName)
@Override
public void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, boolean ignoreExisting)
{
if (tableMetadata.getColumns().stream().anyMatch(column -> column.getComment() != null)) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}

SchemaTableName schemaTableName = tableMetadata.getTable();
String schemaName = schemaTableName.getSchemaName();
String tableName = schemaTableName.getTableName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
case SUPPORTS_AGGREGATION_PUSHDOWN:
case SUPPORTS_RENAME_TABLE:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_DROP_COLUMN:
case SUPPORTS_RENAME_COLUMN:
case SUPPORTS_COMMENT_ON_TABLE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,9 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe
if (tableMetadata.getComment().isPresent()) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with table comment");
}
if (tableMetadata.getColumns().stream().anyMatch(column -> column.getComment() != null)) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}
clientSession.createTable(tableMetadata, ignoreExisting);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,22 @@ public void testCreateTable()
throw new SkipException("TODO");
}

@Override
public void testCreateTableWithColumnComment()
{
// TODO https://github.com/trinodb/trino/issues/12469 Support column comment when creating tables
String tableName = "test_create_" + randomTableSuffix();

assertQueryFails(
"CREATE TABLE " + tableName + "(" +
"id INT WITH (primary_key=true)," +
"a VARCHAR COMMENT 'test comment')" +
"WITH (partition_by_hash_columns = ARRAY['id'], partition_by_hash_buckets = 2)",
"This connector does not support creating tables with column comment");

assertUpdate("DROP TABLE IF EXISTS " + tableName);
}

@Override
public void testDropTable()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
case SUPPORTS_AGGREGATION_PUSHDOWN_STDDEV:
case SUPPORTS_AGGREGATION_PUSHDOWN_VARIANCE:
return true;
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
case SUPPORTS_JOIN_PUSHDOWN_WITH_FULL_JOIN:
case SUPPORTS_JOIN_PUSHDOWN_WITH_DISTINCT_FROM:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ public synchronized MemoryOutputTableHandle beginCreateTable(ConnectorSession se
ImmutableList.Builder<ColumnInfo> columns = ImmutableList.builder();
for (int i = 0; i < tableMetadata.getColumns().size(); i++) {
ColumnMetadata column = tableMetadata.getColumns().get(i);
if (column.getComment() != null) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}
columns.add(new ColumnInfo(new MemoryColumnHandle(i), column.getName(), column.getType()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return false;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_COMMENT_ON_TABLE:
case SUPPORTS_COMMENT_ON_COLUMN:
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,6 @@ public Object[][] guessFieldTypesProvider()
};
}

@Test
public void testCreateTableWithColumnComment()
{
// TODO (https://github.com/trinodb/trino/issues/11162) Merge into io.trino.testing.BaseConnectorTest#testCommentColumn
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_column_comment", "(col integer COMMENT 'test')")) {
assertThat((String) computeScalar("SHOW CREATE TABLE " + table.getName()))
.isEqualTo(format("CREATE TABLE %s.%s.%s (\n" +
" col integer COMMENT 'test'\n" +
")", getSession().getCatalog().orElseThrow(), getSession().getSchema().orElseThrow(), table.getName()));
}
}

@Test
public void createTableWithEveryType()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return false;

case SUPPORTS_COMMENT_ON_COLUMN:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
return false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
case SUPPORTS_COMMENT_ON_TABLE:
return false;

case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
return false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,9 @@ public JdbcOutputTableHandle beginCreateTable(ConnectorSession session, Connecto
rowkeyColumn = Optional.of(ROWKEY);
}
for (ColumnMetadata column : tableColumns) {
if (column.getComment() != null) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}
String columnName = getIdentifierMapping().toRemoteColumnName(connection, column.getName());
columnNames.add(columnName);
columnTypes.add(column.getType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return false;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_COMMENT_ON_TABLE:
case SUPPORTS_COMMENT_ON_COLUMN:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,9 @@ public JdbcOutputTableHandle beginCreateTable(ConnectorSession session, Connecto
rowkeyColumn = Optional.of(ROWKEY);
}
for (ColumnMetadata column : tableColumns) {
if (column.getComment() != null) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}
String columnName = getIdentifierMapping().toRemoteColumnName(connection, column.getName());
columnNames.add(columnName);
columnTypes.add(column.getType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return false;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_COMMENT_ON_TABLE:
case SUPPORTS_COMMENT_ON_COLUMN:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return true;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_COMMENT_ON_TABLE:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,9 @@ public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, Con

long columnId = 1;
for (ColumnMetadata column : tableMetadata.getColumns()) {
if (column.getComment() != null) {
throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with column comment");
}
columnHandles.add(new RaptorColumnHandle(column.getName(), columnId, column.getType()));
columnTypes.add(column.getType());
columnId++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
case SUPPORTS_CREATE_SCHEMA:
case SUPPORTS_RENAME_SCHEMA:
case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_COMMENT_ON_TABLE:
case SUPPORTS_COMMENT_ON_COLUMN:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return false;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
case SUPPORTS_COMMENT_ON_TABLE:
case SUPPORTS_COMMENT_ON_COLUMN:
case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
return false;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
return false;

case SUPPORTS_COMMENT_ON_TABLE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_MATERIALIZED_VIEW;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_SCHEMA;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_TABLE;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_TABLE_WITH_DATA;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_VIEW;
Expand Down Expand Up @@ -1910,6 +1911,24 @@ public void testCreateTableWithTableComment()
assertUpdate("DROP TABLE " + tableName);
}

@Test
public void testCreateTableWithColumnComment()
{
skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE));

String tableName = "test_create_" + randomTableSuffix();

if (!hasBehavior(SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT)) {
assertQueryFails("CREATE TABLE " + tableName + " (a bigint COMMENT 'test comment')", "This connector does not support creating tables with column comment");
return;
}

assertUpdate("CREATE TABLE " + tableName + " (a bigint COMMENT 'test comment')");
assertEquals(getColumnComment(tableName, "a"), "test comment");

assertUpdate("DROP TABLE " + tableName);
}

@Test
public void testCreateTableSchemaNotFound()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public enum TestingConnectorBehavior
SUPPORTS_CREATE_TABLE,
SUPPORTS_CREATE_TABLE_WITH_DATA(SUPPORTS_CREATE_TABLE),
SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT(SUPPORTS_CREATE_TABLE),
SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT(SUPPORTS_CREATE_TABLE),
SUPPORTS_RENAME_TABLE,
SUPPORTS_RENAME_TABLE_ACROSS_SCHEMAS(SUPPORTS_RENAME_TABLE),

Expand Down

0 comments on commit 4911312

Please sign in to comment.