Fix ALTER TABLE SETTINGS being parsed per clause - #320
Merged
git-hulk merged 1 commit intoSep 11, 2026
Merged
Conversation
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
force-pushed
the
fix/alter-table-settings-statement-level
branch
from
September 11, 2026 05:40
6b796bc to
fc11883
Compare
Lance726
marked this pull request as ready for review
September 11, 2026 06:59
git-hulk
approved these changes
Sep 11, 2026
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
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 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 PARTITIONandDROP PARTITION— so most statements ClickHouse accepts failed to parse:Because
SETTINGSmay 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:SETTINGSis now parsed once inparseAlterTable, after the clause list, so it no longer matters which clause comes last.Sweeping all 24 alter clauses with a trailing
SETTINGSagainstclickhouse-local26.8.2.7: 19 forms that ClickHouse accepts now parse and previously did not, 4 already worked and still do, 0 regressions. Mid-statementSETTINGSstays rejected, as ClickHouse rejects it.COMMENT COLUMNstill fails, but for an unrelated reason —COMMENTis missing from the clause dispatch altogether, with or withoutSETTINGS.Breaking AST change:
Settingsmoves off those four clauses ontoAlterTable.Settings. Formatted SQL is unchanged — only theoutput/goldens move, notformat/.