Skip to content

Fix ALTER TABLE SETTINGS being parsed per clause - #320

Merged
git-hulk merged 1 commit into
AfterShip:masterfrom
Lance726:fix/alter-table-settings-statement-level
Sep 11, 2026
Merged

Fix ALTER TABLE SETTINGS being parsed per clause#320
git-hulk merged 1 commit into
AfterShip:masterfrom
Lance726:fix/alter-table-settings-statement-level

Conversation

@Lance726

@Lance726 Lance726 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

SETTINGS on ALTER TABLE is a query-level clause: ClickHouse accepts it once, after the last alter clause. It was parsed per clause instead, and implemented by only four of them — ADD COLUMN, ADD INDEX, DETACH PARTITION and DROP PARTITION — so most statements ClickHouse accepts failed to parse:

$ clickhouse-sql-parser -format "ALTER TABLE t MODIFY COLUMN a Int64 SETTINGS alter_sync = 2"
parse statements error: line 1:37 <EOF> or ';' was expected, but got: "SETTINGS"
ALTER TABLE t MODIFY COLUMN a Int64 SETTINGS alter_sync = 2
                                    ^^^^^^^^

Because SETTINGS may only follow the last clause, and any clause can be last, whether a statement parsed came down to which clause happened to be written last. The two statements below are equivalent and both accepted by ClickHouse, but only the second one parsed:

$ clickhouse-sql-parser -format "ALTER TABLE t ADD COLUMN c Int, DROP COLUMN b SETTINGS alter_sync = 2"
parse statements error: line 1:47 <EOF> or ';' was expected, but got: "SETTINGS"
ALTER TABLE t ADD COLUMN c Int, DROP COLUMN b SETTINGS alter_sync = 2
                                              ^^^^^^^^

$ clickhouse-sql-parser -format "ALTER TABLE t DROP COLUMN b, ADD COLUMN c Int SETTINGS alter_sync = 2"
ALTER TABLE t DROP COLUMN b, ADD COLUMN c Int SETTINGS alter_sync=2

SETTINGS is now parsed once in parseAlterTable, after the clause list, so it no longer matters which clause comes last.

Sweeping all 24 alter clauses with a trailing SETTINGS against clickhouse-local 26.8.2.7: 19 forms that ClickHouse accepts now parse and previously did not, 4 already worked and still do, 0 regressions. Mid-statement SETTINGS stays rejected, as ClickHouse rejects it. COMMENT COLUMN still fails, but for an unrelated reason — COMMENT is missing from the clause dispatch altogether, with or without SETTINGS.

Breaking AST change: Settings moves off those four clauses onto AlterTable.Settings. Formatted SQL is unchanged — only the output/ goldens move, not format/.

@Lance726
Lance726 marked this pull request as draft September 4, 2026 03:23
SETTINGS on ALTER TABLE is a query-level clause: ClickHouse accepts it
once, after the last alter clause, and rejects it mid-statement.

  -- rejected by ClickHouse
  ALTER TABLE t ADD COLUMN c Int SETTINGS alter_sync = 2, DROP COLUMN b
  -- accepted by ClickHouse
  ALTER TABLE t ADD COLUMN c Int, DROP COLUMN b SETTINGS alter_sync = 2

It was instead parsed per clause, by ADD COLUMN, ADD INDEX, DETACH
PARTITION and DROP PARTITION only, so every other form failed to parse
even though ClickHouse accepts it:

  ALTER TABLE t MODIFY COLUMN a Int64 SETTINGS alter_sync = 2
  ALTER TABLE t DROP COLUMN b SETTINGS alter_sync = 2
  ALTER TABLE t ADD COLUMN c Int, DROP COLUMN b SETTINGS alter_sync = 2
  ALTER TABLE t MATERIALIZE INDEX IF EXISTS idx SETTINGS mutations_sync = 2

Parse it once in parseAlterTable instead, after the clause list, and
reject it mid-statement as ClickHouse does.

Formatted SQL is unchanged for the clauses that already accepted
SETTINGS: only the AST placement moves, so their format/ goldens do not
change.

Breaking AST change: AlterTableAddColumn.Settings,
AlterTableAddIndex.Settings, AlterTableDetachPartition.Settings and
AlterTableDropPartition.Settings are removed in favour of
AlterTable.Settings.

Every form above was verified against clickhouse-local 26.8.2.7, in both
directions: the SQL parses here, and this parser's formatted output is
accepted by ClickHouse.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Lance726
Lance726 force-pushed the fix/alter-table-settings-statement-level branch from 6b796bc to fc11883 Compare September 11, 2026 05:40
@Lance726 Lance726 changed the title Move ALTER TABLE SETTINGS to the statement level Fix ALTER TABLE SETTINGS being parsed per clause Sep 11, 2026
@Lance726
Lance726 marked this pull request as ready for review September 11, 2026 06:59
@Lance726
Lance726 requested a review from git-hulk September 11, 2026 07:55
@git-hulk
git-hulk merged commit ce5b8b3 into AfterShip:master Sep 11, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants