Verify column name length in PostgreSQL and SQL Server

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcClient.java

@@ -565,6 +565,7 @@ protected JdbcOutputTableHandle createTable(ConnectorSession session, ConnectorT
 
             for (ColumnMetadata column : columns) {
                 String columnName = identifierMapping.toRemoteColumnName(connection, column.getName());
+                verifyColumnName(connection.getMetaData(), columnName);
                 columnNames.add(columnName);
                 columnTypes.add(column.getType());
                 columnList.add(getColumnDefinitionSql(session, column, columnName));
@@ -762,6 +763,7 @@ public void addColumn(ConnectorSession session, JdbcTableHandle handle, ColumnMe
 
         try (Connection connection = connectionFactory.openConnection(session)) {
             String columnName = column.getName();
+            verifyColumnName(connection.getMetaData(), columnName);
             String remoteColumnName = identifierMapping.toRemoteColumnName(connection, columnName);
             String sql = format(
                     "ALTER TABLE %s ADD %s",
@@ -779,18 +781,24 @@ public void renameColumn(ConnectorSession session, JdbcTableHandle handle, JdbcC
     {
         try (Connection connection = connectionFactory.openConnection(session)) {
             String newRemoteColumnName = identifierMapping.toRemoteColumnName(connection, newColumnName);
-            String sql = format(
-                    "ALTER TABLE %s RENAME COLUMN %s TO %s",
-                    quoted(handle.asPlainTable().getRemoteTableName()),
-                    jdbcColumn.getColumnName(),
-                    newRemoteColumnName);
+            verifyColumnName(connection.getMetaData(), newRemoteColumnName);
+            String sql = renameColumnSql(handle, jdbcColumn, newRemoteColumnName);
             execute(connection, sql);
         }
         catch (SQLException e) {
             throw new TrinoException(JDBC_ERROR, e);
         }
     }
 
+    protected String renameColumnSql(JdbcTableHandle handle, JdbcColumnHandle jdbcColumn, String newRemoteColumnName)
+    {
+        return format(
+                "ALTER TABLE %s RENAME COLUMN %s TO %s",
+                quoted(handle.asPlainTable().getRemoteTableName()),
+                jdbcColumn.getColumnName(),
+                newRemoteColumnName);
+    }
+
     @Override
     public void dropColumn(ConnectorSession session, JdbcTableHandle handle, JdbcColumnHandle column)
     {
@@ -1098,6 +1106,12 @@ protected void verifyTableName(DatabaseMetaData databaseMetadata, String tableNa
         // expect remote databases throw an exception for unsupported table names
     }
 
+    protected void verifyColumnName(DatabaseMetaData databaseMetadata, String columnName)
+            throws SQLException
+    {
+        // expect remote databases throw an exception for unsupported column names
+    }
+
     protected String quoted(@Nullable String catalog, @Nullable String schema, String table)
     {
         StringBuilder sb = new StringBuilder();

plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryConnectorTest.java

@@ -880,6 +880,19 @@ public void testCharVarcharComparison()
                 .hasMessage("Unsupported column type: char(3)");
     }
 
+    @Override
+    protected OptionalInt maxColumnNameLength()
+    {
+        return OptionalInt.of(300);
+    }
+
+    @Override
+    protected void verifyColumnNameLengthFailurePermissible(Throwable e)
+    {
+        assertThat(e)
+                .hasMessageContaining("Fields must contain only letters, numbers, and underscores, start with a letter or underscore, and be at most 300 characters long.");
+    }
+
     private void onBigQuery(@Language("SQL") String sql)
     {
         bigQuerySqlExecutor.execute(sql);

plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/TestCassandraConnectorTest.java

@@ -554,6 +554,15 @@ public void testInsertToTableWithHiddenId()
         execute("DROP TABLE test_create_table");
     }
 
+    @Override
+    public void testCreateTableWithLongColumnName()
+    {
+        // TODO: Find the maximum column name length in Cassandra and enable this test.
+        assertThatThrownBy(super::testCreateTableWithLongColumnName)
+                .hasMessageMatching(".* Mutation of .* bytes is too large.*");
+        throw new SkipException("TODO");
+    }
+
     @Test
     public void testCreateTableAs()
     {

plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java

@@ -425,19 +425,12 @@ public void addColumn(ConnectorSession session, JdbcTableHandle handle, ColumnMe
     }
 
     @Override
-    public void renameColumn(ConnectorSession session, JdbcTableHandle handle, JdbcColumnHandle jdbcColumn, String newColumnName)
+    protected String renameColumnSql(JdbcTableHandle handle, JdbcColumnHandle jdbcColumn, String newRemoteColumnName)
     {
-        try (Connection connection = connectionFactory.openConnection(session)) {
-            String newRemoteColumnName = getIdentifierMapping().toRemoteColumnName(connection, newColumnName);
-            String sql = format("ALTER TABLE %s RENAME COLUMN %s TO %s ",
-                    quoted(handle.getRemoteTableName()),
-                    quoted(jdbcColumn.getColumnName()),
-                    quoted(newRemoteColumnName));
-            execute(connection, sql);
-        }
-        catch (SQLException e) {
-            throw new TrinoException(JDBC_ERROR, e);
-        }
+        return format("ALTER TABLE %s RENAME COLUMN %s TO %s ",
+                quoted(handle.getRemoteTableName()),
+                quoted(jdbcColumn.getColumnName()),
+                quoted(newRemoteColumnName));
     }
 
     @Override

plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/BaseClickHouseConnectorTest.java

@@ -200,6 +200,20 @@ public void testAddColumnWithComment()
         }
     }
 
+    @Override
+    public void testAlterTableAddLongColumnName()
+    {
+        // TODO: Find the maximum column name length in ClickHouse and enable this test.
+        throw new SkipException("TODO");
+    }
+
+    @Override
+    public void testAlterTableRenameColumnToLongName()
+    {
+        // TODO: Find the maximum column name length in ClickHouse and enable this test.
+        throw new SkipException("TODO");
+    }
+
     @Test
     @Override
     public void testShowCreateTable()

plugin/trino-druid/src/test/java/io/trino/plugin/druid/BaseDruidConnectorTest.java

@@ -70,6 +70,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
             case SUPPORTS_CREATE_TABLE:
             case SUPPORTS_CREATE_TABLE_WITH_DATA:
             case SUPPORTS_ADD_COLUMN:
+            case SUPPORTS_RENAME_COLUMN:
             case SUPPORTS_RENAME_TABLE:
             case SUPPORTS_COMMENT_ON_COLUMN:
             case SUPPORTS_COMMENT_ON_TABLE:

plugin/trino-elasticsearch/src/test/java/io/trino/plugin/elasticsearch/BaseElasticsearchConnectorTest.java

@@ -91,6 +91,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
                 return false;
 
             case SUPPORTS_ADD_COLUMN:
+            case SUPPORTS_RENAME_COLUMN:
                 return false;
 
             case SUPPORTS_CREATE_SCHEMA:

plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java

@@ -418,6 +418,32 @@ public void testCreateTableWithLongTableName()
         assertFalse(getQueryRunner().tableExists(getSession(), validTableName));
     }
 
+    @Override
+    public void testCreateTableWithLongColumnName()
+    {
+        // Overridden because DDL in base class can't create Kudu table due to lack of primary key and required table properties
+        String tableName = "test_long_column" + randomTableSuffix();
+        String basColumnName = "col";
+
+        int maxLength = maxColumnNameLength().orElseThrow();
+
+        String validColumnName = basColumnName + "z".repeat(maxLength - basColumnName.length());
+        assertUpdate("CREATE TABLE " + tableName + " (" +
+                "id INT WITH (primary_key=true)," +
+                validColumnName + " bigint)" +
+                "WITH (partition_by_hash_columns = ARRAY['id'], partition_by_hash_buckets = 2)");
+        assertTrue(columnExists(tableName, validColumnName));
+        assertUpdate("DROP TABLE " + tableName);
+
+        String invalidColumnName = validColumnName + "z";
+        assertThatThrownBy(() -> assertUpdate("CREATE TABLE " + tableName + " (" +
+                "id INT WITH (primary_key=true)," +
+                invalidColumnName + " bigint)" +
+                "WITH (partition_by_hash_columns = ARRAY['id'], partition_by_hash_buckets = 2)"))
+                .satisfies(this::verifyColumnNameLengthFailurePermissible);
+        assertFalse(getQueryRunner().tableExists(getSession(), tableName));
+    }
+
     @Override
     public void testCreateTableWithColumnComment()
     {
@@ -692,6 +718,18 @@ protected void verifyTableNameLengthFailurePermissible(Throwable e)
         assertThat(e).hasMessageContaining("invalid table name: identifier");
     }
 
+    @Override
+    protected OptionalInt maxColumnNameLength()
+    {
+        return OptionalInt.of(256);
+    }
+
+    @Override
+    protected void verifyColumnNameLengthFailurePermissible(Throwable e)
+    {
+        assertThat(e).hasMessageContaining("invalid column name: identifier");
+    }
+
     private void assertTableProperty(String tableProperties, String key, String regexValue)
     {
         assertTrue(Pattern.compile(key + "\\s*=\\s*" + regexValue + ",?\\s+").matcher(tableProperties).find(),

plugin/trino-mariadb/src/main/java/io/trino/plugin/mariadb/MariaDbClient.java

@@ -150,6 +150,9 @@ public class MariaDbClient
     // An empty character means that the table doesn't have a comment in MariaDB
     private static final String NO_COMMENT = "";
 
+    // MariaDB Error Codes https://mariadb.com/kb/en/mariadb-error-codes/
+    private static final int PARSE_ERROR = 1064;
+
     private final AggregateFunctionRewriter<JdbcExpression, String> aggregateFunctionRewriter;
 
     @Inject
@@ -457,7 +460,8 @@ public void renameColumn(ConnectorSession session, JdbcTableHandle handle, JdbcC
             execute(connection, sql);
         }
         catch (TrinoException e) {
-            if (e.getCause() instanceof SQLSyntaxErrorException) {
+            // Note: SQLSyntaxErrorException can be thrown also when column name is invalid
+            if (e.getCause() instanceof SQLSyntaxErrorException syntaxError && syntaxError.getErrorCode() == PARSE_ERROR) {
                 throw new TrinoException(NOT_SUPPORTED, "Rename column not supported for the MariaDB server version", e);
             }
             throw e;

plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/BaseMariaDbConnectorTest.java

@@ -353,4 +353,16 @@ protected void verifyTableNameLengthFailurePermissible(Throwable e)
     {
         assertThat(e).hasMessageContaining("Incorrect table name");
     }
+
+    @Override
+    protected OptionalInt maxColumnNameLength()
+    {
+        return OptionalInt.of(64);
+    }
+
+    @Override
+    protected void verifyColumnNameLengthFailurePermissible(Throwable e)
+    {
+        assertThat(e).hasMessageMatching("(.*Identifier name '.*' is too long|.*Incorrect column name.*)");
+    }
 }

plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbConnectorTest.java

@@ -43,4 +43,11 @@ public void testRenameColumn()
         assertThatThrownBy(super::testRenameColumn)
                 .hasMessageContaining("Rename column not supported for the MariaDB server version");
     }
+
+    @Override
+    public void testAlterTableRenameColumnToLongName()
+    {
+        assertThatThrownBy(super::testAlterTableRenameColumnToLongName)
+                .hasMessageContaining("Rename column not supported for the MariaDB server version");
+    }
 }

plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/BaseMySqlConnectorTest.java

@@ -407,6 +407,12 @@ protected void verifyTableNameLengthFailurePermissible(Throwable e)
         assertThat(e).hasMessageMatching("Identifier name .* is too long");
     }
 
+    @Override
+    protected OptionalInt maxColumnNameLength()
+    {
+        return OptionalInt.of(64);
+    }
+
     @Override
     protected SqlExecutor onRemoteDatabase()
     {

plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/TestMySqlConnectorTest.java

@@ -18,6 +18,7 @@
 import org.testng.annotations.Test;
 
 import static io.trino.plugin.mysql.MySqlQueryRunner.createMySqlQueryRunner;
+import static org.assertj.core.api.Assertions.assertThat;
 
 public class TestMySqlConnectorTest
         extends BaseMySqlConnectorTest
@@ -40,4 +41,10 @@ public void testDateYearOfEraPredicate()
                 "SELECT * FROM orders WHERE orderdate = DATE '-1996-09-14'",
                 "Incorrect DATE value: '-1996-09-14'");
     }
+
+    @Override
+    protected void verifyColumnNameLengthFailurePermissible(Throwable e)
+    {
+        assertThat(e).hasMessageMatching("(Incorrect column name '.*'|Identifier name '.*' is too long)");
+    }
 }

plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/TestMySqlLegacyConnectorTest.java

@@ -135,6 +135,14 @@ public void testRenameColumn()
                 .hasStackTraceContaining("RENAME COLUMN x TO before_y");
     }
 
+    @Override
+    public void testAlterTableRenameColumnToLongName()
+    {
+        assertThatThrownBy(super::testAlterTableRenameColumnToLongName)
+                .hasMessageContaining("You have an error in your SQL syntax")
+                .hasStackTraceContaining("RENAME COLUMN x");
+    }
+
     @Override
     protected Optional<DataMappingTestSetup> filterDataMappingSmokeTestData(DataMappingTestSetup dataMappingTestSetup)
     {
@@ -145,4 +153,10 @@ protected Optional<DataMappingTestSetup> filterDataMappingSmokeTestData(DataMapp
         }
         return super.filterDataMappingSmokeTestData(dataMappingTestSetup);
     }
+
+    @Override
+    protected void verifyColumnNameLengthFailurePermissible(Throwable e)
+    {
+        assertThat(e).hasMessageMatching("Identifier name .* is too long");
+    }
 }

plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/BaseOracleConnectorTest.java

@@ -506,6 +506,18 @@ protected void verifyTableNameLengthFailurePermissible(Throwable e)
         assertThat(e).hasMessage("ORA-00972: identifier is too long\n");
     }
 
+    @Override
+    protected OptionalInt maxColumnNameLength()
+    {
+        return OptionalInt.of(30);
+    }
+
+    @Override
+    protected void verifyColumnNameLengthFailurePermissible(Throwable e)
+    {
+        assertThat(e).hasMessage("ORA-00972: identifier is too long\n");
+    }
+
     private void predicatePushdownTest(String oracleType, String oracleLiteral, String operator, String filterLiteral)
     {
         String tableName = ("test_pdown_" + oracleType.replaceAll("[^a-zA-Z0-9]", ""))

plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixConnectorTest.java

@@ -138,6 +138,15 @@ public void testRenameColumn()
         throw new SkipException("Rename column is not yet supported by Phoenix connector");
     }
 
+    @Override
+    public void testAlterTableRenameColumnToLongName()
+    {
+        assertThatThrownBy(super::testAlterTableRenameColumnToLongName)
+                // TODO (https://github.com/trinodb/trino/issues/7205) support column rename in Phoenix
+                .hasMessageContaining("Syntax error. Encountered \"RENAME\"");
+        throw new SkipException("Rename column is not yet supported by Phoenix connector");
+    }
+
     @Override
     public void testInsert()
     {
@@ -680,6 +689,20 @@ public void testCreateTableWithLongTableName()
         throw new SkipException("TODO");
     }
 
+    @Override
+    public void testCreateTableWithLongColumnName()
+    {
+        // TODO: Find the maximum column name length in Phoenix and enable this test.
+        throw new SkipException("TODO");
+    }
+
+    @Override
+    public void testAlterTableAddLongColumnName()
+    {
+        // TODO: Find the maximum column name length in Phoenix and enable this test.
+        throw new SkipException("TODO");
+    }
+
     @Override
     protected OptionalInt maxTableNameLength()
     {

plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java

@@ -1030,6 +1030,17 @@ protected void verifyTableName(DatabaseMetaData databaseMetadata, String tableNa
         }
     }
 
+    @Override
+    protected void verifyColumnName(DatabaseMetaData databaseMetadata, String columnName)
+            throws SQLException
+    {
+        // PostgreSQL truncates table name to 63 chars silently
+        // PostgreSQL driver caches the max column name length in a DatabaseMetaData object. The cost to call this method per column is low.
+        if (columnName.length() > databaseMetadata.getMaxColumnNameLength()) {
+            throw new TrinoException(NOT_SUPPORTED, format("Column name must be shorter than or equal to '%s' characters but got '%s': '%s'", databaseMetadata.getMaxColumnNameLength(), columnName.length(), columnName));
+        }
+    }
+
     private static ColumnMapping charColumnMapping(int charLength)
     {
         if (charLength > CharType.MAX_LENGTH) {