Skip to content

TableEditorImpl.reorderColumn duplicates a column when moving a mixed-case column after the last column #2501

Description

@TimurRakhmatullin86

Bug report

TableEditorImpl.reorderColumn(...) corrupts the column list when a column whose name contains an uppercase letter is moved after the current last column.

sortedColumns is a LinkedHashMap keyed by the lower-cased column name everywhere in the class — add() (put(defn.name().toLowerCase(), ...)), columnWithName() (get(name.toLowerCase())), removeColumn(), replaceColumn(), and the else branch of reorderColumn itself. Only the "just append" fast-path uses the raw name:

if (afterColumn != null && afterColumn.position() == sortedColumns.size()) {
    // Just append ...
    sortedColumns.remove(columnName);            // raw name, not lower-cased
    sortedColumns.put(columnName, columnToMove);  // raw name, not lower-cased
}
else {
    sortedColumns.remove(columnName.toLowerCase());     // else branch lower-cases
    ...
    newColumns.put(columnToMove.name().toLowerCase(), columnToMove);
    ...
}

Reproduction

Table with columns C1, C2, C3 (keys c1, c2, c3). reorderColumn("C1", "C3") (move C1 after the last column):

  1. append branch is taken (afterColumn.position() == size);
  2. remove("C1") removes nothing (the key is c1);
  3. put("C1", ...) inserts a new key → the map now has 4 entries: [C1, C2, C3, C1]C1 duplicated and never actually moved.

Expected [C2, C3, C1] (3 columns). With all-lowercase names the same call works, confirming it's purely the key casing.

Production impact (MySQL / MariaDB)

The Antlr DDL parsers preserve identifier case, and AlterTableParserListener.handleExitModifyColumn/handleExitChangeColumn call reorderColumn(column.name(), afterColumn). So:

CREATE TABLE t (Id INT, Foo VARCHAR(10), Bar INT);
ALTER TABLE t MODIFY COLUMN Foo VARCHAR(20) AFTER Bar;   -- Bar is the last column

duplicates the Foo column. TableSchemaBuilder then builds two Connect fields named Foo, throwing SchemaBuilderException: ... field name duplication and halting the connector on an otherwise-valid ALTER.

Fix

Lower-case the key in the append branch, mirroring the rest of the class:

sortedColumns.remove(columnName.toLowerCase());
sortedColumns.put(columnToMove.name().toLowerCase(), columnToMove);

Coverage

TableEditorTest.shouldReorderColumns() uses uppercase names but never moves a column to after the last one, so the append branch is untested. A unit test that moves a column after the last column exposes the 4-column result. PR with fix + test to follow.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions