fix(parser): track backtick identifiers; unify quote-aware SQL scanning

.changeset/consolidate-quote-aware-sql-scanning.md

@@ -0,0 +1,9 @@
+---
+"@chkit/clickhouse": patch
+"@chkit/plugin-pull": patch
+"@chkit/core": patch
+---
+
+Fix `chkit pull` silently dropping a table's projections (and mangling its `ORDER BY`/`PRIMARY KEY`) when a column has a backtick-quoted name containing a parenthesis, e.g. `` `weird)name` `` (#196). The create-table body scanner counted that paren as structure, truncating the parse.
+
+The root cause was four separate copies of "scan SQL while ignoring quoted regions", which had already drifted — one tracked no quotes at all, another missed backticks (#197). They now share a single quote-aware primitive (`nextQuote`) in `@chkit/core`, alongside shared `stripWrappingParens` and `findMatchingParen` helpers, so the rule lives in one place and every scanner (`splitTopLevelComma`, the projection index normalizer, the pull key-clause parser, and the create-table body finder) handles single-quoted strings, double-quoted identifiers, and backtick identifiers identically.

packages/clickhouse/src/create-table-parser.ts

@@ -1,4 +1,9 @@
-import { normalizeProjectionIndex, normalizeSQLFragment, splitTopLevelComma } from '@chkit/core'
+import {
+  findMatchingParen,
+  normalizeProjectionIndex,
+  normalizeSQLFragment,
+  splitTopLevelComma,
+} from '@chkit/core'
 
 type ProjectionDefinitionShape =
   | { name: string; query: string }
@@ -32,40 +37,14 @@ function parseClauseFromCreateTableQuery(
 function findColumnListBounds(
   createTableQuery: string
 ): { open: number; close: number } | undefined {
+  // Require a table-level `) ENGINE =` so we don't treat some other parenthesised
+  // fragment as the column list.
   const engineMatch = /\)\s*ENGINE\s*=/i.exec(createTableQuery)
   if (!engineMatch || engineMatch.index === undefined) return undefined
-  const left = createTableQuery.slice(0, engineMatch.index + 1)
-  const openIndex = left.indexOf('(')
+  const openIndex = createTableQuery.indexOf('(')
   if (openIndex === -1) return undefined
-
-  let depth = 0
-  let inString = false
-  let stringQuote = "'"
-  for (let i = openIndex; i < left.length; i += 1) {
-    const char = left[i]
-    if (!char) continue
-    if (inString) {
-      if (char === stringQuote && left[i - 1] !== '\\') {
-        inString = false
-      }
-      continue
-    }
-    if (char === "'" || char === '"') {
-      inString = true
-      stringQuote = char
-      continue
-    }
-    if (char === '(') {
-      depth += 1
-      continue
-    }
-    if (char === ')') {
-      depth -= 1
-      if (depth === 0) return { open: openIndex, close: i }
-    }
-  }
-
-  return undefined
+  const close = findMatchingParen(createTableQuery, openIndex)
+  return close === undefined ? undefined : { open: openIndex, close }
 }
 
 function extractCreateTableBody(createTableQuery: string | undefined): string | undefined {

packages/clickhouse/src/index.test.ts

@@ -503,6 +503,23 @@ ORDER BY a`
     expect(parseOrderByFromCreateTableQuery(query)).toBe('id')
     expect(parseSettingsFromCreateTableQuery(query)).toEqual({ index_granularity: '8192' })
   })
+
+  // Regression for #196: a backtick-quoted column name containing a paren used
+  // to unbalance the body scan and truncate the parse, dropping the projection.
+  test('handles a backtick column name containing a paren', () => {
+    const query = `CREATE TABLE app.events (\`id\` UInt64, \`weird)name\` String, PROJECTION p INDEX id TYPE basic) ENGINE = MergeTree ORDER BY id`
+
+    expect(parseProjectionsFromCreateTableQuery(query)).toEqual([
+      { name: 'p', index: 'id', type: 'basic' },
+    ])
+    expect(parseOrderByFromCreateTableQuery(query)).toBe('id')
+  })
+
+  test('keeps backtick identifiers intact inside key clauses', () => {
+    const query = `CREATE TABLE app.events (\`id\` UInt64, \`w)x\` String) ENGINE = MergeTree ORDER BY (\`w)x\`, id)`
+
+    expect(parseOrderByFromCreateTableQuery(query)).toBe('(`w)x`, id)')
+  })
 })
 
 describe('formatConnectionError', () => {

packages/core/src/index.ts

@@ -9,6 +9,7 @@ export {
 export { planDiff } from './planner.js'
 export { createSnapshot } from './snapshot.js'
 export { splitTopLevelComma } from './key-clause.js'
+export { findMatchingParen, stripWrappingParens } from './sql-scan.js'
 export { isIndexProjection, normalizeProjectionIndex } from './projection.js'
 export { normalizeEngine, normalizeSQLFragment } from './sql-normalizer.js'
 export { toCreateSQL } from './sql.js'

packages/core/src/key-clause.ts

@@ -1,24 +1,22 @@
+import { nextQuote } from './sql-scan.js'
+
 export function splitTopLevelComma(input: string): string[] {
   const out: string[] = []
   let current = ''
   let depth = 0
   let quote: "'" | '"' | '`' | null = null
 
   for (let i = 0; i < input.length; i += 1) {
     const char = input[i] ?? ''
-    const prev = i > 0 ? input[i - 1] : ''
-
-    if (quote) {
-      current += char
-      if (char === quote && prev !== '\\') quote = null
-      continue
-    }
+    const prev = i > 0 ? (input[i - 1] ?? '') : ''
+    const quoteBefore = quote
+    quote = nextQuote(char, prev, quote)
 
-    if (char === "'" || char === '"' || char === '`') {
-      quote = char
+    // A char inside a quoted literal (body or delimiter) is never structural.
+    if (quoteBefore !== null || quote !== null) {
       current += char
       continue
     }
 
     if (char === '(') {
       depth += 1

packages/core/src/projection.ts

@@ -1,5 +1,6 @@
 import type { IndexProjectionDefinition, ProjectionDefinition } from './model-types.js'
 import { splitTopLevelComma } from './key-clause.js'
+import { nextQuote, stripWrappingParens } from './sql-scan.js'
 import { normalizeSQLFragment } from './sql-normalizer.js'
 
 export function isIndexProjection(
@@ -8,50 +9,19 @@
   return 'index' in projection
 }
 
-function stripWrappingParens(input: string): string {
-  if (!input.startsWith('(') || !input.endsWith(')')) return input
-
-  // Only strip when the leading paren closes at the very end, so `(a), (b)`
-  // keeps both groups. Parens inside quoted identifiers and string literals
-  // are text, not nesting — `` (`weird)name`) `` is still a single wrapped
-  // expression.
-  let depth = 0
-  let quote: "'" | '"' | '`' | null = null
-  for (let i = 0; i < input.length; i += 1) {
-    const char = input[i]
-    if (quote) {
-      if (char === quote && input[i - 1] !== '\\') quote = null
-      continue
-    }
-    if (char === "'" || char === '"' || char === '`') {
-      quote = char
-      continue
-    }
-    if (char === '(') depth += 1
-    else if (char === ')') {
-      depth -= 1
-      if (depth === 0) return i === input.length - 1 ? input.slice(1, -1).trim() : input
-    }
-  }
-  return input
-}
-
 /** ClickHouse prints one space after every argument separator. */
 function spaceAfterCommas(input: string): string {
   let out = ''
   let quote: "'" | '"' | '`' | null = null
   for (let i = 0; i < input.length; i += 1) {
     const char = input[i] ?? ''
-    if (quote) {
-      out += char
-      if (char === quote && input[i - 1] !== '\\') quote = null
-      continue
-    }
-    if (char === "'" || char === '"' || char === '`') {
-      quote = char
+    const prev = i > 0 ? (input[i - 1] ?? '') : ''
+    const quoteBefore = quote
+    quote = nextQuote(char, prev, quote)
+    if (quoteBefore !== null || quote !== null) {
       out += char
       continue
     }
     // Whitespace is already collapsed to single spaces by normalizeSQLFragment,
     // so a comma is followed by at most one space.
     if (char === ',') {

packages/core/src/sql-scan.test.ts

@@ -0,0 +1,49 @@
+import { describe, expect, test } from 'bun:test'
+
+import { splitTopLevelComma } from './key-clause.js'
+import { findMatchingParen, stripWrappingParens } from './sql-scan.js'
+
+describe('splitTopLevelComma', () => {
+  test('splits only at top-level commas', () => {
+    expect(splitTopLevelComma('a, b, c')).toEqual(['a', 'b', 'c'])
+    expect(splitTopLevelComma('cityHash64(a, b), c')).toEqual(['cityHash64(a, b)', 'c'])
+  })
+
+  test('ignores commas and parens inside quotes of every kind', () => {
+    expect(splitTopLevelComma("'x,y', z")).toEqual(["'x,y'", 'z'])
+    expect(splitTopLevelComma('"x,y", z')).toEqual(['"x,y"', 'z'])
+    expect(splitTopLevelComma('`a,b`, c')).toEqual(['`a,b`', 'c'])
+    expect(splitTopLevelComma('`a)b`, c')).toEqual(['`a)b`', 'c'])
+  })
+})
+
+describe('stripWrappingParens', () => {
+  test('peels exactly one wrapping layer, and only a genuine wrapper', () => {
+    expect(stripWrappingParens('(a, b)')).toBe('a, b')
+    expect(stripWrappingParens('((a, b))')).toBe('(a, b)')
+    expect(stripWrappingParens('(a), (b)')).toBe('(a), (b)')
+    expect(stripWrappingParens('a, b')).toBe('a, b')
+  })
+
+  test('is not fooled by a paren inside a backtick identifier', () => {
+    expect(stripWrappingParens('(`w)x`)')).toBe('`w)x`')
+    expect(stripWrappingParens('(`w)x`, a)')).toBe('`w)x`, a')
+  })
+})
+
+describe('findMatchingParen', () => {
+  test('finds the close matching the open at the given index', () => {
+    expect(findMatchingParen('(a, b) rest', 0)).toBe(5)
+    expect(findMatchingParen('f(g(x)) rest', 1)).toBe(6)
+  })
+
+  test('ignores parens inside quotes', () => {
+    // The ')' inside the quoted region must not close the group.
+    expect(findMatchingParen('(`w)x`) tail', 0)).toBe(6)
+    expect(findMatchingParen("('a)b') tail", 0)).toBe(6)
+  })
+
+  test('returns undefined when unbalanced', () => {
+    expect(findMatchingParen('(a, b', 0)).toBeUndefined()
+  })
+})

packages/core/src/sql-scan.ts

@@ -0,0 +1,74 @@
+// Quote-aware SQL scanning primitives. Characters inside a single-quoted
+// string, a double-quoted identifier, or a backtick-quoted identifier are
+// literal text, not structure — parens and commas there must be ignored.
+//
+// Every scanner in the codebase shares `nextQuote` so the rule lives in exactly
+// one place and the copies cannot drift apart again (#197). Missing backtick
+// tracking here is what let a column named `weird)name` truncate a parsed
+// table body (#196).
+
+type QuoteChar = "'" | '"' | '`'
+
+/**
+ * The quote state after reading `char`, given the previous character and the
+ * state before it. `null` means "not inside a quoted literal". A quote closes
+ * on a matching, unescaped quote char; ClickHouse's doubled-quote escaping
+ * (`''`) falls out correctly as a close immediately followed by a re-open.
+ */
+export function nextQuote(char: string, prevChar: string, quote: QuoteChar | null): QuoteChar | null {
+  if (quote) return char === quote && prevChar !== '\\' ? null : quote
+  if (char === "'" || char === '"' || char === '`') return char
+  return null
+}
+
+/** True when `char` is part of a quoted literal (its body or its delimiters). */
+function isQuoted(char: string, prevChar: string, quoteBefore: QuoteChar | null): boolean {
+  return quoteBefore !== null || nextQuote(char, prevChar, quoteBefore) !== null
+}
+
+/**
+ * Peel one layer of wrapping parentheses, but only when the leading `(` closes
+ * at the very end — so `(a, b)` becomes `a, b` while `(a), (b)` is left intact.
+ */
+export function stripWrappingParens(input: string): string {
+  if (!input.startsWith('(') || !input.endsWith(')')) return input
+
+  let depth = 0
+  let quote: QuoteChar | null = null
+  for (let i = 0; i < input.length; i += 1) {
+    const char = input[i] ?? ''
+    const prev = i > 0 ? (input[i - 1] ?? '') : ''
+    const quoted = isQuoted(char, prev, quote)
+    quote = nextQuote(char, prev, quote)
+    if (quoted) continue
+    if (char === '(') depth += 1
+    else if (char === ')') {
+      depth -= 1
+      if (depth === 0) return i === input.length - 1 ? input.slice(1, -1).trim() : input
+    }
+  }
+  return input
+}
+
+/**
+ * Index of the `)` that matches the `(` at `openIndex`, or undefined when the
+ * parens are unbalanced. Quote-aware, so parens inside identifiers/strings
+ * don't count.
+ */
+export function findMatchingParen(input: string, openIndex: number): number | undefined {
+  let depth = 0
+  let quote: QuoteChar | null = null
+  for (let i = openIndex; i < input.length; i += 1) {
+    const char = input[i] ?? ''
+    const prev = i > 0 ? (input[i - 1] ?? '') : ''
+    const quoted = isQuoted(char, prev, quote)
+    quote = nextQuote(char, prev, quote)
+    if (quoted) continue
+    if (char === '(') depth += 1
+    else if (char === ')') {
+      depth -= 1
+      if (depth === 0) return i
+    }
+  }
+  return undefined
+}

packages/plugin-pull/src/index.ts

@@ -19,6 +19,7 @@ import {
   type SafeParseable,
   type SchemaDefinition,
   splitTopLevelComma,
+  stripWrappingParens,
   type TableDefinition,
   withFactoryDefaults,
 } from '@chkit/core'
@@ -390,26 +391,7 @@ function summarizeSkippedObjects(
 
 function splitTopLevelCommaSeparated(input: string | undefined): string[] {
   if (!input) return []
-  return splitTopLevelComma(input).map(normalizeWrappedTuple)
-}
-
-function normalizeWrappedTuple(input: string): string {
-  const trimmed = input.trim()
-  if (!trimmed.startsWith('(') || !trimmed.endsWith(')')) {
-    return trimmed
-  }
-
-  let depth = 0
-  for (let i = 0; i < trimmed.length; i += 1) {
-    const char = trimmed[i]
-    if (char === '(') depth += 1
-    if (char === ')') depth -= 1
-    if (depth === 0 && i < trimmed.length - 1) {
-      return trimmed
-    }
-  }
-
-  return trimmed.slice(1, -1).trim()
+  return splitTopLevelComma(input).map(stripWrappingParens)
 }
 
 async function listNonTableRows(