Move ALTER TABLE SETTINGS to the statement level - #320
Draft
Lance726 wants to merge 2 commits into
Draft
Conversation
…ment
ADD INDEX had no SETTINGS clause, so a statement such as
ALTER TABLE t ADD INDEX IF NOT EXISTS idx c TYPE minmax
GRANULARITY 1 SETTINGS alter_sync = 2
failed to parse, while ADD COLUMN and the partition clauses already
accepted one. Parse it the same way they do.
The formatter also wrote the clause as `ADD IF NOT EXISTS INDEX`, which
ClickHouse rejects, so formatted output could not be parsed again. Split
the part of TableIndex.FormatSQL that follows the INDEX keyword so
ALTER TABLE can emit `ADD INDEX IF NOT EXISTS` instead.
Both forms were verified against clickhouse-local 26.8.2.7.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
marked this pull request as draft
September 4, 2026 03:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SETTINGSonALTER TABLEis query-level: ClickHouse accepts it once, after the last alter clause, and rejects it mid-statement.It was parsed per clause instead, and only by
ADD COLUMN,ADD INDEX,DETACH PARTITIONandDROP PARTITION. Everything else ClickHouse accepts failed to parse:It is now parsed once in
parseAlterTable, after the clause list, and rejected mid-statement as ClickHouse does.Breaking AST change:
Settingsmoves off those four clauses ontoAlterTable.Settings. Formatted SQL is unchanged — only theoutput/goldens move, notformat/.Verified against
clickhouse-local26.8.2.7.