Skip to content

fix(pull,drift): parse clauses past column list; treat derived primary key as clean#198

Merged
alvarogar4 merged 1 commit into
mainfrom
alvaro/num-7479-drift-primary-key-and-pull-engine-clause
Jul 20, 2026
Merged

fix(pull,drift): parse clauses past column list; treat derived primary key as clean#198
alvarogar4 merged 1 commit into
mainfrom
alvaro/num-7479-drift-primary-key-and-pull-engine-clause

Conversation

@alvarogar4

Copy link
Copy Markdown
Member

Summary

Fixes #190 and #194 — two compounding bugs on tables whose ORDER BY is declared alongside a projection or a derived primary key. Both verified against live ClickHouse 26.3.

#190chkit pull leaked the engine into orderBy/primaryKey. The clause parsers matched the first ENGINE/ORDER BY/PRIMARY KEY/PARTITION BY/TTL/SETTINGS keyword anywhere in SHOW CREATE TABLE. A projection whose SELECT body contains ORDER BY (the reporter's case) — or a column-level TTL — sits in the column list, before the table-level clauses, so the parser matched the inner keyword and swallowed the engine string into the clause:

orderBy: ["csin", "day)) ENGINE = ReplicatedReplacingMergeTree('/clickhouse/tables/{cluster}/...', '{replica}', _version)"]

Table-level clauses are now parsed only from the portion of the query after the column list.

#194chkit drift always reported primary_key_mismatch for tables whose PRIMARY KEY is derived from ORDER BY. ClickHouse omits the derived key from SHOW CREATE, but the canonicalized schema carries it (canonical.ts), so the live side ('') and schema side ((a, b)) never matched. Drift now applies the same derivation to the live side.

The two compound: fixing #190 makes pull emit a valid ORDER BY-only schema, and #194 makes that schema read clean against its source table.

Test plan

  • typecheck + lint clean; touched packages green (core/clickhouse/plugin-pull/cli)
  • Parser unit tests: the issue Engine in primary key and order by after pull #190 shape (projection SELECT ... ORDER BY, replicated engine with {cluster}/{replica} params) parses every clause correctly; plus a column-level TTL case — both query shapes confirmed against a live instance as what ClickHouse actually emits
  • Drift unit tests: a derived primary key reads clean, and a genuine primary-key difference still reports primary_key_mismatch
  • Mutation-checked — reverting each fix fails its test; restoring passes
  • Full real-CLI e2e against live ClickHouse 26.3: create the Engine in primary key and order by after pull #190-shaped table → pull (clean orderBy/primaryKey, no engine text) → generatedrift reports no

Scope

Kept to #190 and #194. The related #196 (parser doesn't track backtick identifiers) and #197 (duplicated paren-stripping in plugin-pull) are untouched — #196 is a separate, rarer parse gap, and #197's helper backs key-clause parsing that this PR doesn't need to move.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Biy2vG7iixRiBsBBsFg5Nu

…ved primary key as clean

Two related bugs on tables whose ORDER BY sits alongside a projection or a
derived primary key. Both verified against live ClickHouse 26.3.

#190 — chkit pull leaked the engine into orderBy/primaryKey. The clause
parsers matched the first ENGINE/ORDER BY/PRIMARY KEY/PARTITION BY/TTL/
SETTINGS keyword anywhere in SHOW CREATE TABLE. A projection whose SELECT
body contains ORDER BY (or a column-level TTL) sits in the column list,
before the table-level clauses, so the parser matched the inner keyword and
swallowed the engine into the clause. Table-level clauses are now parsed
only from the portion after the column list.

#194 — chkit drift always reported primary_key_mismatch for tables whose
PRIMARY KEY is derived from ORDER BY. ClickHouse omits the derived key from
SHOW CREATE, but the canonicalized schema carries it, so the two never
matched. Drift now applies the same derivation to the live side; a genuine
primary-key difference is still reported.

The two compound: fixing #190 makes pull emit a valid ORDER BY-only schema,
and #194 makes that schema read clean against its source table.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Biy2vG7iixRiBsBBsFg5Nu
@alvarogar4
alvarogar4 merged commit 8296b8a into main Jul 20, 2026
6 checks passed
@alvarogar4
alvarogar4 deleted the alvaro/num-7479-drift-primary-key-and-pull-engine-clause branch July 20, 2026 10:29
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.

Engine in primary key and order by after pull

1 participant