fix(drift): compare SQL expressions in ClickHouse's canonical form

.changeset/drift-ch-canonical-expressions.md

@@ -0,0 +1,8 @@
+---
+"@chkit/clickhouse": patch
+"chkit": patch
+---
+
+`chkit drift` no longer reports false drift when a schema expression is spelled differently from how ClickHouse stores it. ClickHouse reformats expressions on the way in — spacing argument separators and operators, adding precedence parentheses, and rewriting `INTERVAL 5 YEAR` to `toIntervalYear(5)` — so a skip index, `PARTITION BY`, `ORDER BY`, or `TTL` written as `cityHash64(a,b)` was stored as `cityHash64(a, b)` and reported as permanent drift that no migration could fix (#195).
+
+`drift` now normalizes expressions through ClickHouse's own formatter (`formatQuerySingleLine`) before comparing, in a single batched round-trip, so equivalent expressions match regardless of spelling. When the connected server can't format a fragment — an older server without the function, or an expression it can't parse — that fragment falls back to plain text comparison, so behavior is never worse than before. `@chkit/clickhouse` gains `canonicalizeSqlFragments` for this.

apps/docs/src/content/docs/cli/drift.md

@@ -33,6 +33,10 @@ Global flags documented on [CLI Overview](/cli/overview/#global-flags).
 
 When comparing engines, `SharedMergeTree` is normalized to `MergeTree`. This prevents false positives on managed environments (e.g. [ObsessionDB](https://obsessiondb.com)) where the server transparently substitutes `SharedMergeTree` for `MergeTree`.
 
+### Expression normalization
+
+ClickHouse rewrites SQL expressions when it stores them — it spaces argument separators and operators, adds precedence parentheses, and rewrites `INTERVAL 5 YEAR` to `toIntervalYear(5)`. So a skip index, `PARTITION BY`, `ORDER BY`, or `TTL` expression written as `cityHash64(a,b)` is stored as `cityHash64(a, b)`. To avoid reporting these as drift, `drift` normalizes expressions through ClickHouse's own formatter before comparing, so equivalent expressions match regardless of spelling. When the connected server can't format an expression, comparison falls back to plain text matching.
+
 ### Drift reason codes
 
 **Object-level drift:**

packages/cli/src/commands/drift/compare.ts

@@ -3,13 +3,32 @@
   isIndexProjection,
   normalizeProjectionIndex,
   normalizeSQLFragment,
+  splitTopLevelComma,
   type ColumnDefinition,
   type ProjectionDefinition,
   type SkipIndexDefinition,
   type TableDefinition,
 } from '@chkit/core'
 import { diffByName, diffNamedShapeMaps, diffSettings } from './diff.js'
 
+/**
+ * Canonicalizes SQL fragments to the exact form ClickHouse stores, so a schema
+ * fragment compares equal to what a live table reports even when the two are
+ * spelled differently (`cityHash64(a,b)` vs `cityHash64(a, b)`, `n*2+1` vs
+ * `(n * 2) + 1`, `INTERVAL 5 YEAR` vs `toIntervalYear(5)`). Only ClickHouse's own
+ * formatter can produce this, so it is injected by the drift command; when it is
+ * absent (offline, or a fragment ClickHouse couldn't parse) each field falls
+ * back to plain string normalization (#195).
+ */
+export interface SqlCanonicalizer {
+  expression(fragment: string): string | null
+  query(fragment: string): string | null
+}
+
+function canonicalizeExpression(base: string, canonicalizer?: SqlCanonicalizer): string {
+  return canonicalizer?.expression(base) ?? base
+}
+
 type TableDriftReasonCode =
   | 'missing_column'
   | 'extra_column'
@@ -220,37 +239,104 @@
   }
 }
 
-function normalizeIndexShape(index: SkipIndexDefinition): string {
+function normalizeIndexShape(index: SkipIndexDefinition, canonicalizer?: SqlCanonicalizer): string {
   return [
-    `expr=${normalizeSQLFragment(index.expression)}`,
+    `expr=${canonicalizeExpression(normalizeSQLFragment(index.expression), canonicalizer)}`,
     `type=${renderIndexTypeFingerprint(index)}`,
     `granularity=${index.granularity}`,
   ].join('|')
 }
 
-function normalizeProjectionShape(projection: ProjectionDefinition): string {
+function normalizeProjectionShape(
+  projection: ProjectionDefinition,
+  canonicalizer?: SqlCanonicalizer
+): string {
   if (isIndexProjection(projection)) {
     return [
       `index=${normalizeProjectionIndex(projection.index)}`,
       `type=${projection.type.trim()}`,
     ].join('|')
   }
-  return `query=${normalizeSQLFragment(projection.query)}`
+  const base = normalizeSQLFragment(projection.query)
+  return `query=${canonicalizer?.query(base) ?? base}`
 }
 
-function normalizeClause(value: string | undefined): string {
+/** Strip backticks and one layer of wrapping parens; the shared prep both the
+ *  comparison and the fragment collector build on. */
+function normalizeClauseBase(value: string | undefined): string {
   if (!value) return ''
   const normalized = normalizeSQLFragment(value).replace(/`/g, '')
   const wrapped = normalized.match(/^\((.*)\)$/)
   return wrapped?.[1] ? normalizeSQLFragment(wrapped[1]) : normalized
 }
 
+function normalizeClause(value: string | undefined, canonicalizer?: SqlCanonicalizer): string {
+  const base = normalizeClauseBase(value)
+  if (!canonicalizer || base === '') return base
+  // Canonicalize each element so a function expression in a key/partition clause
+  // (`cityHash64(a,b)`) matches ClickHouse's stored spelling.
+  return splitTopLevelComma(base)
+    .map((element) => canonicalizeExpression(element.trim(), canonicalizer))
+    .join(', ')
+}
+
 function normalizeEngine(value: string | undefined): string {
   if (!value) return ''
   return coreNormalizeEngine(normalizeSQLFragment(value)).toLowerCase()
 }
 
-export function compareTableShape(expected: TableDefinition, actual: ActualTableShape): TableDriftDetail | null {
+/**
+ * Every SQL fragment `compareTableShape` will look up in a `SqlCanonicalizer`,
+ * at the exact granularity it looks them up (whole expressions for index/ttl,
+ * per-element for key/partition clauses, whole query for SELECT projections).
+ * The drift command collects these across all compared tables, formats them in
+ * one round-trip, and hands back a map-backed canonicalizer.
+ */
+export function collectTableSqlFragments(
+  expected: TableDefinition,
+  actual: ActualTableShape
+): { expressions: string[]; queries: string[] } {
+  const expressions: string[] = []
+  const queries: string[] = []
+
+  const addExpression = (raw: string | undefined) => {
+    if (raw) expressions.push(normalizeSQLFragment(raw))
+  }
+  const addClause = (value: string | undefined) => {
+    const base = normalizeClauseBase(value)
+    if (base) expressions.push(...splitTopLevelComma(base).map((element) => element.trim()))
+  }
+
+  for (const index of expected.indexes ?? []) addExpression(index.expression)
+  for (const index of actual.indexes) addExpression(index.expression)
+
+  addExpression(expected.ttl)
+  addExpression(actual.ttl)
+
+  addClause((expected.primaryKey.length > 0 ? expected.primaryKey : expected.orderBy).join(', '))
+  addClause(actual.primaryKey ?? actual.orderBy)
+  addClause(expected.orderBy.join(', '))
+  addClause(actual.orderBy)
+  addClause((expected.uniqueKey ?? []).join(', '))
+  addClause(actual.uniqueKey)
+  addClause(expected.partitionBy)
+  addClause(actual.partitionBy)
+
+  for (const projection of expected.projections ?? []) {
+    if (!isIndexProjection(projection)) queries.push(normalizeSQLFragment(projection.query))
+  }
+  for (const projection of actual.projections) {
+    if (!isIndexProjection(projection)) queries.push(normalizeSQLFragment(projection.query))
+  }
+
+  return { expressions, queries }
+}
+
+export function compareTableShape(
+  expected: TableDefinition,
+  actual: ActualTableShape,
+  canonicalizer?: SqlCanonicalizer
+): TableDriftDetail | null {
   const columnDiff = diffByName(
     expected.columns,
     actual.columns,
@@ -264,13 +350,21 @@
   const settingDiffs = diffSettings(expected.settings ?? {}, actual.settings)
 
   const expectedIndexes = new Map(
-    (expected.indexes ?? []).map((idx) => [idx.name, normalizeIndexShape(idx)])
+    (expected.indexes ?? []).map((idx) => [idx.name, normalizeIndexShape(idx, canonicalizer)])
+  )
+  const actualIndexes = new Map(
+    actual.indexes.map((idx) => [idx.name, normalizeIndexShape(idx, canonicalizer)])
   )
-  const actualIndexes = new Map(actual.indexes.map((idx) => [idx.name, normalizeIndexShape(idx)]))
   const indexDiffs = diffNamedShapeMaps(expectedIndexes, actualIndexes)
 
-  const expectedTTL = expected.ttl ? normalizeSQLFragment(expected.ttl) : ''
-  const actualTTL = actual.ttl ? normalizeSQLFragment(actual.ttl) : ''
+  const expectedTTL = canonicalizeExpression(
+    expected.ttl ? normalizeSQLFragment(expected.ttl) : '',
+    canonicalizer
+  )
+  const actualTTL = canonicalizeExpression(
+    actual.ttl ? normalizeSQLFragment(actual.ttl) : '',
+    canonicalizer
+  )
   const ttlMismatch = expectedTTL !== actualTTL
 
   const engineMismatch = normalizeEngine(expected.engine) !== normalizeEngine(actual.engine)
@@ -279,28 +373,32 @@
   // Mirror that on both sides (as canonical.ts does for the schema), else every
   // such table drifts forever (#194).
   const expectedPrimaryKey = normalizeClause(
-    (expected.primaryKey.length > 0 ? expected.primaryKey : expected.orderBy).join(', ')
+    (expected.primaryKey.length > 0 ? expected.primaryKey : expected.orderBy).join(', '),
+    canonicalizer
   )
-  const actualPrimaryKey = normalizeClause(actual.primaryKey ?? actual.orderBy)
+  const actualPrimaryKey = normalizeClause(actual.primaryKey ?? actual.orderBy, canonicalizer)
   const primaryKeyMismatch = expectedPrimaryKey !== actualPrimaryKey
-  const expectedOrderBy = normalizeClause(expected.orderBy.join(', '))
-  const actualOrderBy = normalizeClause(actual.orderBy)
+  const expectedOrderBy = normalizeClause(expected.orderBy.join(', '), canonicalizer)
+  const actualOrderBy = normalizeClause(actual.orderBy, canonicalizer)
   const orderByMismatch = expectedOrderBy !== actualOrderBy
-  const expectedUniqueKey = normalizeClause((expected.uniqueKey ?? []).join(', '))
-  const actualUniqueKey = normalizeClause(actual.uniqueKey)
+  const expectedUniqueKey = normalizeClause((expected.uniqueKey ?? []).join(', '), canonicalizer)
+  const actualUniqueKey = normalizeClause(actual.uniqueKey, canonicalizer)
   const uniqueKeyMismatch = expectedUniqueKey !== actualUniqueKey
-  const expectedPartitionBy = normalizeClause(expected.partitionBy)
-  const actualPartitionBy = normalizeClause(actual.partitionBy)
+  const expectedPartitionBy = normalizeClause(expected.partitionBy, canonicalizer)
+  const actualPartitionBy = normalizeClause(actual.partitionBy, canonicalizer)
   const partitionByMismatch = expectedPartitionBy !== actualPartitionBy
 
   const expectedProjections = new Map(
     (expected.projections ?? []).map((projection) => [
       projection.name,
-      normalizeProjectionShape(projection),
+      normalizeProjectionShape(projection, canonicalizer),
     ])
   )
   const actualProjections = new Map(
-    actual.projections.map((projection) => [projection.name, normalizeProjectionShape(projection)])
+    actual.projections.map((projection) => [
+      projection.name,
+      normalizeProjectionShape(projection, canonicalizer),
+    ])
   )
   const projectionDiffs = diffNamedShapeMaps(expectedProjections, actualProjections)
 

packages/cli/src/commands/drift/payload.ts

@@ -1,17 +1,59 @@
 import { join } from 'node:path'
 
-import { isUnknownDatabaseError, type ClickHouseExecutor, type SchemaObjectRef } from '@chkit/clickhouse'
+import {
+  canonicalizeSqlFragments,
+  isUnknownDatabaseError,
+  type ClickHouseExecutor,
+  type IntrospectedTable,
+  type SchemaObjectRef,
+} from '@chkit/clickhouse'
 import type { ChxConfig, Snapshot, TableDefinition } from '@chkit/core'
 
 import { debug } from '../../runtime/debug.js'
 import type { TableScope } from '../../runtime/table-scope.js'
 import {
+  collectTableSqlFragments,
   compareSchemaObjects,
   compareTableShape,
   type ObjectDriftDetail,
+  type SqlCanonicalizer,
   type TableDriftDetail,
 } from './compare.js'
 
+/**
+ * Build a canonicalizer for every SQL fragment across the compared tables in a
+ * single ClickHouse round-trip. Returns undefined when there is nothing to
+ * canonicalize or ClickHouse can't format (old server, offline) — the comparer
+ * then falls back to plain string normalization.
+ */
+async function buildSqlCanonicalizer(
+  db: ClickHouseExecutor,
+  pairs: Array<{ expected: TableDefinition; actual: IntrospectedTable }>
+): Promise<SqlCanonicalizer | undefined> {
+  const expressions: string[] = []
+  const queries: string[] = []
+  for (const { expected, actual } of pairs) {
+    const fragments = collectTableSqlFragments(expected, actual)
+    expressions.push(...fragments.expressions)
+    queries.push(...fragments.queries)
+  }
+  if (expressions.length === 0 && queries.length === 0) return undefined
+
+  try {
+    const [expressionMap, queryMap] = await Promise.all([
+      canonicalizeSqlFragments(db, expressions, { wrap: true }),
+      canonicalizeSqlFragments(db, queries, { wrap: false }),
+    ])
+    return {
+      expression: (fragment) => expressionMap.get(fragment) ?? null,
+      query: (fragment) => queryMap.get(fragment) ?? null,
+    }
+  } catch (error) {
+    debug('drift', `sql canonicalization unavailable, using string comparison: ${String(error)}`)
+    return undefined
+  }
+}
+
 export interface DriftPayload {
   scope?: TableScope
   snapshotFile: string
@@ -119,12 +161,13 @@ export async function buildDriftPayload(
     expectedTables.map((table) => [`${table.database}.${table.name}`, table])
   )
   const actualTables = await db.listTableDetails([...expectedDatabases])
-  const tableDrift = actualTables
-    .map((actual) => {
-      const expected = expectedTableMap.get(`${actual.database}.${actual.name}`)
-      if (!expected) return null
-      return compareTableShape(expected, actual)
-    })
+  const pairs = actualTables.flatMap((actual) => {
+    const expected = expectedTableMap.get(`${actual.database}.${actual.name}`)
+    return expected ? [{ expected, actual }] : []
+  })
+  const canonicalizer = await buildSqlCanonicalizer(db, pairs)
+  const tableDrift = pairs
+    .map(({ expected, actual }) => compareTableShape(expected, actual, canonicalizer))
     .filter((item): item is NonNullable<typeof item> => item !== null)
     .sort((a, b) => a.table.localeCompare(b.table))