feat(schema): add reliable full-text index support - #215
Conversation
Vex-Session: session-d0436b17cf386452740a9d64
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28a2456734
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } else { | ||
| const point = String.fromCodePoint(body.codePointAt(i) ?? 0) | ||
| bytes.push(...Buffer.from(point)) |
There was a problem hiding this comment.
Replace Buffer in the runtime-neutral core lexer
@chkit/core is explicitly intended to remain usable in non-Node runtimes such as Cloudflare Workers, but every ordinary character in a single-quoted text-index fragment now passes through the Node-only Buffer global. Without the optional Node compatibility layer, calling toCreateSQL, canonicalizeDefinitions, or planDiff for a tokenizer such as splitByString([' ']) throws ReferenceError: Buffer is not defined; use a runtime-neutral UTF-8 encoder instead.
Useful? React with 👍 / 👎.
| // Compare redundant identifier quotes without removing them from generated SQL. | ||
| export function textSQLFingerprint(sql: string): string { | ||
| return JSON.stringify( | ||
| textSQLTokens(sql).map((token) => token.replace(/^([`"])([A-Za-z_][A-Za-z0-9_]*)\1$/, '$2')), |
There was a problem hiding this comment.
Preserve quotes around keyword identifiers
The fingerprint strips quotes from every syntactically simple identifier without checking whether those quotes are required. For example, lower(select) and lower(select) receive identical fingerprints even though select is a SQL keyword and the unquoted form is not an equivalent expression, so migration planning and drift checks can silently ignore this change; the mirrored Python fingerprint has the same collision.
Useful? React with 👍 / 👎.
Vex-Session: session-d0436b17cf386452740a9d64
Summary
Add ClickHouse full-text (
text) indexes to the TypeScript and Python schema DSLs, migrations, introspection, pull, and drift checks. This is a standalone replacement for #212, built from currentmainafter #211.TextEncoderin core; SQL validation, rendering, snapshots, and migration planning share this pure normalization logic. No NodeBufferdependency is needed.NULLand theNULLliteral now produce distinct migration plans and drift results.MATERIALIZE INDEX, documented and tested.Validation
Shared adversarial fixtures run in both languages. Live tests start with independently written SQL, introspect it, render and import a schema, recreate a second table, and compare actual indexed search results. Coverage includes CLI generate → migrate → drift → change → migrate, adding an index to existing rows, materialization, newer phrase-search/postprocessing options, and every printable string escape.
Review regressions cover operation without the
Bufferglobal, ordinary and escaped Unicode, 14 identifier names with multiple case/quote variants across expressions and preprocessors/postprocessors, live comparisons of quoted names versus SQL literals, and a quotedNULLcolumn through pull → changed expression → migration → materialization → search.Added CI jobs for ClickHouse 26.3 and 26.8, running both TypeScript and Python feature suites. The newer options are exercised on 26.8; basic functionality and tuning run on both versions.
Validation of
0bd1d0b(local and CI):Server limitation
ClickHouse 26.3 can drop required identifier quotes from index metadata for column names such as
trueorinf, making a lossless pull impossible for those expressions. This is documented; chkit conservatively reports the quote difference as drift rather than equating a column reference with a literal. Prefer ordinary column names on affected servers.