From 426013bf7b11e33ed0f21b4a72af054e105d4848 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Tue, 14 Jul 2026 12:19:57 +0200 Subject: [PATCH 01/15] feat(core): add dictionary schema primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add DictionaryDefinition/DictionaryAttribute types, the dictionary() builder, validation rules, canonicalization, CREATE DICTIONARY SQL rendering, and a binary planner diff (ClickHouse dictionaries have no ALTER — any structural change becomes a single CREATE OR REPLACE DICTIONARY, with the source password masked so a redacted [HIDDEN] value never causes phantom drift). Co-Authored-By: Claude Sonnet 5 --- packages/core/src/canonical.ts | 34 ++++++++- packages/core/src/index.test.ts | 122 +++++++++++++++++++++++++++++++ packages/core/src/index.ts | 2 +- packages/core/src/model-types.ts | 42 ++++++++++- packages/core/src/model.ts | 11 ++- packages/core/src/planner.ts | 57 +++++++++++++++ packages/core/src/sql.ts | 27 +++++++ packages/core/src/validate.ts | 77 +++++++++++++++++++ 8 files changed, 368 insertions(+), 4 deletions(-) diff --git a/packages/core/src/canonical.ts b/packages/core/src/canonical.ts index fa809d6d..c838770b 100644 --- a/packages/core/src/canonical.ts +++ b/packages/core/src/canonical.ts @@ -1,5 +1,7 @@ import type { ColumnDefinition, + DictionaryAttribute, + DictionaryDefinition, MaterializedViewDefinition, MaterializedViewRefresh, ProjectionDefinition, @@ -20,7 +22,8 @@ function sortByName(items: T[]): T[] { function sortKind(kind: SchemaDefinition['kind']): number { if (kind === 'table') return 0 if (kind === 'view') return 1 - return 2 + if (kind === 'materialized_view') return 2 + return 3 } function canonicalizeColumn(column: ColumnDefinition): ColumnDefinition { @@ -168,9 +171,38 @@ function canonicalizeMaterializedView(def: MaterializedViewDefinition): Material return canonical } +function canonicalizeDictionaryAttribute(attribute: DictionaryAttribute): DictionaryAttribute { + return { + ...attribute, + name: attribute.name.trim(), + type: typeof attribute.type === 'string' ? attribute.type.trim() : attribute.type, + } +} + +function canonicalizeDictionary(def: DictionaryDefinition): DictionaryDefinition { + return { + ...def, + database: def.database.trim(), + name: def.name.trim(), + renamedFrom: def.renamedFrom + ? { + database: def.renamedFrom.database?.trim(), + name: def.renamedFrom.name.trim(), + } + : undefined, + attributes: def.attributes.map(canonicalizeDictionaryAttribute), + primaryKey: normalizeKeyColumns(def.primaryKey), + source: normalizeSQLFragment(def.source), + layout: normalizeSQLFragment(def.layout), + lifetime: normalizeSQLFragment(def.lifetime), + comment: def.comment?.trim(), + } +} + export function canonicalizeDefinition(def: SchemaDefinition): SchemaDefinition { if (def.kind === 'table') return canonicalizeTable(def) if (def.kind === 'view') return canonicalizeView(def) + if (def.kind === 'dictionary') return canonicalizeDictionary(def) return canonicalizeMaterializedView(def) } diff --git a/packages/core/src/index.test.ts b/packages/core/src/index.test.ts index 9a782ebe..7f1fbe78 100644 --- a/packages/core/src/index.test.ts +++ b/packages/core/src/index.test.ts @@ -5,6 +5,8 @@ import { canonicalizeDefinitions, codec, collectDefinitionsFromModule, + dictionary, + isSchemaDefinition, materializedView, planDiff, schema, @@ -1675,3 +1677,123 @@ describe('@chkit/core refreshable materialized views', () => { ).toBe(false) }) }) + +describe('@chkit/core dictionaries', () => { + const baseDictionary = { + database: 'app', + name: 'users_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'name', type: 'String' }, + { name: 'email', type: 'String', default: '' }, + ], + primaryKey: ['id'], + source: `MYSQL(host 'db' port 3306 user 'reader' password 'secret' db 'app' table 'users')`, + layout: `HASHED()`, + lifetime: `300`, + } as const + + test('dictionary() builds a valid definition and isSchemaDefinition accepts it', () => { + const dict = dictionary({ ...baseDictionary }) + expect(dict.kind).toBe('dictionary') + expect(isSchemaDefinition(dict)).toBe(true) + }) + + test('renders CREATE DICTIONARY SQL', () => { + const dict = dictionary({ ...baseDictionary, comment: 'User lookup dictionary' }) + const sql = toCreateSQL(dict) + expect(sql).toContain('CREATE DICTIONARY IF NOT EXISTS app.users_dict') + expect(sql).toContain('PRIMARY KEY `id`') + expect(sql).toContain("SOURCE(MYSQL(host 'db' port 3306") + expect(sql).toContain('LAYOUT(HASHED())') + expect(sql).toContain('LIFETIME(300)') + expect(sql).toContain("COMMENT 'User lookup dictionary'") + }) + + test('validation: missing primaryKey', () => { + const issues = validateDefinitions([dictionary({ ...baseDictionary, primaryKey: [] })]) + expect(issues.map((i) => i.code)).toContain('dictionary_missing_primary_key') + }) + + test('validation: primaryKey references missing attribute', () => { + const issues = validateDefinitions([ + dictionary({ ...baseDictionary, primaryKey: ['not_an_attribute'] }), + ]) + expect(issues.map((i) => i.code)).toContain('dictionary_primary_key_missing_attribute') + }) + + test('validation: missing source/layout/lifetime', () => { + const issues = validateDefinitions([ + dictionary({ ...baseDictionary, source: '', layout: '', lifetime: '' }), + ]) + const codes = issues.map((i) => i.code) + expect(codes).toContain('dictionary_missing_source') + expect(codes).toContain('dictionary_missing_layout') + expect(codes).toContain('dictionary_missing_lifetime') + }) + + test('validation: default and expression are mutually exclusive', () => { + const issues = validateDefinitions([ + dictionary({ + ...baseDictionary, + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'name', type: 'String', default: 'x', expression: 'upper(name)' }, + ], + }), + ]) + expect(issues.map((i) => i.code)).toContain('dictionary_attribute_default_expression_exclusive') + }) + + test('binary diff: unchanged definitions produce no operations', () => { + const oldDefs = [dictionary({ ...baseDictionary })] + const newDefs = [dictionary({ ...baseDictionary })] + const plan = planDiff(oldDefs, newDefs) + expect(plan.operations).toHaveLength(0) + }) + + test('binary diff: structural change produces a single CREATE OR REPLACE DICTIONARY op', () => { + const oldDefs = [dictionary({ ...baseDictionary })] + const newDefs = [dictionary({ ...baseDictionary, layout: 'COMPLEX_KEY_HASHED()' })] + const plan = planDiff(oldDefs, newDefs) + expect(plan.operations).toHaveLength(1) + expect(plan.operations[0]?.type).toBe('create_dictionary') + expect(plan.operations[0]?.sql).toContain('CREATE OR REPLACE DICTIONARY') + expect(plan.operations[0]?.risk).toBe('caution') + }) + + test('binary diff: removing a dictionary produces a drop_dictionary op', () => { + const oldDefs = [dictionary({ ...baseDictionary })] + const plan = planDiff(oldDefs, []) + expect(plan.operations).toHaveLength(1) + expect(plan.operations[0]?.type).toBe('drop_dictionary') + expect(plan.operations[0]?.sql).toBe('DROP DICTIONARY IF EXISTS app.users_dict;') + expect(plan.operations[0]?.risk).toBe('danger') + }) + + test('binary diff: source differing only by password produces no diff', () => { + const oldDefs = [dictionary({ ...baseDictionary })] + const newDefs = [ + dictionary({ + ...baseDictionary, + source: `MYSQL(host 'db' port 3306 user 'reader' password 'a-different-secret' db 'app' table 'users')`, + }), + ] + const plan = planDiff(oldDefs, newDefs) + expect(plan.operations).toHaveLength(0) + }) + + test('create ordering: create_dictionary ranks after create_table', () => { + const usersTable = table({ + database: 'app', + name: 'users', + columns: [{ name: 'id', type: 'UInt64' }], + engine: 'MergeTree()', + primaryKey: ['id'], + orderBy: ['id'], + }) + const plan = planDiff([], [usersTable, dictionary({ ...baseDictionary })]) + const types = plan.operations.map((op) => op.type) + expect(types.indexOf('create_table')).toBeLessThan(types.indexOf('create_dictionary')) + }) +}) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index ce75f806..255d88f9 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -10,7 +10,7 @@ export { planDiff } from './planner.js' export { createSnapshot } from './snapshot.js' export { splitTopLevelComma } from './key-clause.js' export { normalizeEngine, normalizeSQLFragment } from './sql-normalizer.js' -export { toCreateSQL } from './sql.js' +export { renderDictionarySQL, toCreateSQL } from './sql.js' export { applyOnClusterToPlan, onClusterClause } from './on-cluster.js' export { canonicalizeCodec, diff --git a/packages/core/src/model-types.ts b/packages/core/src/model-types.ts index 2b3bbf09..49043065 100644 --- a/packages/core/src/model-types.ts +++ b/packages/core/src/model-types.ts @@ -156,7 +156,39 @@ export interface MaterializedViewDefinition { comment?: string } -export type SchemaDefinition = TableDefinition | ViewDefinition | MaterializedViewDefinition +export interface DictionaryAttribute { + name: string + type: PrimitiveColumnType | string + /** DEFAULT / null_value for missing keys. */ + default?: string | number | boolean + /** EXPRESSION — computed from source columns. Mutually exclusive with default. */ + expression?: string + hierarchical?: boolean + injective?: boolean + isObjectId?: boolean +} + +export interface DictionaryDefinition { + kind: 'dictionary' + database: string + name: string + renamedFrom?: { database?: string; name: string } + attributes: DictionaryAttribute[] + primaryKey: string[] + /** Raw SOURCE(...) body, e.g. `MYSQL(host '...' password '${env}' ...)`. */ + source: string + /** Raw LAYOUT(...) body, e.g. `HASHED()` / `COMPLEX_KEY_HASHED()`. */ + layout: string + /** Raw LIFETIME(...) body, e.g. `300` / `MIN 300 MAX 360`. */ + lifetime: string + comment?: string +} + +export type SchemaDefinition = + | TableDefinition + | ViewDefinition + | MaterializedViewDefinition + | DictionaryDefinition export interface ChxCheckConfig { failOnPending?: boolean @@ -278,6 +310,8 @@ export type MigrationOperationType = | 'alter_table_drop_projection' | 'alter_table_reset_setting' | 'alter_table_modify_ttl' + | 'create_dictionary' + | 'drop_dictionary' export interface MigrationOperation { type: MigrationOperationType @@ -320,6 +354,12 @@ export type ValidationIssueCode = | 'codec_chain_must_end_with_general' | 'codec_chain_multiple_general' | 'codec_chain_empty' + | 'dictionary_missing_primary_key' + | 'dictionary_primary_key_missing_attribute' + | 'dictionary_missing_source' + | 'dictionary_missing_layout' + | 'dictionary_missing_lifetime' + | 'dictionary_attribute_default_expression_exclusive' export interface ValidationIssue { code: ValidationIssueCode diff --git a/packages/core/src/model.ts b/packages/core/src/model.ts index f81d2c72..7d46afb0 100644 --- a/packages/core/src/model.ts +++ b/packages/core/src/model.ts @@ -5,6 +5,7 @@ import type { ChxConfigInput, ChxResolvedConfig, ChxUserConfig, + DictionaryDefinition, MaterializedViewDefinition, SchemaDefinition, TableDefinition, @@ -96,6 +97,12 @@ export function materializedView( return { ...input, kind: 'materialized_view' } } +export function dictionary( + input: Omit +): DictionaryDefinition { + return { ...input, kind: 'dictionary' } +} + export function schema(...definitions: SchemaDefinition[]): SchemaDefinition[] { return definitions } @@ -103,5 +110,7 @@ export function schema(...definitions: SchemaDefinition[]): SchemaDefinition[] { export function isSchemaDefinition(value: unknown): value is SchemaDefinition { if (!value || typeof value !== 'object') return false const kind = (value as { kind?: string }).kind - return kind === 'table' || kind === 'view' || kind === 'materialized_view' + return ( + kind === 'table' || kind === 'view' || kind === 'materialized_view' || kind === 'dictionary' + ) } diff --git a/packages/core/src/planner.ts b/packages/core/src/planner.ts index a5b766e3..9ea8b36a 100644 --- a/packages/core/src/planner.ts +++ b/packages/core/src/planner.ts @@ -3,6 +3,7 @@ import { diffByName, diffClauses, diffSettings } from './diff-primitives.js' import type { ColumnDefinition, ColumnRenameSuggestion, + DictionaryDefinition, MaterializedViewDefinition, MaterializedViewRefresh, MigrationOperation, @@ -24,6 +25,7 @@ import { renderAlterModifyTTL, renderAlterRemoveCodec, renderAlterResetSetting, + renderDictionarySQL, toCreateSQL, } from './sql.js' import { assertValidDefinitions } from './validate.js' @@ -55,6 +57,15 @@ function pushDropOperation( }) return } + if (def.kind === 'dictionary') { + operations.push( { + type: 'drop_dictionary', + key: definitionKey(def), + risk, + sql: `DROP DICTIONARY IF EXISTS ${def.database}.${def.name};`, + }) + return + } operations.push( { type: 'drop_materialized_view', key: definitionKey(def), @@ -86,6 +97,15 @@ function pushCreateOperation( }) return } + if (def.kind === 'dictionary') { + operations.push( { + type: 'create_dictionary', + key: definitionKey(def), + risk, + sql: toCreateSQL(def), + }) + return + } operations.push( { type: 'create_materialized_view', key: definitionKey(def), @@ -296,6 +316,38 @@ function diffMaterializedView( return [] } +// Redacts inline SOURCE() credentials for comparison only, mirroring what +// ClickHouse itself does on introspection (`[HIDDEN]`). A definition whose +// only difference is the password must not diff every run — never used to +// render DDL, so the real secret still reaches ClickHouse. +function maskDictionarySecrets(source: string): string { + return source.replace(/password\s+'(?:[^'\\]|\\.)*'/gi, "password '[HIDDEN]'") +} + +function dictionaryComparisonShape(def: DictionaryDefinition) { + const { source, ...rest } = def + return { ...rest, source: maskDictionarySecrets(source) } +} + +function diffDictionary( + oldDef: DictionaryDefinition, + newDef: DictionaryDefinition +): MigrationOperation[] { + const unchanged = + JSON.stringify(dictionaryComparisonShape(oldDef)) === + JSON.stringify(dictionaryComparisonShape(newDef)) + if (unchanged) return [] + + return [ + { + type: 'create_dictionary', + key: definitionKey(newDef), + risk: 'caution', + sql: renderDictionarySQL(newDef, true), + }, + ] +} + function diffTables(oldDef: TableDefinition, newDef: TableDefinition): TableDiffResult { if (requiresTableRecreate(oldDef, newDef)) { return { @@ -550,6 +602,11 @@ export function planDiff(oldDefinitions: SchemaDefinition[], newDefinitions: Sch continue } + if (newDef.kind === oldDef.kind && newDef.kind === 'dictionary' && oldDef.kind === 'dictionary') { + operations.push(...diffDictionary(oldDef, newDef)) + continue + } + if (newDef.kind !== oldDef.kind) { pushDropOperation(operations, oldDef, 'danger') } diff --git a/packages/core/src/sql.ts b/packages/core/src/sql.ts index 48104996..867a131e 100644 --- a/packages/core/src/sql.ts +++ b/packages/core/src/sql.ts @@ -1,5 +1,7 @@ import type { ColumnDefinition, + DictionaryAttribute, + DictionaryDefinition, MaterializedViewDefinition, MaterializedViewRefresh, SchemaDefinition, @@ -145,10 +147,35 @@ export function renderAlterModifyRefresh(def: MaterializedViewDefinition): strin return `ALTER TABLE ${def.database}.${def.name} MODIFY ${renderRefreshClause(def.refresh)};` } +function renderDictionaryAttribute(attr: DictionaryAttribute): string { + let out = `\`${attr.name}\` ${attr.type}` + if (attr.expression !== undefined) out += ` EXPRESSION ${attr.expression}` + else if (attr.default !== undefined) out += ` DEFAULT ${renderDefault(attr.default)}` + if (attr.hierarchical) out += ' HIERARCHICAL' + if (attr.injective) out += ' INJECTIVE' + if (attr.isObjectId) out += ' IS_OBJECT_ID' + return out +} + +export function renderDictionarySQL(def: DictionaryDefinition, replace = false): string { + const verb = replace ? 'CREATE OR REPLACE DICTIONARY' : 'CREATE DICTIONARY IF NOT EXISTS' + const attrs = def.attributes.map(renderDictionaryAttribute).join(',\n ') + const pk = renderKeyClauseColumns(def.primaryKey, new Set(def.attributes.map((a) => a.name))) + const clauses = [ + `PRIMARY KEY ${pk}`, + `SOURCE(${def.source})`, + `LAYOUT(${def.layout})`, + `LIFETIME(${def.lifetime})`, + ] + if (def.comment) clauses.push(`COMMENT '${def.comment.replace(/'/g, "''")}'`) + return `${verb} ${def.database}.${def.name}\n(\n ${attrs}\n)\n${clauses.join('\n')};` +} + export function toCreateSQL(def: SchemaDefinition): string { assertValidDefinitions([def]) if (def.kind === 'table') return renderTableSQL(def) if (def.kind === 'view') return renderViewSQL(def) + if (def.kind === 'dictionary') return renderDictionarySQL(def) return renderMaterializedViewSQL(def) } diff --git a/packages/core/src/validate.ts b/packages/core/src/validate.ts index 89d8a21c..67afcabb 100644 --- a/packages/core/src/validate.ts +++ b/packages/core/src/validate.ts @@ -3,6 +3,7 @@ import { canonicalizeCodec, isGeneralCodec, isRawCodec } from './codec.js' import { isPlainColumnReference, normalizeKeyColumns } from './key-clause.js' import type { ColumnDefinition, + DictionaryDefinition, MaterializedViewDefinition, MaterializedViewRefresh, SchemaDefinition, @@ -222,6 +223,80 @@ function validateMaterializedViewDefinition( } } +function validateDictionaryDefinition(def: DictionaryDefinition, issues: ValidationIssue[]): void { + const attributeSeen = new Set() + const attributeSet = new Set() + for (const attribute of def.attributes) { + if (attributeSeen.has(attribute.name)) { + pushValidationIssue( + issues, + def, + 'duplicate_column_name', + `Dictionary ${def.database}.${def.name} has duplicate attribute name "${attribute.name}"` + ) + continue + } + attributeSeen.add(attribute.name) + attributeSet.add(attribute.name) + + if (attribute.default !== undefined && attribute.expression !== undefined) { + pushValidationIssue( + issues, + def, + 'dictionary_attribute_default_expression_exclusive', + `Dictionary ${def.database}.${def.name} attribute "${attribute.name}" sets both "default" and "expression"; choose one` + ) + } + } + + if (def.primaryKey.length === 0) { + pushValidationIssue( + issues, + def, + 'dictionary_missing_primary_key', + `Dictionary ${def.database}.${def.name} requires a non-empty primaryKey` + ) + } else { + for (const column of def.primaryKey) { + if (!attributeSet.has(column)) { + pushValidationIssue( + issues, + def, + 'dictionary_primary_key_missing_attribute', + `Dictionary ${def.database}.${def.name} primaryKey references missing attribute "${column}"` + ) + } + } + } + + if (def.source.trim().length === 0) { + pushValidationIssue( + issues, + def, + 'dictionary_missing_source', + `Dictionary ${def.database}.${def.name} requires a non-empty "source"` + ) + } + + if (def.layout.trim().length === 0) { + pushValidationIssue( + issues, + def, + 'dictionary_missing_layout', + `Dictionary ${def.database}.${def.name} requires a non-empty "layout"` + ) + } + + if (def.lifetime.trim().length === 0) { + pushValidationIssue( + issues, + def, + 'dictionary_missing_lifetime', + `Dictionary ${def.database}.${def.name} requires a non-empty "lifetime"` + ) + } +} + export function validateDefinitions(definitions: SchemaDefinition[]): ValidationIssue[] { const issues: ValidationIssue[] = [] const objectKeys = new Set() @@ -242,6 +317,8 @@ export function validateDefinitions(definitions: SchemaDefinition[]): Validation validateTableDefinition(def, issues) } else if (def.kind === 'materialized_view') { validateMaterializedViewDefinition(def, issues, definitions) + } else if (def.kind === 'dictionary') { + validateDictionaryDefinition(def, issues) } } From 0a3cb4ab93b203694249fbb7e72c273ff1703466 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Tue, 14 Jul 2026 12:20:30 +0200 Subject: [PATCH 02/15] feat(cli): wire dictionary into drift, destructive-op safety, and DDL waits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Classify drop_dictionary as destructive (SQL-scan rule + planner-marker path, with a dependency-break warning), add dictionary to the drift existence-comparison kind union (following the materialized_view precedent — existence-drift, not deep-shape), and wait on system.tables for create_dictionary/drop_dictionary DDL propagation. Co-Authored-By: Claude Sonnet 5 --- packages/cli/src/commands/drift/compare.ts | 2 +- packages/cli/src/runtime/safety-markers.ts | 11 ++++++- packages/cli/src/test/drift.test.ts | 29 +++++++++++++++++++ .../src/test/runtime/safety-markers.test.ts | 26 +++++++++++++++++ packages/clickhouse/src/ddl-propagation.ts | 25 ++++++++++++++-- 5 files changed, 88 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/commands/drift/compare.ts b/packages/cli/src/commands/drift/compare.ts index 64c70dd6..9cb04f63 100644 --- a/packages/cli/src/commands/drift/compare.ts +++ b/packages/cli/src/commands/drift/compare.ts @@ -26,7 +26,7 @@ type ObjectDriftReasonCode = 'missing_object' | 'extra_object' | 'kind_mismatch' type DriftReasonCode = ObjectDriftReasonCode | TableDriftReasonCode interface SchemaObjectShape { - kind: 'table' | 'view' | 'materialized_view' + kind: 'table' | 'view' | 'materialized_view' | 'dictionary' database: string name: string } diff --git a/packages/cli/src/runtime/safety-markers.ts b/packages/cli/src/runtime/safety-markers.ts index 3e23d522..65244f43 100644 --- a/packages/cli/src/runtime/safety-markers.ts +++ b/packages/cli/src/runtime/safety-markers.ts @@ -142,6 +142,14 @@ function describeDestructiveOperation( recommendation: 'Confirm replacement view rollout and dependency readiness.', } } + if (type === 'drop_dictionary') { + return { + warningCode: 'drop_dictionary_dependency_break', + reason: 'Dropping a dictionary removes a lookup source used by dictGet() calls and joins.', + impact: 'Queries and materialized views that call dictGet() against this dictionary will fail.', + recommendation: 'Confirm no queries or views still reference this dictionary before approving.', + } + } return { warningCode: 'destructive_operation_review_required', @@ -198,6 +206,7 @@ const DESTRUCTIVE_SQL_RULES: Array<{ type: string; re: RegExp }> = [ { type: 'drop_materialized_view', re: /\bDROP\s+MATERIALIZED\s+VIEW\b/i }, { type: 'drop_view', re: /\bDROP\s+VIEW\b/i }, { type: 'drop_table', re: /\bDROP\s+(?:TEMPORARY\s+)?TABLE\b/i }, + { type: 'drop_dictionary', re: /\bDROP\s+DICTIONARY\b/i }, { type: 'alter_table_drop_column', re: /\bDROP\s+COLUMN\b/i }, { type: 'truncate_table', re: /\bTRUNCATE\s+(?:TABLE|DATABASE|ALL\s+TABLES)\b/i }, { type: 'detach', re: /\bDETACH\s+(?:TABLE|VIEW|DICTIONARY|DATABASE|PARTITION|PART)\b/i }, @@ -212,7 +221,7 @@ function classifyDestructiveStatement(statement: string): string | null { function extractObjectKey(statement: string): string { const match = statement.match( - /\b(?:TABLE|VIEW|DATABASE)\s+(?:IF\s+EXISTS\s+)?`?([\w.]+)`?/i, + /\b(?:TABLE|VIEW|DATABASE|DICTIONARY)\s+(?:IF\s+EXISTS\s+)?`?([\w.]+)`?/i, ) return match?.[1] ?? 'unknown' } diff --git a/packages/cli/src/test/drift.test.ts b/packages/cli/src/test/drift.test.ts index 8cce1df2..85e12f91 100644 --- a/packages/cli/src/test/drift.test.ts +++ b/packages/cli/src/test/drift.test.ts @@ -23,6 +23,35 @@ describe('@chkit/cli drift comparer', () => { ) }) + test('treats a dictionary like other non-table kinds for existence drift', () => { + const result = compareSchemaObjects( + [{ kind: 'dictionary', database: 'app', name: 'users_dict' }], + [] + ) + + expect(result.missing).toEqual(['dictionary:app.users_dict']) + expect(result.objectDrift).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: 'missing_object', + object: 'dictionary:app.users_dict', + expectedKind: 'dictionary', + }), + ]) + ) + }) + + test('no drift when a dictionary exists on both sides', () => { + const result = compareSchemaObjects( + [{ kind: 'dictionary', database: 'app', name: 'users_dict' }], + [{ kind: 'dictionary', database: 'app', name: 'users_dict' }] + ) + + expect(result.missing).toHaveLength(0) + expect(result.extra).toHaveLength(0) + expect(result.objectDrift).toHaveLength(0) + }) + test('emits object-level drift reason codes', () => { const result = compareSchemaObjects( [ diff --git a/packages/cli/src/test/runtime/safety-markers.test.ts b/packages/cli/src/test/runtime/safety-markers.test.ts index 9491717c..aef8fb4c 100644 --- a/packages/cli/src/test/runtime/safety-markers.test.ts +++ b/packages/cli/src/test/runtime/safety-markers.test.ts @@ -164,6 +164,7 @@ describe('scanDestructiveSqlStatements (defense-in-depth for unmarked SQL)', () ['truncate', 'TRUNCATE TABLE default.events;', 'truncate_table'], ['drop view', 'DROP VIEW default.events_v;', 'drop_view'], ['drop materialized view', 'DROP MATERIALIZED VIEW default.events_mv;', 'drop_materialized_view'], + ['drop dictionary', 'DROP DICTIONARY default.users_dict;', 'drop_dictionary'], ['detach', 'DETACH TABLE default.events;', 'detach'], ['drop database', 'DROP DATABASE analytics;', 'drop_database'], ] @@ -203,6 +204,18 @@ describe('scanDestructiveSqlStatements (defense-in-depth for unmarked SQL)', () expect(marker?.warningCode).toBe('drop_column_irreversible') expect(marker?.summary).toContain('DROP COLUMN') }) + + test('synthesizes a danger marker (key + preview) for a hand-written DROP DICTIONARY', () => { + const sql = 'DROP DICTIONARY default.users_dict;' + const markers = collectUnmarkedDestructiveStatements('20260101_handwritten.sql', sql) + expect(markers).toHaveLength(1) + const marker = markers[0] + expect(marker?.type).toBe('drop_dictionary') + expect(marker?.risk).toBe('danger') + expect(marker?.key).toBe('default.users_dict') + expect(marker?.warningCode).toBe('drop_dictionary_dependency_break') + expect(marker?.summary).toContain('DROP DICTIONARY') + }) }) describe('collectDestructiveOperationMarkers table-recreate warning (#23)', () => { @@ -235,6 +248,19 @@ describe('collectDestructiveOperationMarkers table-recreate warning (#23)', () = expect(markers[0]?.warningCode).toBe('drop_table_data_loss') }) + test('a planner-emitted drop_dictionary gets the dependency-break warning', () => { + const sql = [ + '-- operation: drop_dictionary key=dictionary:default.users_dict risk=danger', + 'DROP DICTIONARY IF EXISTS default.users_dict;', + ].join('\n') + + const markers = collectDestructiveOperationMarkers('20260101_drop_dict.sql', sql) + expect(markers).toHaveLength(1) + const marker = markers[0] + expect(marker?.type).toBe('drop_dictionary') + expect(marker?.warningCode).toBe('drop_dictionary_dependency_break') + }) + test('dropping one table while creating a different one is not a recreate', () => { const sql = [ '-- operation: drop_table key=table:default.old_events risk=danger', diff --git a/packages/clickhouse/src/ddl-propagation.ts b/packages/clickhouse/src/ddl-propagation.ts index 4ed7b4ea..180ad2da 100644 --- a/packages/clickhouse/src/ddl-propagation.ts +++ b/packages/clickhouse/src/ddl-propagation.ts @@ -33,6 +33,21 @@ export async function waitForView( }, RETRY_OPTIONS) } +export async function waitForDictionary( + executor: ClickHouseExecutor, + database: string, + dictionaryName: string, +): Promise { + await pRetry(async () => { + const rows = await executor.query<{ x: number }>( + `SELECT 1 AS x FROM system.dictionaries WHERE database = '${database}' AND name = '${dictionaryName}'`, + ) + if (rows.length === 0) { + throw new Error(`waitForDictionary: ${database}.${dictionaryName} not yet visible`) + } + }, RETRY_OPTIONS) +} + export async function waitForColumn( executor: ClickHouseExecutor, database: string, @@ -89,8 +104,8 @@ export async function waitForTableAbsent( } /** - * Parses an operation key like "table:app.users" or "table:app.users:column:name" - * into its components. + * Parses an operation key like "table:app.users", "table:app.users:column:name", + * or "dictionary:app.users_dict" into its components. */ function parseOperationKey(key: string): | { @@ -99,7 +114,7 @@ function parseOperationKey(key: string): column: string | undefined } | undefined { - const tableMatch = key.match(/^table:([^.]+)\.([^:]+)/) + const tableMatch = key.match(/^(?:table|dictionary):([^.]+)\.([^:]+)/) if (!tableMatch) return undefined // biome-ignore lint/style/noNonNullAssertion: capture groups guaranteed by regex match const database = tableMatch[1]! @@ -130,6 +145,9 @@ export async function waitForDDLPropagation( case 'create_materialized_view': return waitForView(executor, parsed.database, parsed.table) + case 'create_dictionary': + return waitForTable(executor, parsed.database, parsed.table) + case 'alter_table_add_column': case 'alter_table_modify_column': if (parsed.column) { @@ -145,6 +163,7 @@ export async function waitForDDLPropagation( case 'drop_table': case 'drop_view': case 'drop_materialized_view': + case 'drop_dictionary': return waitForTableAbsent( executor, parsed.database, From 0a58bd26a4619dd39b22d6ba85a422262f7e703b Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Tue, 14 Jul 2026 12:20:55 +0200 Subject: [PATCH 03/15] feat(clickhouse,plugin-pull): introspect dictionaries and pull them into schema files Add create-dictionary-parser.ts to parse attributes/primaryKey/ source/layout/lifetime/comment out of a dictionary's create_table_query (verified live: DDL dictionaries populate system.tables.create_table_query, with inline passwords already redacted to [HIDDEN]). Map engine 'Dictionary' to kind 'dictionary' instead of skipping it, and render pulled dictionaries as dictionary({...}) calls with a leading comment when the source password was redacted. Co-Authored-By: Claude Sonnet 5 --- .../src/create-dictionary-parser.ts | 236 ++++++++++++++++++ packages/clickhouse/src/index.test.ts | 70 +++++- packages/clickhouse/src/index.ts | 15 +- packages/plugin-pull/src/index.test.ts | 76 ++++++ packages/plugin-pull/src/index.ts | 27 +- packages/plugin-pull/src/render-schema.ts | 44 ++++ packages/plugin-pull/src/view-parser.ts | 45 +++- 7 files changed, 506 insertions(+), 7 deletions(-) create mode 100644 packages/clickhouse/src/create-dictionary-parser.ts diff --git a/packages/clickhouse/src/create-dictionary-parser.ts b/packages/clickhouse/src/create-dictionary-parser.ts new file mode 100644 index 00000000..55f7755d --- /dev/null +++ b/packages/clickhouse/src/create-dictionary-parser.ts @@ -0,0 +1,236 @@ +import { splitTopLevelComma } from '@chkit/core' + +export interface ParsedDictionaryAttribute { + name: string + type: string + default?: string | number + expression?: string + hierarchical?: boolean + injective?: boolean + isObjectId?: boolean +} + +const MODIFIER_KEYWORDS = ['DEFAULT', 'EXPRESSION', 'HIERARCHICAL', 'INJECTIVE', 'IS_OBJECT_ID'] + +function extractBalancedParenGroup( + text: string, + fromIndex: number +): { content: string; endIndex: number } | undefined { + const openIndex = text.indexOf('(', fromIndex) + if (openIndex === -1) return undefined + let depth = 0 + let inString = false + let stringQuote = "'" + for (let i = openIndex; i < text.length; i += 1) { + const char = text[i] + if (!char) continue + if (inString) { + if (char === stringQuote && text[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 { content: text.slice(openIndex + 1, i), endIndex: i + 1 } + } + } + return undefined +} + +function extractCreateDictionaryBody(query: string | undefined): string | undefined { + if (!query) return undefined + const nameMatch = query.match( + /\bCREATE\s+(?:OR\s+REPLACE\s+)?DICTIONARY\s+(?:IF\s+NOT\s+EXISTS\s+)?[`\w.]+\s*/i + ) + if (!nameMatch || nameMatch.index === undefined) return undefined + const start = nameMatch.index + nameMatch[0].length + const group = extractBalancedParenGroup(query, start) + if (!group) return undefined + const trimmed = group.content.trim() + return trimmed.length > 0 ? trimmed : undefined +} + +function extractKeywordParenBody(query: string, keyword: string): string | undefined { + const match = new RegExp(`\\b${keyword}\\s*\\(`, 'i').exec(query) + if (!match || match.index === undefined) return undefined + const openIndex = match.index + match[0].length - 1 + const group = extractBalancedParenGroup(query, openIndex) + return group ? group.content.trim() : undefined +} + +// Scans a single attribute's trailing text (everything after `name type`) for the +// first top-level (paren-depth 0) modifier keyword, so a type argument like +// `Decimal(9, 2)` isn't mistaken for the start of a modifier. +function splitAttributeTypeAndModifiers(rest: string): { type: string; tail: string } { + let depth = 0 + let inString = false + let stringQuote = "'" + for (let i = 0; i < rest.length; i += 1) { + const char = rest[i] + if (!char) continue + if (inString) { + if (char === stringQuote && rest[i - 1] !== '\\') inString = false + continue + } + if (char === "'" || char === '"') { + inString = true + stringQuote = char + continue + } + if (char === '(') { + depth += 1 + continue + } + if (char === ')') { + depth -= 1 + continue + } + const prevChar = rest[i - 1] + if (depth === 0 && /[A-Za-z_]/.test(char) && (i === 0 || /\s/.test(prevChar ?? ' '))) { + const remainder = rest.slice(i) + for (const keyword of MODIFIER_KEYWORDS) { + if (new RegExp(`^${keyword}\\b`, 'i').test(remainder)) { + return { type: rest.slice(0, i).trim(), tail: remainder.trim() } + } + } + } + } + return { type: rest.trim(), tail: '' } +} + +function consumeQuotedOrBareValue(text: string): { value: string | number; rest: string } { + const trimmed = text.trimStart() + if (trimmed.startsWith("'")) { + let out = '' + let i = 1 + while (i < trimmed.length) { + const char = trimmed[i] + if (char === "'") { + if (trimmed[i + 1] === "'") { + out += "'" + i += 2 + continue + } + i += 1 + break + } + if (char === '\\' && trimmed[i + 1] === "'") { + out += "'" + i += 2 + continue + } + out += char + i += 1 + } + return { value: out, rest: trimmed.slice(i) } + } + const match = trimmed.match(/^\S+/) + const token = match?.[0] ?? '' + const rest = trimmed.slice(token.length) + const num = Number(token) + return { value: token !== '' && !Number.isNaN(num) ? num : token, rest } +} + +function applyAttributeModifiers(attribute: ParsedDictionaryAttribute, tail: string): void { + let remaining = tail.trim() + while (remaining.length > 0) { + if (/^DEFAULT\b/i.test(remaining)) { + remaining = remaining.replace(/^DEFAULT\s+/i, '') + const { value, rest } = consumeQuotedOrBareValue(remaining) + attribute.default = value + remaining = rest.trim() + continue + } + if (/^EXPRESSION\b/i.test(remaining)) { + remaining = remaining.replace(/^EXPRESSION\s+/i, '') + const { type: expression, tail: exprRest } = splitAttributeTypeAndModifiers(remaining) + attribute.expression = expression + remaining = exprRest.trim() + continue + } + if (/^HIERARCHICAL\b/i.test(remaining)) { + attribute.hierarchical = true + remaining = remaining.replace(/^HIERARCHICAL\b/i, '').trim() + continue + } + if (/^INJECTIVE\b/i.test(remaining)) { + attribute.injective = true + remaining = remaining.replace(/^INJECTIVE\b/i, '').trim() + continue + } + if (/^IS_OBJECT_ID\b/i.test(remaining)) { + attribute.isObjectId = true + remaining = remaining.replace(/^IS_OBJECT_ID\b/i, '').trim() + continue + } + break + } +} + +export function parseDictionaryAttributesFromCreateDictionaryQuery( + query: string | undefined +): ParsedDictionaryAttribute[] { + const body = extractCreateDictionaryBody(query) + if (!body) return [] + const attributes: ParsedDictionaryAttribute[] = [] + for (const part of splitTopLevelComma(body)) { + const trimmed = part.trim() + if (!trimmed) continue + const nameMatch = trimmed.match(/^(?:`([^`]+)`|([A-Za-z_]\w*))\s+([\s\S]+)$/) + if (!nameMatch) continue + const name = (nameMatch[1] ?? nameMatch[2] ?? '').trim() + const rest = (nameMatch[3] ?? '').trim() + if (!name || !rest) continue + const { type, tail } = splitAttributeTypeAndModifiers(rest) + if (!type) continue + const attribute: ParsedDictionaryAttribute = { name, type } + applyAttributeModifiers(attribute, tail) + attributes.push(attribute) + } + return attributes +} + +export function parseDictionaryPrimaryKeyFromCreateDictionaryQuery( + query: string | undefined +): string[] { + if (!query) return [] + const match = + /\)\s*PRIMARY\s+KEY\s+([\s\S]*?)(?:\bSOURCE\s*\(|\bLAYOUT\s*\(|\bLIFETIME\s*\(|\bCOMMENT\b|;|$)/i.exec( + query + ) + const raw = match?.[1]?.trim() + if (!raw) return [] + return splitTopLevelComma(raw) + .map((part) => part.trim().replace(/^`|`$/g, '')) + .filter(Boolean) +} + +export function parseSourceFromCreateDictionaryQuery(query: string | undefined): string | undefined { + if (!query) return undefined + return extractKeywordParenBody(query, 'SOURCE') +} + +export function parseLayoutFromCreateDictionaryQuery(query: string | undefined): string | undefined { + if (!query) return undefined + return extractKeywordParenBody(query, 'LAYOUT') +} + +export function parseLifetimeFromCreateDictionaryQuery(query: string | undefined): string | undefined { + if (!query) return undefined + return extractKeywordParenBody(query, 'LIFETIME') +} + +export function parseCommentFromCreateDictionaryQuery(query: string | undefined): string | undefined { + if (!query) return undefined + const match = /\bCOMMENT\s+'((?:[^'\\]|\\.|'')*)'/i.exec(query) + if (!match?.[1]) return undefined + return match[1].replace(/\\'/g, "'").replace(/''/g, "'") +} diff --git a/packages/clickhouse/src/index.test.ts b/packages/clickhouse/src/index.test.ts index b5e3cffb..20541312 100644 --- a/packages/clickhouse/src/index.test.ts +++ b/packages/clickhouse/src/index.test.ts @@ -10,12 +10,18 @@ import { createStatelessClickHouseClient, formatConnectionError, inferSchemaKindFromEngine, + parseCommentFromCreateDictionaryQuery, + parseDictionaryAttributesFromCreateDictionaryQuery, + parseDictionaryPrimaryKeyFromCreateDictionaryQuery, parseEngineFromCreateTableQuery, + parseLayoutFromCreateDictionaryQuery, + parseLifetimeFromCreateDictionaryQuery, parseOrderByFromCreateTableQuery, parsePartitionByFromCreateTableQuery, parsePrimaryKeyFromCreateTableQuery, parseProjectionsFromCreateTableQuery, parseSettingsFromCreateTableQuery, + parseSourceFromCreateDictionaryQuery, parseTTLFromCreateTableQuery, parseUniqueKeyFromCreateTableQuery, } from './index' @@ -158,7 +164,7 @@ describe('@chkit/clickhouse smoke', () => { expect(inferSchemaKindFromEngine('MergeTree')).toBe('table') expect(inferSchemaKindFromEngine('View')).toBe('view') expect(inferSchemaKindFromEngine('MaterializedView')).toBe('materialized_view') - expect(inferSchemaKindFromEngine('Dictionary')).toBeNull() + expect(inferSchemaKindFromEngine('Dictionary')).toBe('dictionary') }) test('parses settings and ttl from create table query', () => { @@ -486,3 +492,65 @@ describe('formatConnectionError', () => { expect(formatConnectionError(new Error('Unknown data type family: Nope'), 'https://x')).toBeUndefined() }) }) + +describe('create-dictionary-parser', () => { + const query = `CREATE DICTIONARY default.users_dict +( + \`id\` UInt64, + \`name\` String, + \`email\` String DEFAULT '', + \`parent_id\` UInt64 HIERARCHICAL +) +PRIMARY KEY id +SOURCE(MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')) +LAYOUT(HASHED()) +LIFETIME(MIN 0 MAX 300) +COMMENT 'User lookup dictionary'` + + test('parses attributes including defaults and modifiers', () => { + expect(parseDictionaryAttributesFromCreateDictionaryQuery(query)).toEqual([ + { name: 'id', type: 'UInt64' }, + { name: 'name', type: 'String' }, + { name: 'email', type: 'String', default: '' }, + { name: 'parent_id', type: 'UInt64', hierarchical: true }, + ]) + }) + + test('parses primary key', () => { + expect(parseDictionaryPrimaryKeyFromCreateDictionaryQuery(query)).toEqual(['id']) + }) + + test('parses source, layout, lifetime, comment', () => { + expect(parseSourceFromCreateDictionaryQuery(query)).toBe( + "MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')" + ) + expect(parseLayoutFromCreateDictionaryQuery(query)).toBe('HASHED()') + expect(parseLifetimeFromCreateDictionaryQuery(query)).toBe('MIN 0 MAX 300') + expect(parseCommentFromCreateDictionaryQuery(query)).toBe('User lookup dictionary') + }) + + test('returns empty results for undefined query', () => { + expect(parseDictionaryAttributesFromCreateDictionaryQuery(undefined)).toEqual([]) + expect(parseDictionaryPrimaryKeyFromCreateDictionaryQuery(undefined)).toEqual([]) + expect(parseSourceFromCreateDictionaryQuery(undefined)).toBeUndefined() + }) + + test('parses a composite primary key and an expression attribute', () => { + const compositeQuery = `CREATE DICTIONARY default.pairs_dict +( + \`a\` String, + \`b\` String, + \`full_name\` String EXPRESSION concat(a, ' ', b) +) +PRIMARY KEY a, b +SOURCE(HTTP(url 'http://example.com/pairs' format 'TSV')) +LAYOUT(COMPLEX_KEY_HASHED()) +LIFETIME(300)` + expect(parseDictionaryPrimaryKeyFromCreateDictionaryQuery(compositeQuery)).toEqual(['a', 'b']) + expect(parseDictionaryAttributesFromCreateDictionaryQuery(compositeQuery)).toEqual([ + { name: 'a', type: 'String' }, + { name: 'b', type: 'String' }, + { name: 'full_name', type: 'String', expression: "concat(a, ' ', b)" }, + ]) + }) +}) diff --git a/packages/clickhouse/src/index.ts b/packages/clickhouse/src/index.ts index d1504e46..8a3390b6 100644 --- a/packages/clickhouse/src/index.ts +++ b/packages/clickhouse/src/index.ts @@ -92,7 +92,7 @@ export interface ClickHouseExecutor { } export interface SchemaObjectRef { - kind: 'table' | 'view' | 'materialized_view' + kind: 'table' | 'view' | 'materialized_view' | 'dictionary' database: string name: string } @@ -159,13 +159,23 @@ export { parseTTLFromCreateTableQuery, parseUniqueKeyFromCreateTableQuery, } from './create-table-parser.js' +export { + parseCommentFromCreateDictionaryQuery, + parseDictionaryAttributesFromCreateDictionaryQuery, + parseDictionaryPrimaryKeyFromCreateDictionaryQuery, + parseLayoutFromCreateDictionaryQuery, + parseLifetimeFromCreateDictionaryQuery, + parseSourceFromCreateDictionaryQuery, + type ParsedDictionaryAttribute, +} from './create-dictionary-parser.js' export function inferSchemaKindFromEngine( engine: string, ): SchemaObjectRef['kind'] | null { if (engine === 'View') return 'view' if (engine === 'MaterializedView') return 'materialized_view' - if (!engine || engine === 'Dictionary') return null + if (engine === 'Dictionary') return 'dictionary' + if (!engine) return null return 'table' } @@ -468,6 +478,7 @@ export function assertStreamedQuerySucceeded(input: { export { waitForColumn, waitForDDLPropagation, + waitForDictionary, waitForTable, waitForTableAbsent, waitForView, diff --git a/packages/plugin-pull/src/index.test.ts b/packages/plugin-pull/src/index.test.ts index c8f99770..2d0a3fad 100644 --- a/packages/plugin-pull/src/index.test.ts +++ b/packages/plugin-pull/src/index.test.ts @@ -259,6 +259,42 @@ export default schema(app_events, app_events_view, analytics_daily_mv) expect(content).toContain('export default schema(app_events_view, app_events_mv)') }) + test('renders a dictionary definition with a hidden-secret note', () => { + const content = renderSchemaFile([ + { + kind: 'dictionary', + database: 'app', + name: 'users_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'name', type: 'String' }, + { name: 'email', type: 'String', default: '' }, + ], + primaryKey: ['id'], + source: "MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')", + layout: 'HASHED()', + lifetime: '300', + comment: 'User lookup dictionary', + }, + ]) + + expect(content).toContain("import { schema, dictionary } from '@chkit/core'") + expect(content).toContain( + "// NOTE: password redacted by ClickHouse — replace '[HIDDEN]' with your credential (e.g. process.env.X)." + ) + expect(content).toContain('const app_users_dict = dictionary({') + expect(content).toContain('{ name: "id", type: "UInt64" }') + expect(content).toContain('{ name: "email", type: "String", default: "" }') + expect(content).toContain('primaryKey: ["id"]') + expect(content).toContain( + 'source: "MYSQL(host \'db\' port 3306 user \'reader\' password \'[HIDDEN]\' db \'app\' table \'users\')"' + ) + expect(content).toContain('layout: "HASHED()"') + expect(content).toContain('lifetime: "300"') + expect(content).toContain('comment: "User lookup dictionary"') + expect(content).toContain('export default schema(app_users_dict)') + }) + test('renders refreshable materialized view with full refresh block', () => { const content = renderSchemaFile([ { @@ -633,6 +669,46 @@ AS SELECT today() AS day, toUInt64(1) AS total` refresh: { every: '1 HOUR', append: true }, }) }) + + test('mapSystemTableRowToDefinition parses a dictionary row', () => { + const definition = __testUtils.mapSystemTableRowToDefinition({ + database: 'app', + name: 'users_dict', + engine: 'Dictionary', + create_table_query: `CREATE DICTIONARY app.users_dict +( + \`id\` UInt64, + \`name\` String +) +PRIMARY KEY id +SOURCE(MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')) +LAYOUT(HASHED()) +LIFETIME(MIN 0 MAX 300)`, + }) + expect(definition).toEqual({ + kind: 'dictionary', + database: 'app', + name: 'users_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'name', type: 'String' }, + ], + primaryKey: ['id'], + source: "MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')", + layout: 'HASHED()', + lifetime: 'MIN 0 MAX 300', + }) + }) + + test('mapSystemTableRowToDefinition returns null for a dictionary with an unparsable query', () => { + const definition = __testUtils.mapSystemTableRowToDefinition({ + database: 'app', + name: 'broken_dict', + engine: 'Dictionary', + create_table_query: '', + }) + expect(definition).toBeNull() + }) }) describe('@chkit/plugin-pull skipped objects summary', () => { diff --git a/packages/plugin-pull/src/index.ts b/packages/plugin-pull/src/index.ts index dbb6be03..5f0f2e82 100644 --- a/packages/plugin-pull/src/index.ts +++ b/packages/plugin-pull/src/index.ts @@ -270,7 +270,11 @@ async function pullSchema(input: { } const outFile = resolve(process.cwd(), input.options.outFile) - let objects: Array<{ kind: 'table' | 'view' | 'materialized_view'; database: string; name: string }> = [] + let objects: Array<{ + kind: 'table' | 'view' | 'materialized_view' | 'dictionary' + database: string + name: string + }> = [] let selectedDatabases = input.options.databases if (db && (!customIntrospector || selectedDatabases.length === 0)) { @@ -346,6 +350,19 @@ function mapIntrospectedObjectToDefinition(introspected: IntrospectedObject): Sc as: introspected.as, } } + if (introspected.kind === 'dictionary') { + return { + kind: 'dictionary', + database: introspected.database, + name: introspected.name, + attributes: introspected.attributes, + primaryKey: introspected.primaryKey, + source: introspected.source, + layout: introspected.layout, + lifetime: introspected.lifetime, + ...(introspected.comment ? { comment: introspected.comment } : {}), + } + } return { kind: 'materialized_view', database: introspected.database, @@ -367,7 +384,11 @@ function normalizeDefault(value: TableDefinition['columns'][number]['default']): } function summarizeSkippedObjects( - objects: Array<{ kind: 'table' | 'view' | 'materialized_view'; database: string; name: string }>, + objects: Array<{ + kind: 'table' | 'view' | 'materialized_view' | 'dictionary' + database: string + name: string + }>, definitions: SchemaDefinition[], selectedDatabases: string[] ): Array<{ kind: string; count: number }> { @@ -423,7 +444,7 @@ async function listNonTableRows( FROM system.tables WHERE is_temporary = 0 AND database IN (${quotedDatabases}) - AND engine IN ('View', 'MaterializedView')` + AND engine IN ('View', 'MaterializedView', 'Dictionary')` ) } diff --git a/packages/plugin-pull/src/render-schema.ts b/packages/plugin-pull/src/render-schema.ts index 14ec4a71..974e865b 100644 --- a/packages/plugin-pull/src/render-schema.ts +++ b/packages/plugin-pull/src/render-schema.ts @@ -4,6 +4,8 @@ import { type ColumnCodec, type ColumnCodecSpec, type ColumnDefinition, + type DictionaryAttribute, + type DictionaryDefinition, type MaterializedViewDefinition, type MaterializedViewRefresh, type ProjectionDefinition, @@ -40,6 +42,7 @@ function renderImportStatement(canonical: SchemaDefinition[]): string { const hasTable = canonical.some((definition) => definition.kind === 'table') const hasView = canonical.some((definition) => definition.kind === 'view') const hasMaterializedView = canonical.some((definition) => definition.kind === 'materialized_view') + const hasDictionary = canonical.some((definition) => definition.kind === 'dictionary') const hasRawCodec = canonical.some( (definition) => definition.kind === 'table' && @@ -49,6 +52,7 @@ function renderImportStatement(canonical: SchemaDefinition[]): string { if (hasTable) imports.push('table') if (hasView) imports.push('view') if (hasMaterializedView) imports.push('materializedView') + if (hasDictionary) imports.push('dictionary') if (hasRawCodec) imports.push('codec') return `import { ${imports.join(', ')} } from '@chkit/core'` } @@ -59,6 +63,8 @@ function renderDefinition(definition: SchemaDefinition, variableName: string): s return renderTableDefinition(definition, variableName) case 'view': return renderViewDefinition(definition, variableName) + case 'dictionary': + return renderDictionaryDefinition(definition, variableName) default: return renderMaterializedViewDefinition(definition, variableName) } @@ -117,6 +123,44 @@ function renderMaterializedViewDefinition( return lines } +const HIDDEN_SECRET_NOTE = + "// NOTE: password redacted by ClickHouse — replace '[HIDDEN]' with your credential (e.g. process.env.X)." + +function renderDictionaryDefinition(definition: DictionaryDefinition, variableName: string): string[] { + const lines: string[] = [] + if (definition.source.includes('[HIDDEN]')) { + lines.push(HIDDEN_SECRET_NOTE) + } + lines.push(...renderDeclarationHeader('dictionary', variableName, definition.database, definition.name)) + lines.push(' attributes: [') + for (const attribute of definition.attributes) { + lines.push(` ${renderDictionaryAttribute(attribute)},`) + } + lines.push(' ],') + lines.push(` primaryKey: ${renderStringArray(definition.primaryKey)},`) + lines.push(` source: ${renderString(definition.source)},`) + lines.push(` layout: ${renderString(definition.layout)},`) + lines.push(` lifetime: ${renderString(definition.lifetime)},`) + if (definition.comment) { + lines.push(` comment: ${renderString(definition.comment)},`) + } + lines.push('})') + return lines +} + +function renderDictionaryAttribute(attribute: DictionaryAttribute): string { + const parts: string[] = [`name: ${renderString(attribute.name)}`, `type: ${renderString(attribute.type)}`] + if (attribute.expression !== undefined) { + parts.push(`expression: ${renderString(attribute.expression)}`) + } else if (attribute.default !== undefined) { + parts.push(`default: ${renderLiteral(attribute.default)}`) + } + if (attribute.hierarchical) parts.push('hierarchical: true') + if (attribute.injective) parts.push('injective: true') + if (attribute.isObjectId) parts.push('isObjectId: true') + return `{ ${parts.join(', ')} }` +} + function renderDeclarationHeader( factory: string, variableName: string, diff --git a/packages/plugin-pull/src/view-parser.ts b/packages/plugin-pull/src/view-parser.ts index 5ab81e56..554d4841 100644 --- a/packages/plugin-pull/src/view-parser.ts +++ b/packages/plugin-pull/src/view-parser.ts @@ -1,5 +1,15 @@ -import { inferSchemaKindFromEngine, type IntrospectedTable } from '@chkit/clickhouse' +import { + inferSchemaKindFromEngine, + parseCommentFromCreateDictionaryQuery, + parseDictionaryAttributesFromCreateDictionaryQuery, + parseDictionaryPrimaryKeyFromCreateDictionaryQuery, + parseLayoutFromCreateDictionaryQuery, + parseLifetimeFromCreateDictionaryQuery, + parseSourceFromCreateDictionaryQuery, + type IntrospectedTable, +} from '@chkit/clickhouse' import type { + DictionaryDefinition, MaterializedViewDefinition, MaterializedViewRefresh, ViewDefinition, @@ -19,8 +29,38 @@ export type IntrospectedObject = MaterializedViewDefinition, 'database' | 'name' | 'to' | 'as' | 'refresh' >) + | ({ kind: 'dictionary' } & Pick< + DictionaryDefinition, + 'database' | 'name' | 'attributes' | 'primaryKey' | 'source' | 'layout' | 'lifetime' | 'comment' + >) | IntrospectedTable +function mapDictionaryRowToDefinition( + row: SystemTableRow +): Extract | null { + const query = row.create_table_query + const attributes = parseDictionaryAttributesFromCreateDictionaryQuery(query) + const primaryKey = parseDictionaryPrimaryKeyFromCreateDictionaryQuery(query) + const source = parseSourceFromCreateDictionaryQuery(query) + const layout = parseLayoutFromCreateDictionaryQuery(query) + const lifetime = parseLifetimeFromCreateDictionaryQuery(query) + if (attributes.length === 0 || primaryKey.length === 0 || !source || !layout || !lifetime) { + return null + } + const comment = parseCommentFromCreateDictionaryQuery(query) + return { + kind: 'dictionary', + database: row.database, + name: row.name, + attributes, + primaryKey, + source, + layout, + lifetime, + ...(comment ? { comment } : {}), + } +} + export function mapSystemTableRowToDefinition( row: SystemTableRow ): Exclude | null { @@ -38,6 +78,9 @@ export function mapSystemTableRowToDefinition( const base = { kind: 'materialized_view' as const, database: row.database, name: row.name, to, as } return refresh ? { ...base, refresh } : base } + if (kind === 'dictionary') { + return mapDictionaryRowToDefinition(row) + } return null } From 4b4d0b3cc0883b72af7d7716ed1fd769adf48ff1 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Tue, 14 Jul 2026 12:21:18 +0200 Subject: [PATCH 04/15] feat(plugin-codegen): generate typed interfaces for dictionaries Emit a TypeScript interface (and Zod schema when emitZod is enabled) from a dictionary's attributes, sharing the same field-rendering path as table row types. Dictionaries are always included in codegen output regardless of includeViews, since their shape is structured rather than an arbitrary query. Co-Authored-By: Claude Sonnet 5 --- .../src/generators/type-artifacts.ts | 53 +++++++++--- packages/plugin-codegen/src/index.test.ts | 82 ++++++++++++++++++- packages/plugin-codegen/src/naming.ts | 14 +++- packages/plugin-codegen/src/types.ts | 3 +- 4 files changed, 137 insertions(+), 15 deletions(-) diff --git a/packages/plugin-codegen/src/generators/type-artifacts.ts b/packages/plugin-codegen/src/generators/type-artifacts.ts index 1f7c9561..90eaece0 100644 --- a/packages/plugin-codegen/src/generators/type-artifacts.ts +++ b/packages/plugin-codegen/src/generators/type-artifacts.ts @@ -1,6 +1,7 @@ import { canonicalizeDefinitions, type ColumnDefinition, + type DictionaryDefinition, type MaterializedViewDefinition, type TableDefinition, type ViewDefinition, @@ -230,17 +231,18 @@ export function mapColumnType( return { tsType, zodType: resolved.zodType, nullable } } -function renderTableInterface( - table: TableDefinition, +function renderFieldsInterface( + fields: ColumnDefinition[], interfaceName: string, + pathPrefix: string, options: Required ): { lines: string[]; findings: CodegenFinding[] } { const lines: string[] = [`export type ${interfaceName} = {`] const findings: CodegenFinding[] = [] const zodFields: string[] = [] - for (const column of table.columns) { - const path = `${table.database}.${table.name}.${column.name}` + for (const column of fields) { + const path = `${pathPrefix}.${column.name}` const mapped = mapColumnType({ column, path }, options) if (mapped.finding) findings.push(mapped.finding) lines.push(` ${renderPropertyName(column.name)}: ${mapped.tsType}`) @@ -261,6 +263,31 @@ function renderTableInterface( return { lines, findings } } +function renderTableInterface( + table: TableDefinition, + interfaceName: string, + options: Required +): { lines: string[]; findings: CodegenFinding[] } { + return renderFieldsInterface(table.columns, interfaceName, `${table.database}.${table.name}`, options) +} + +function renderDictionaryInterface( + definition: DictionaryDefinition, + interfaceName: string, + options: Required +): { lines: string[]; findings: CodegenFinding[] } { + const fields: ColumnDefinition[] = definition.attributes.map((attribute) => ({ + name: attribute.name, + type: attribute.type, + })) + return renderFieldsInterface( + fields, + interfaceName, + `${definition.database}.${definition.name}`, + options + ) +} + function renderViewInterface( definition: ViewDefinition | MaterializedViewDefinition, interfaceName: string @@ -284,11 +311,15 @@ export function generateTypeArtifacts( const normalized = normalizeCodegenOptions(input.options) const definitions = canonicalizeDefinitions(input.definitions) const sortedDefinitions = definitions - .filter((definition): definition is TableDefinition | ViewDefinition | MaterializedViewDefinition => { - if (definition.kind === 'table') return true - if (!normalized.includeViews) return false - return definition.kind === 'view' || definition.kind === 'materialized_view' - }) + .filter( + ( + definition + ): definition is TableDefinition | ViewDefinition | MaterializedViewDefinition | DictionaryDefinition => { + if (definition.kind === 'table' || definition.kind === 'dictionary') return true + if (!normalized.includeViews) return false + return definition.kind === 'view' || definition.kind === 'materialized_view' + } + ) .sort((a, b) => { if (a.database !== b.database) return a.database.localeCompare(b.database) return a.name.localeCompare(b.name) @@ -302,7 +333,9 @@ export function generateTypeArtifacts( const rendered = entry.definition.kind === 'table' ? renderTableInterface(entry.definition, entry.interfaceName, normalized) - : renderViewInterface(entry.definition, entry.interfaceName) + : entry.definition.kind === 'dictionary' + ? renderDictionaryInterface(entry.definition, entry.interfaceName, normalized) + : renderViewInterface(entry.definition, entry.interfaceName) findings.push(...rendered.findings) bodyLines.push(...rendered.lines) bodyLines.push('') diff --git a/packages/plugin-codegen/src/index.test.ts b/packages/plugin-codegen/src/index.test.ts index 40b176d1..f7d321ab 100644 --- a/packages/plugin-codegen/src/index.test.ts +++ b/packages/plugin-codegen/src/index.test.ts @@ -4,7 +4,7 @@ import { dirname, join, resolve } from 'node:path' import { tmpdir } from 'node:os' import { pathToFileURL } from 'node:url' -import { resolveConfig, schema, table } from '@chkit/core' +import { dictionary, resolveConfig, schema, table } from '@chkit/core' import { createCodegenPlugin, @@ -386,6 +386,86 @@ describe('@chkit/plugin-codegen generation', () => { `// This file is auto-generated by chkit codegen — do not edit manually.\n// chkit-codegen-version: 0.1.0\n\nimport { z } from 'zod'\n\nexport type AppUsersRow = {\n id: string\n email: string | null\n}\n\nexport const AppUsersRowSchema = z.object({\n id: z.string(),\n email: z.string().nullable(),\n})\n\nexport type AppUsersRowInput = z.input\nexport type AppUsersRowOutput = z.output\n` ) }) + + test('generates a typed interface for a dictionary from its attributes', () => { + const definitions = schema( + dictionary({ + database: 'app', + name: 'users_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'email', type: 'String' }, + ], + primaryKey: ['id'], + source: "MYSQL(host 'db' port 3306 user 'reader' password '' db 'app' table 'users')", + layout: 'HASHED()', + lifetime: '300', + }) + ) + + const result = generateTypeArtifacts({ + definitions, + options: { tableNameStyle: 'pascal', bigintMode: 'string' }, + now: new Date('2026-01-01T00:00:00.000Z'), + toolVersion: '0.1.0', + }) + + expect(result.content).toBe( + `// This file is auto-generated by chkit codegen — do not edit manually.\n// chkit-codegen-version: 0.1.0\n\nexport type AppUsersDictRow = {\n id: string\n email: string\n}\n` + ) + expect(result.declarationCount).toBe(1) + }) + + test('generates a dictionary interface with Zod schema when emitZod=true', () => { + const definitions = schema( + dictionary({ + database: 'app', + name: 'users_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'email', type: 'String' }, + ], + primaryKey: ['id'], + source: "MYSQL(host 'db' port 3306 user 'reader' password '' db 'app' table 'users')", + layout: 'HASHED()', + lifetime: '300', + }) + ) + + const result = generateTypeArtifacts({ + definitions, + options: { emitZod: true }, + now: new Date('2026-01-01T00:00:00.000Z'), + toolVersion: '0.1.0', + }) + + expect(result.content).toBe( + `// This file is auto-generated by chkit codegen — do not edit manually.\n// chkit-codegen-version: 0.1.0\n\nimport { z } from 'zod'\n\nexport type AppUsersDictRow = {\n id: string\n email: string\n}\n\nexport const AppUsersDictRowSchema = z.object({\n id: z.string(),\n email: z.string(),\n})\n\nexport type AppUsersDictRowInput = z.input\nexport type AppUsersDictRowOutput = z.output\n` + ) + }) + + test('a dictionary is always included regardless of includeViews', () => { + const definitions = schema( + dictionary({ + database: 'app', + name: 'users_dict', + attributes: [{ name: 'id', type: 'UInt64' }], + primaryKey: ['id'], + source: "MYSQL(host 'db' port 3306 user 'reader' password '' db 'app' table 'users')", + layout: 'HASHED()', + lifetime: '300', + }) + ) + + const result = generateTypeArtifacts({ + definitions, + options: { includeViews: false }, + now: new Date('2026-01-01T00:00:00.000Z'), + }) + + expect(result.declarationCount).toBe(1) + expect(result.content).toContain('export type AppUsersDictRow') + }) }) describe('@chkit/plugin-codegen ingest generation', () => { diff --git a/packages/plugin-codegen/src/naming.ts b/packages/plugin-codegen/src/naming.ts index e6018936..59ac168f 100644 --- a/packages/plugin-codegen/src/naming.ts +++ b/packages/plugin-codegen/src/naming.ts @@ -1,4 +1,9 @@ -import type { MaterializedViewDefinition, TableDefinition, ViewDefinition } from '@chkit/core' +import type { + DictionaryDefinition, + MaterializedViewDefinition, + TableDefinition, + ViewDefinition, +} from '@chkit/core' import type { CodegenPluginOptions, ResolvedTableName } from './types.js' @@ -39,7 +44,10 @@ export function renderPropertyName(name: string): string { } function baseRowTypeName( - definition: Pick, + definition: Pick< + TableDefinition | ViewDefinition | MaterializedViewDefinition | DictionaryDefinition, + 'database' | 'name' + >, style: Required['tableNameStyle'] ): string { const combined = `${definition.database}_${definition.name}` @@ -54,7 +62,7 @@ function baseRowTypeName( } export function resolveTableNames( - definitions: Array, + definitions: Array, style: Required['tableNameStyle'] ): ResolvedTableName[] { const baseNames = definitions.map((definition) => ({ diff --git a/packages/plugin-codegen/src/types.ts b/packages/plugin-codegen/src/types.ts index a3e91f6f..ea417791 100644 --- a/packages/plugin-codegen/src/types.ts +++ b/packages/plugin-codegen/src/types.ts @@ -1,5 +1,6 @@ import type { ChxInlinePluginRegistration, + DictionaryDefinition, FlagDef, FlagMapping, MaterializedViewDefinition, @@ -132,7 +133,7 @@ export interface CodegenPluginCheckResult { } export interface ResolvedTableName { - definition: TableDefinition | ViewDefinition | MaterializedViewDefinition + definition: TableDefinition | ViewDefinition | MaterializedViewDefinition | DictionaryDefinition interfaceName: string } From a0c35811c5f5db6fd8d3e6876b36c54cbb115a66 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Tue, 14 Jul 2026 12:22:01 +0200 Subject: [PATCH 05/15] test(e2e): add live dictionary lifecycle test and EXPLAIN AST coverage Add waitForDictionary() (polls system.dictionaries) alongside waitForTable/waitForView, and migrate-dictionary.e2e.test.ts covering create -> insert -> dictGet -> structural CREATE OR REPLACE -> dictGet -> drift-clean -> hand-drop -> drift-dirty (missing_object) -> restore -> drop-blocked-without---allow-destructive. Add CREATE DICTIONARY / CREATE OR REPLACE DICTIONARY / dictionary drop cases to the EXPLAIN AST SQL-validation suite. Verified end-to-end against a local ClickHouse 26.6 Docker container: all cases pass, plus a manual pull round-trip against a CLICKHOUSE()-sourced dictionary confirmed system.tables.create_table_query is populated for DDL dictionaries and the pulled schema file typechecks. Co-Authored-By: Claude Sonnet 5 --- packages/cli/src/test/e2e-testkit.ts | 1 + .../src/test/migrate-dictionary.e2e.test.ts | 253 ++++++++++++++++++ packages/clickhouse/src/e2e-testkit.ts | 1 + packages/core/src/sql-validation.e2e.test.ts | 109 +++++++- 4 files changed, 362 insertions(+), 2 deletions(-) create mode 100644 packages/cli/src/test/migrate-dictionary.e2e.test.ts diff --git a/packages/cli/src/test/e2e-testkit.ts b/packages/cli/src/test/e2e-testkit.ts index 1584fbe2..681225be 100644 --- a/packages/cli/src/test/e2e-testkit.ts +++ b/packages/cli/src/test/e2e-testkit.ts @@ -18,6 +18,7 @@ export { waitForTable, waitForView, waitForColumn, + waitForDictionary, waitForRows, } from '@chkit/clickhouse/e2e-testkit' diff --git a/packages/cli/src/test/migrate-dictionary.e2e.test.ts b/packages/cli/src/test/migrate-dictionary.e2e.test.ts new file mode 100644 index 00000000..9568ba04 --- /dev/null +++ b/packages/cli/src/test/migrate-dictionary.e2e.test.ts @@ -0,0 +1,253 @@ +import { describe, expect, test } from 'bun:test' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { + CORE_ENTRY, + createJournalTableName, + createLiveExecutor, + createPrefix, + formatTestDiagnostic, + getRequiredEnv, + quoteIdent, + runCli, + runCliWithRetry, + waitForDictionary, + waitForTable, +} from './e2e-testkit.js' + +/** + * Sources the dictionary from ClickHouse's own HTTP interface (SOURCE(HTTP(...))) + * pointed back at the seed table, rather than a native-protocol CLICKHOUSE() + * source or a filesystem FILE() source — both require connection details + * (native TCP port, server-local file paths) this suite cannot assume for a + * managed target like ObsessionDB. HTTP only needs the same URL/credentials + * `getRequiredEnv()` already validated. + */ +function renderSchema(input: { + database: string + tableName: string + dictName: string + clickhouseUrl: string + clickhouseUser: string + clickhousePassword: string + lifetime: string +}): string { + const query = `SELECT id, name FROM ${input.database}.${input.tableName} FORMAT JSONEachRow` + const sourceUrl = `${input.clickhouseUrl}/?query=${encodeURIComponent(query)}&user=${encodeURIComponent(input.clickhouseUser)}&password=${encodeURIComponent(input.clickhousePassword)}` + + return ( + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\n` + + `const users = table({\n` + + ` database: '${input.database}',\n` + + ` name: '${input.tableName}',\n` + + ` columns: [\n` + + ` { name: 'id', type: 'UInt64' },\n` + + ` { name: 'name', type: 'String' },\n` + + ` ],\n` + + ` engine: 'MergeTree()',\n` + + ` primaryKey: ['id'],\n` + + ` orderBy: ['id'],\n` + + `})\n\n` + + `const usersDict = dictionary({\n` + + ` database: '${input.database}',\n` + + ` name: '${input.dictName}',\n` + + ` attributes: [\n` + + ` { name: 'id', type: 'UInt64' },\n` + + ` { name: 'name', type: 'String' },\n` + + ` ],\n` + + ` primaryKey: ['id'],\n` + + ` source: "HTTP(url '${sourceUrl}' format 'JSONEachRow')",\n` + + ` layout: 'HASHED()',\n` + + ` lifetime: '${input.lifetime}',\n` + + `})\n\n` + + `export default schema(users, usersDict)\n` + ) +} + +function renderTableOnlySchema(database: string, tableName: string): string { + return ( + `import { schema, table } from '${CORE_ENTRY}'\n\n` + + `export default schema(table({\n` + + ` database: '${database}',\n` + + ` name: '${tableName}',\n` + + ` columns: [\n` + + ` { name: 'id', type: 'UInt64' },\n` + + ` { name: 'name', type: 'String' },\n` + + ` ],\n` + + ` engine: 'MergeTree()',\n` + + ` primaryKey: ['id'],\n` + + ` orderBy: ['id'],\n` + + `}))\n` + ) +} + +describe('@chkit/cli migrate dictionary e2e', () => { + test( + 'create -> replace -> drift -> drop lifecycle for a ClickHouse dictionary', + async () => { + const liveEnv = getRequiredEnv() + const executor = createLiveExecutor(liveEnv) + const database = liveEnv.clickhouseDatabase + const journalTable = createJournalTableName('dict') + const cliEnv = { CHKIT_JOURNAL_TABLE: journalTable, CI: '1' } + const prefix = createPrefix('dict') + const tableName = `${prefix}users` + const dictName = `${prefix}users_dict` + + const dir = await mkdtemp(join(tmpdir(), 'chkit-dict-e2e-')) + const configPath = join(dir, 'clickhouse.config.ts') + const schemaPath = join(dir, 'schema.ts') + const outDir = join(dir, 'chkit') + + const schemaInput = { + database, + tableName, + dictName, + clickhouseUrl: liveEnv.clickhouseUrl, + clickhouseUser: liveEnv.clickhouseUser, + clickhousePassword: liveEnv.clickhousePassword, + } + + try { + await writeFile(schemaPath, renderSchema({ ...schemaInput, lifetime: '300' }), 'utf8') + await writeFile( + configPath, + `export default {\n` + + ` schema: '${schemaPath}',\n` + + ` outDir: '${outDir}',\n` + + ` migrationsDir: '${join(outDir, 'migrations')}',\n` + + ` metaDir: '${join(outDir, 'meta')}',\n` + + ` clickhouse: {\n` + + ` url: '${liveEnv.clickhouseUrl}',\n` + + ` username: '${liveEnv.clickhouseUser}',\n` + + ` password: '${liveEnv.clickhousePassword}',\n` + + ` database: '${database}',\n` + + ` },\n}\n`, + 'utf8' + ) + + // 1. generate + migrate creates the source table and the dictionary. + const genInit = runCli( + dir, + ['generate', '--config', configPath, '--name', 'init', '--migration-id', '20990101000000', '--json'], + cliEnv + ) + expect(genInit.exitCode).toBe(0) + + const migrateInit = await runCliWithRetry( + dir, + ['migrate', '--config', configPath, '--execute', '--json'], + { extraEnv: cliEnv } + ) + if (migrateInit.exitCode !== 0) { + throw new Error(formatTestDiagnostic('migrate --execute (init) failed', migrateInit)) + } + await waitForTable(executor, database, tableName) + await waitForDictionary(executor, database, dictName) + + await executor.command( + `INSERT INTO ${quoteIdent(database)}.${quoteIdent(tableName)} (id, name) VALUES (1, 'Alice')` + ) + + const seeded = await executor.query<{ name: string }>( + `SELECT dictGet('${database}.${dictName}', 'name', toUInt64(1)) AS name` + ) + expect(seeded[0]?.name).toBe('Alice') + + // 2. a structural change (lifetime) regenerates as a single CREATE OR + // REPLACE DICTIONARY; the dictionary keeps serving correct data after. + await writeFile(schemaPath, renderSchema({ ...schemaInput, lifetime: '600' }), 'utf8') + const genReplace = runCli( + dir, + ['generate', '--config', configPath, '--name', 'replace', '--migration-id', '20990101000001', '--json'], + cliEnv + ) + expect(genReplace.exitCode).toBe(0) + const replaceMigrationPath = join(outDir, 'migrations', '20990101000001_replace.sql') + const replaceSql = await Bun.file(replaceMigrationPath).text() + expect(replaceSql).toContain('CREATE OR REPLACE DICTIONARY') + + const migrateReplace = await runCliWithRetry( + dir, + ['migrate', '--config', configPath, '--execute', '--json'], + { extraEnv: cliEnv } + ) + if (migrateReplace.exitCode !== 0) { + throw new Error(formatTestDiagnostic('migrate --execute (replace) failed', migrateReplace)) + } + await waitForDictionary(executor, database, dictName) + + const afterReplace = await executor.query<{ name: string }>( + `SELECT dictGet('${database}.${dictName}', 'name', toUInt64(1)) AS name` + ) + expect(afterReplace[0]?.name).toBe('Alice') + + // 3. drift is clean immediately after apply; existence-drift (the MV + // precedent — dictionaries are not deep-shape-compared) is detected + // once the dictionary is hand-dropped out from under chkit. + const driftClean = runCli(dir, ['drift', '--config', configPath, '--json'], cliEnv) + expect(driftClean.exitCode).toBe(0) + const driftCleanPayload = JSON.parse(driftClean.stdout) as { drifted: boolean } + expect(driftCleanPayload.drifted).toBe(false) + + await executor.command(`DROP DICTIONARY IF EXISTS ${quoteIdent(database)}.${quoteIdent(dictName)}`) + + const driftDirty = runCli(dir, ['drift', '--config', configPath, '--json'], cliEnv) + expect(driftDirty.exitCode).toBe(0) + const driftDirtyPayload = JSON.parse(driftDirty.stdout) as { + drifted: boolean + objectDrift: Array<{ code: string; object: string }> + } + expect(driftDirtyPayload.drifted).toBe(true) + expect( + driftDirtyPayload.objectDrift.some( + (item) => item.code === 'missing_object' && item.object === `dictionary:${database}.${dictName}` + ) + ).toBe(true) + + // Restore the dictionary directly (out-of-band, like the hand-DROP + // above) so the drop-blocking assertion below observes a real + // `drop_dictionary` op rather than a no-op create. `chkit migrate` + // can't do this restore itself: the schema file hasn't changed since + // the last generate, so there's no new pending migration to apply — + // the journal already recorded the dictionary as created. + const createDictionaryStatement = replaceSql.match(/CREATE OR REPLACE DICTIONARY[\s\S]*?;/)?.[0] + expect(createDictionaryStatement).toBeTruthy() + await executor.command(createDictionaryStatement as string) + await waitForDictionary(executor, database, dictName) + + // 4. removing the dictionary from schema plans a DROP DICTIONARY, + // blocked without --allow-destructive. + await writeFile(schemaPath, renderTableOnlySchema(database, tableName), 'utf8') + const genDrop = runCli( + dir, + ['generate', '--config', configPath, '--name', 'drop_dict', '--migration-id', '20990101000002', '--json'], + cliEnv + ) + expect(genDrop.exitCode).toBe(0) + + const migrateDrop = runCli(dir, ['migrate', '--config', configPath, '--execute', '--json'], cliEnv) + expect(migrateDrop.exitCode).toBe(3) + const dropPayload = JSON.parse(migrateDrop.stdout) as { + destructiveOperations: Array<{ type: string; warningCode: string }> + } + expect(dropPayload.destructiveOperations.some((op) => op.type === 'drop_dictionary')).toBe(true) + expect( + dropPayload.destructiveOperations.some((op) => op.warningCode === 'drop_dictionary_dependency_break') + ).toBe(true) + + // Blocked, not applied — the dictionary must still exist. + await waitForDictionary(executor, database, dictName) + } finally { + await rm(dir, { recursive: true, force: true }) + await executor.command(`DROP DICTIONARY IF EXISTS ${quoteIdent(database)}.${quoteIdent(dictName)}`) + await executor.command(`DROP TABLE IF EXISTS ${quoteIdent(database)}.${quoteIdent(tableName)}`) + await executor.command(`DROP TABLE IF EXISTS ${quoteIdent(database)}.${quoteIdent(journalTable)}`) + await executor.close() + } + }, + 240_000 + ) +}) diff --git a/packages/clickhouse/src/e2e-testkit.ts b/packages/clickhouse/src/e2e-testkit.ts index 13f2e20b..efcf5667 100644 --- a/packages/clickhouse/src/e2e-testkit.ts +++ b/packages/clickhouse/src/e2e-testkit.ts @@ -106,5 +106,6 @@ export { waitForTable, waitForView, waitForColumn, + waitForDictionary, waitForRows, } from './ddl-propagation.js' diff --git a/packages/core/src/sql-validation.e2e.test.ts b/packages/core/src/sql-validation.e2e.test.ts index 824c5593..92038af6 100644 --- a/packages/core/src/sql-validation.e2e.test.ts +++ b/packages/core/src/sql-validation.e2e.test.ts @@ -13,8 +13,8 @@ import type { SkipIndexDefinition, TableDefinition, } from './model-types.js' -import { table, view, materializedView } from './model.js' -import { toCreateSQL } from './sql.js' +import { table, view, materializedView, dictionary } from './model.js' +import { toCreateSQL, renderDictionarySQL } from './sql.js' import { renderAlterAddColumn, renderAlterModifyColumn, @@ -879,6 +879,67 @@ ORDER BY (\`id\`, toDate(\`created_at\`))` }) }) + // ========================================================================= + // CREATE DICTIONARY + // ========================================================================= + + describe('CREATE DICTIONARY', () => { + const baseDictionary = { + database: 'default', + name: 'test_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'name', type: 'String' }, + ], + primaryKey: ['id'], + source: "FILE(path '/dev/null' format 'CSV')", + layout: 'FLAT()', + lifetime: '300', + } as const + + test('minimal dictionary', async () => { + const def = dictionary({ ...baseDictionary }) + await assertValidSQL(client, toCreateSQL(def)) + }) + + test('dictionary with DEFAULT / EXPRESSION / HIERARCHICAL attributes', async () => { + const def = dictionary({ + ...baseDictionary, + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'name', type: 'String', default: '' }, + { name: 'parent_id', type: 'UInt64', hierarchical: true }, + { name: 'upper_name', type: 'String', expression: 'upper(name)' }, + ], + }) + await assertValidSQL(client, toCreateSQL(def)) + }) + + test('dictionary with COMPLEX_KEY_HASHED layout and composite primary key', async () => { + const def = dictionary({ + ...baseDictionary, + attributes: [ + { name: 'a', type: 'String' }, + { name: 'b', type: 'String' }, + { name: 'value', type: 'String' }, + ], + primaryKey: ['a', 'b'], + layout: 'COMPLEX_KEY_HASHED()', + }) + await assertValidSQL(client, toCreateSQL(def)) + }) + + test('dictionary with comment', async () => { + const def = dictionary({ ...baseDictionary, comment: 'A test dictionary' }) + await assertValidSQL(client, toCreateSQL(def)) + }) + + test('CREATE OR REPLACE DICTIONARY', async () => { + const def = dictionary({ ...baseDictionary }) + await assertValidSQL(client, renderDictionarySQL(def, true)) + }) + }) + // ========================================================================= // ALTER TABLE — MODIFY REFRESH // ========================================================================= @@ -1221,6 +1282,50 @@ ORDER BY (\`id\`, toDate(\`created_at\`))` } }) + test('dictionary structural change — CREATE OR REPLACE', async () => { + const oldDict = dictionary({ + database: 'default', + name: 'plan_dict', + attributes: [{ name: 'id', type: 'UInt64' }, { name: 'name', type: 'String' }], + primaryKey: ['id'], + source: "FILE(path '/dev/null' format 'CSV')", + layout: 'FLAT()', + lifetime: '300', + }) + const newDict = dictionary({ + database: 'default', + name: 'plan_dict', + attributes: [{ name: 'id', type: 'UInt64' }, { name: 'name', type: 'String' }], + primaryKey: ['id'], + source: "FILE(path '/dev/null' format 'CSV')", + layout: 'FLAT()', + lifetime: '600', + }) + const plan = planDiff([oldDict], [newDict]) + expect(plan.operations.length).toBe(1) + for (const op of plan.operations) { + await assertValidSQL(client, op.sql) + } + }) + + test('dictionary drop', async () => { + const oldDict = dictionary({ + database: 'default', + name: 'plan_dict_drop', + attributes: [{ name: 'id', type: 'UInt64' }], + primaryKey: ['id'], + source: "FILE(path '/dev/null' format 'CSV')", + layout: 'FLAT()', + lifetime: '300', + }) + const plan = planDiff([oldDict], []) + expect(plan.operations.length).toBe(1) + expect(plan.operations[0]?.type).toBe('drop_dictionary') + for (const op of plan.operations) { + await assertValidSQL(client, op.sql) + } + }) + test('CREATE DATABASE', async () => { const newDef = table({ database: 'analytics', From 0568c7aa53b80d3ea482c953527e2fe75a199272 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Tue, 14 Jul 2026 12:22:24 +0200 Subject: [PATCH 06/15] docs: document the dictionary schema primitive Add dictionary() to the DSL reference (fields, attributes, credential handling via env-var interpolation, no-ALTER semantics), mention it in the schema overview and ClickHouse compatibility guide, and document the pull plugin's [HIDDEN]-password handling and codegen's dictionary interface generation. Co-Authored-By: Claude Sonnet 5 --- .../docs/guides/clickhouse-compatibility.md | 1 + apps/docs/src/content/docs/plugins/codegen.md | 3 +- apps/docs/src/content/docs/plugins/pull.md | 35 +++++++- .../src/content/docs/schema/dsl-reference.md | 86 ++++++++++++++++++- apps/docs/src/content/docs/schema/overview.md | 4 +- 5 files changed, 124 insertions(+), 5 deletions(-) diff --git a/apps/docs/src/content/docs/guides/clickhouse-compatibility.md b/apps/docs/src/content/docs/guides/clickhouse-compatibility.md index 0cae56b4..84a4caf6 100644 --- a/apps/docs/src/content/docs/guides/clickhouse-compatibility.md +++ b/apps/docs/src/content/docs/guides/clickhouse-compatibility.md @@ -20,6 +20,7 @@ A few schema features depend on the ClickHouse version of your target: | [Refreshable materialized views](/schema/refreshable-views/) | Production-ready on **24.10+** (no flag). Experimental and flag-gated on 23.12–24.9. chkit targets 24.10+. | | `set` data-skipping index | **ClickHouse 26+** requires the `set(0)` form rather than a bare `set`; chkit emits `set(maxRows)` accordingly. See the [DSL reference](/schema/dsl-reference/). | | `uniqueKey` | Renders `UNIQUE KEY` DDL, which is supported on ObsessionDB / ClickHouse Cloud engines but rejected by vanilla `MergeTree`. | +| [`dictionary()`](/schema/dsl-reference/#dictionary) | DDL `CREATE DICTIONARY` only — supported broadly on 21.x+. chkit does not model XML-config dictionaries or `RENAME`/`SYSTEM RELOAD DICTIONARY`. | If you target a single-node open-source ClickHouse, prefer the standard `MergeTree` engine family and avoid the Cloud/Shared-only features above. diff --git a/apps/docs/src/content/docs/plugins/codegen.md b/apps/docs/src/content/docs/plugins/codegen.md index 556d88e4..fd704b2e 100644 --- a/apps/docs/src/content/docs/plugins/codegen.md +++ b/apps/docs/src/content/docs/plugins/codegen.md @@ -10,6 +10,7 @@ This document covers practical usage of the optional `codegen` plugin. ## What it does - Generates deterministic TypeScript row types from chkit schema definitions. +- Generates a typed interface (and optional Zod schema) for each `dictionary()` from its `attributes` — dictionaries are always included, regardless of `includeViews`. - Optionally generates Zod schemas from the same definitions. - Optionally generates typed ingestion functions for inserting rows into ClickHouse tables. Generated ingest helpers gzip-compress request bodies by default and can opt out per call. - Optionally generates a self-contained runtime migration module with all migration SQL inlined, for environments without filesystem access (e.g., Cloudflare Workers). @@ -186,4 +187,4 @@ When `runOnGenerate` is enabled (the default), the migration module is regenerat - Query-level type inference is not included. - Arbitrary SQL expression typing is not included. -- Views/materialized views are opt-in and emitted conservatively. +- Views/materialized views are opt-in (`includeViews`) and emitted conservatively (`{ [key: string]: unknown }`). Dictionaries are always included and typed from `attributes`, since their shape is structured rather than an arbitrary query. diff --git a/apps/docs/src/content/docs/plugins/pull.md b/apps/docs/src/content/docs/plugins/pull.md index 2b12d138..9bb5729e 100644 --- a/apps/docs/src/content/docs/plugins/pull.md +++ b/apps/docs/src/content/docs/plugins/pull.md @@ -1,6 +1,6 @@ --- title: Pull Plugin -description: Introspect live ClickHouse tables, views, and materialized views and generate chkit schema files. +description: Introspect live ClickHouse tables, views, materialized views, and dictionaries and generate chkit schema files. sidebar: order: 3 --- @@ -11,6 +11,7 @@ This document covers practical usage of the optional `pull` plugin. - Connects to a live ClickHouse instance and introspects table metadata (columns, engines, indexes, projections, partitioning, TTL, settings). - Introspects views and materialized views (including `TO` clause parsing). +- Introspects dictionaries (attributes, primary key, `SOURCE`/`LAYOUT`/`LIFETIME`), preserving ClickHouse's `[HIDDEN]` password redaction — see [Credential handling](#credential-handling-hidden-passwords). - Generates a deterministic TypeScript schema file using `@chkit/core` builders. - Supports filtering by database and dry-run previews. @@ -106,7 +107,39 @@ export default schema(app_events, app_events_view, app_events_mv) Tables may also include `uniqueKey`, `ttl`, `settings`, `indexes`, and `projections` when present in the source metadata. +## Credential handling (`[HIDDEN]` passwords) + +ClickHouse redacts inline `SOURCE(...)` passwords to `[HIDDEN]` on introspection (`system.tables.create_table_query`, `SHOW CREATE DICTIONARY`) and does not offer a way to recover the original value via `pull` — chkit does not attempt `format_display_secrets_in_show_and_select`. When a pulled dictionary's `source` contains `[HIDDEN]`, the generated file emits it verbatim with a leading comment: + +```ts +// NOTE: password redacted by ClickHouse — replace '[HIDDEN]' with your credential (e.g. process.env.X). +const default_users_dict = dictionary({ + database: "default", + name: "users_dict", + attributes: [ + { name: "id", type: "UInt64" }, + { name: "name", type: "String" }, + ], + primaryKey: ["id"], + source: "MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')", + layout: "HASHED()", + lifetime: "300", +}) +``` + +Replace `[HIDDEN]` with a real credential — typically an environment-variable interpolation, matching how you'd author the dictionary by hand (see [Credentials in `source`](/schema/dsl-reference/#credentials-in-source)): + +```ts +source: `MYSQL(host 'db' port 3306 user 'reader' password '${process.env.MYSQL_PASSWORD}' db 'app' table 'users')`, +``` + +For round-trip fidelity without a manual edit, use [named collections](https://clickhouse.com/docs/operations/named-collections) on the ClickHouse side instead of an inline password — chkit does not require this, but it's the ClickHouse-native way to avoid the redaction entirely. + +Because chkit masks the `password '...'` token before comparing `source` for drift, a live `[HIDDEN]` value is never reported as perpetual drift against your real secret once you've filled it in. + ## Current limits - Materialized views without a `TO` clause are skipped. +- Dictionaries whose `create_table_query` can't be parsed (attributes, primary key, or `SOURCE`/`LAYOUT`/`LIFETIME` missing) are skipped. +- XML-config dictionaries (not created via DDL) are not introspected. - Requires a live ClickHouse connection. diff --git a/apps/docs/src/content/docs/schema/dsl-reference.md b/apps/docs/src/content/docs/schema/dsl-reference.md index 753cbc1d..0f4d9645 100644 --- a/apps/docs/src/content/docs/schema/dsl-reference.md +++ b/apps/docs/src/content/docs/schema/dsl-reference.md @@ -8,7 +8,7 @@ sidebar: Schema files are TypeScript files that export definitions using functions from `@chkit/core`. All exported definitions are collected when chkit loads schema files matched by the `schema` glob in your [configuration](/configuration/overview/). ```ts -import { schema, table, view, materializedView } from '@chkit/core' +import { schema, table, view, materializedView, dictionary } from '@chkit/core' ``` ## `schema()` @@ -353,6 +353,84 @@ const dailyReport = materializedView({ See [Refreshable materialized views](/schema/refreshable-views/) for the full `refresh` field reference, including APPEND mode, `DEPENDS ON`, and the ClickHouse rules that chkit validates. +## `dictionary()` + +Creates a [ClickHouse dictionary](https://clickhouse.com/docs/sql-reference/dictionaries) definition — a key-value lookup structure backed by an external or in-database source, queried with `dictGet()`. + +```ts +dictionary(input: Omit): DictionaryDefinition +``` + +```ts +import { dictionary } from '@chkit/core' + +const usersDict = dictionary({ + database: 'default', + name: 'users_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'name', type: 'String' }, + { name: 'email', type: 'String', default: '' }, + ], + primaryKey: ['id'], + source: `MYSQL(host 'db' port 3306 user 'reader' password '${process.env.MYSQL_PASSWORD}' db 'app' table 'users')`, + layout: `HASHED()`, + lifetime: `300`, + comment: 'User lookup dictionary', +}) +``` + +### Required fields + +| Field | Type | Description | +|-------|------|-------------| +| `database` | `string` | ClickHouse database name | +| `name` | `string` | Dictionary name | +| `attributes` | `DictionaryAttribute[]` | Attribute definitions (see [Dictionary attributes](#dictionary-attributes)) | +| `primaryKey` | `string[]` | Key attribute name(s) — every entry must name a declared attribute | +| `source` | `string` | Raw `SOURCE(...)` body, e.g. `` `MYSQL(host '...' password '...' ...)` `` | +| `layout` | `string` | Raw `LAYOUT(...)` body, e.g. `` `HASHED()` `` or `` `COMPLEX_KEY_HASHED()` `` | +| `lifetime` | `string` | Raw `LIFETIME(...)` body, e.g. `` `300` `` or `` `MIN 300 MAX 360` `` | + +### Optional fields + +| Field | Type | Description | +|-------|------|-------------| +| `comment` | `string` | Dictionary comment | +| `renamedFrom` | `{ database?: string; name: string }` | Previous identity for rename tracking | + +:::note +`source`, `layout`, and `lifetime` are **raw strings**, not a typed sub-DSL — chkit passes them through to ClickHouse verbatim inside `SOURCE(...)`, `LAYOUT(...)`, and `LIFETIME(...)` respectively. There is no key/layout coupling validation or required-parameter checking (e.g. `size_in_cells` for `HASHED`); ClickHouse validates the DDL when it's applied. +::: + +### Dictionary attributes + +Each entry in the `attributes` array is a `DictionaryAttribute`. + +| Field | Type | Description | +|-------|------|-------------| +| `name` | `string` | Attribute name | +| `type` | `string` | ClickHouse type | +| `default` | `string \| number \| boolean` | `DEFAULT` value for missing keys. Mutually exclusive with `expression` | +| `expression` | `string` | `EXPRESSION` computed from source columns. Mutually exclusive with `default` | +| `hierarchical` | `boolean` | Marks the attribute `HIERARCHICAL` | +| `injective` | `boolean` | Marks the attribute `INJECTIVE` | +| `isObjectId` | `boolean` | Marks the attribute `IS_OBJECT_ID` (MongoDB sources) | + +### Credentials in `source` + +Inline credentials in `source` (e.g. a MySQL/PostgreSQL `password '...'`) should be interpolated from environment variables at schema-authoring time, the same way you'd handle any other secret in a TypeScript config file: + +```ts +source: `MYSQL(host 'db' password '${process.env.MYSQL_PASSWORD}' ...)`, +``` + +ClickHouse redacts inline passwords back to `[HIDDEN]` on introspection (`SHOW CREATE DICTIONARY`, `system.dictionaries`). chkit masks the `password '...'` token before comparing `source` for drift/diff purposes, so a live `[HIDDEN]` value is never reported as perpetual drift against your real secret — see [Pull: credential handling](/plugins/pull/#credential-handling-hidden-passwords). + +### No `ALTER DICTIONARY` + +ClickHouse has no `ALTER DICTIONARY` — every structural change to a dictionary is rendered as a single `CREATE OR REPLACE DICTIONARY` statement (atomic, dependency-safe). See [Structural vs. alterable properties](#structural-vs-alterable-properties). + ## Type system reference The [codegen plugin](/plugins/codegen/) maps ClickHouse types to TypeScript types using these rules: @@ -448,6 +526,10 @@ chkit validates schema definitions and throws a `ChxValidationError` if any issu - **Empty codec chain** (`codec_chain_empty`) -- a `codec` array with no steps; provide at least one codec or omit the field - **Multiple general codecs** (`codec_chain_multiple_general`) -- more than one general codec in a chain; only one is allowed - **Codec chain must end with a general codec** (`codec_chain_must_end_with_general`) -- preprocessors must precede the single general codec (`NONE`, `LZ4`, `LZ4HC`, `ZSTD`, `T64`, `GCD`, `ALP`) +- **Dictionary missing primary key** (`dictionary_missing_primary_key`) -- a dictionary's `primaryKey` is empty +- **Dictionary primary key references missing attribute** (`dictionary_primary_key_missing_attribute`) -- a `primaryKey` entry doesn't name a declared attribute +- **Dictionary missing source/layout/lifetime** (`dictionary_missing_source`, `dictionary_missing_layout`, `dictionary_missing_lifetime`) -- one of these raw-string fields is empty +- **Dictionary attribute default/expression exclusive** (`dictionary_attribute_default_expression_exclusive`) -- an attribute sets both `default` and `expression` ## Structural vs. alterable properties @@ -459,6 +541,8 @@ When a property changes, chkit determines whether the table can be altered in pl Views and materialized views always use drop + recreate. +Dictionaries have no ALTER at all: any change to `attributes`, `primaryKey`, `layout`, `lifetime`, or `comment` renders as a single `CREATE OR REPLACE DICTIONARY` (`risk=caution`). A `source` change is compared with the password masked (see [Credentials in `source`](#credentials-in-source)), so only a real credential/connection change triggers a replace. Removing a dictionary from schema emits `DROP DICTIONARY` (`risk=danger`, requires `--allow-destructive`). + :::danger Changing a structural property on an existing table generates a `DROP TABLE` followed by `CREATE TABLE` — **all rows are permanently deleted and the table is recreated empty**. The data is not copied over. The drop is classified `risk=danger` (blocked without `--allow-destructive`) and `chkit migrate` flags it with the distinct `table_recreate_data_loss` warning. To preserve data, migrate by hand instead: create a new table with the desired structure, `INSERT INTO new SELECT ... FROM old`, then swap names and drop the old table. ::: diff --git a/apps/docs/src/content/docs/schema/overview.md b/apps/docs/src/content/docs/schema/overview.md index de7656d6..40c65dd4 100644 --- a/apps/docs/src/content/docs/schema/overview.md +++ b/apps/docs/src/content/docs/schema/overview.md @@ -5,7 +5,7 @@ sidebar: order: 1 --- -In chkit, your ClickHouse schema lives in TypeScript files. You declare tables, views, and materialized views as plain values using functions from `@chkit/core`, group them with `schema()`, and let chkit handle the rest — diffing them against the database, generating migration SQL, and applying it safely. +In chkit, your ClickHouse schema lives in TypeScript files. You declare tables, views, materialized views, and dictionaries as plain values using functions from `@chkit/core`, group them with `schema()`, and let chkit handle the rest — diffing them against the database, generating migration SQL, and applying it safely. A typical schema file looks like this: @@ -33,7 +33,7 @@ chkit discovers schema files using the `schema` glob in your [configuration](/co ## Concepts -- **Definitions** — tables, views, and materialized views are values created with `table()`, `view()`, and `materializedView()`. They describe the *desired* state of your database. +- **Definitions** — tables, views, materialized views, and dictionaries are values created with `table()`, `view()`, `materializedView()`, and `dictionary()`. They describe the *desired* state of your database. - **Schema groups** — `schema(...)` collects definitions into a single export, but any exported definition is also discovered automatically. - **Diff + plan** — when you run `chkit generate`, chkit compares your schema to the live database (or the last applied state) and emits migration SQL. - **Engines** — `MergeTree`, `ReplacingMergeTree`, `AggregatingMergeTree`, and their `Shared` variants for [ObsessionDB](https://obsessiondb.com) are all first-class. From e8360f2324cb47a61421127e497e712e7401a8b5 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Wed, 15 Jul 2026 14:03:48 +0200 Subject: [PATCH 07/15] feat(core,clickhouse,plugin-pull): support dictionary RANGE, SETTINGS, and BIDIRECTIONAL The dictionary model, SQL renderer, and introspection parser only covered PRIMARY KEY/SOURCE/LAYOUT/LIFETIME and a handful of attribute modifiers, so chkit silently dropped RANGE(...) (required by RANGE_HASHED layouts), SETTINGS(...), and the BIDIRECTIONAL attribute modifier on every pull and re-render. Verified the exact rendered DDL against a live ClickHouse instance, including the full introspect-then-parse round trip. Co-Authored-By: Claude Sonnet 5 --- apps/docs/src/content/docs/plugins/pull.md | 2 +- .../src/content/docs/schema/dsl-reference.md | 5 ++ .../src/create-dictionary-parser.ts | 49 +++++++++++++- packages/clickhouse/src/index.test.ts | 39 +++++++++++ packages/clickhouse/src/index.ts | 2 + packages/core/src/canonical.ts | 8 +++ packages/core/src/index.test.ts | 48 ++++++++++++++ packages/core/src/model-types.ts | 8 +++ packages/core/src/sql-validation.e2e.test.ts | 28 ++++++++ packages/core/src/sql.ts | 22 ++++++- packages/core/src/validate.ts | 22 +++++++ packages/plugin-pull/src/index.test.ts | 64 +++++++++++++++++++ packages/plugin-pull/src/index.ts | 2 + packages/plugin-pull/src/render-schema.ts | 9 +++ packages/plugin-pull/src/view-parser.ts | 17 ++++- 15 files changed, 321 insertions(+), 4 deletions(-) diff --git a/apps/docs/src/content/docs/plugins/pull.md b/apps/docs/src/content/docs/plugins/pull.md index 9bb5729e..aad3a1f5 100644 --- a/apps/docs/src/content/docs/plugins/pull.md +++ b/apps/docs/src/content/docs/plugins/pull.md @@ -11,7 +11,7 @@ This document covers practical usage of the optional `pull` plugin. - Connects to a live ClickHouse instance and introspects table metadata (columns, engines, indexes, projections, partitioning, TTL, settings). - Introspects views and materialized views (including `TO` clause parsing). -- Introspects dictionaries (attributes, primary key, `SOURCE`/`LAYOUT`/`LIFETIME`), preserving ClickHouse's `[HIDDEN]` password redaction — see [Credential handling](#credential-handling-hidden-passwords). +- Introspects dictionaries (attributes — including `HIERARCHICAL`/`BIDIRECTIONAL`/`INJECTIVE`/`IS_OBJECT_ID` modifiers — primary key, `SOURCE`/`LAYOUT`/`LIFETIME`/`RANGE`/`SETTINGS`), preserving ClickHouse's `[HIDDEN]` password redaction — see [Credential handling](#credential-handling-hidden-passwords). - Generates a deterministic TypeScript schema file using `@chkit/core` builders. - Supports filtering by database and dry-run previews. diff --git a/apps/docs/src/content/docs/schema/dsl-reference.md b/apps/docs/src/content/docs/schema/dsl-reference.md index 0f4d9645..7ceb4568 100644 --- a/apps/docs/src/content/docs/schema/dsl-reference.md +++ b/apps/docs/src/content/docs/schema/dsl-reference.md @@ -396,6 +396,8 @@ const usersDict = dictionary({ | Field | Type | Description | |-------|------|-------------| +| `range` | `{ min: string; max: string }` | `RANGE(MIN ... MAX ...)` — required by `RANGE_HASHED` / `COMPLEX_KEY_RANGE_HASHED` layouts. Both `min` and `max` must name declared attributes | +| `settings` | `Record` | Raw `SETTINGS(...)` key/value pairs, e.g. `{ dictionary_use_async_executor: 1 }` | | `comment` | `string` | Dictionary comment | | `renamedFrom` | `{ database?: string; name: string }` | Previous identity for rename tracking | @@ -414,6 +416,7 @@ Each entry in the `attributes` array is a `DictionaryAttribute`. | `default` | `string \| number \| boolean` | `DEFAULT` value for missing keys. Mutually exclusive with `expression` | | `expression` | `string` | `EXPRESSION` computed from source columns. Mutually exclusive with `default` | | `hierarchical` | `boolean` | Marks the attribute `HIERARCHICAL` | +| `bidirectional` | `boolean` | Marks the attribute `BIDIRECTIONAL` — enables parent/child lookups in both directions. Only valid alongside `hierarchical` | | `injective` | `boolean` | Marks the attribute `INJECTIVE` | | `isObjectId` | `boolean` | Marks the attribute `IS_OBJECT_ID` (MongoDB sources) | @@ -530,6 +533,8 @@ chkit validates schema definitions and throws a `ChxValidationError` if any issu - **Dictionary primary key references missing attribute** (`dictionary_primary_key_missing_attribute`) -- a `primaryKey` entry doesn't name a declared attribute - **Dictionary missing source/layout/lifetime** (`dictionary_missing_source`, `dictionary_missing_layout`, `dictionary_missing_lifetime`) -- one of these raw-string fields is empty - **Dictionary attribute default/expression exclusive** (`dictionary_attribute_default_expression_exclusive`) -- an attribute sets both `default` and `expression` +- **Dictionary range references missing attribute** (`dictionary_range_missing_attribute`) -- `range.min`/`range.max` doesn't name a declared attribute +- **Dictionary bidirectional requires hierarchical** (`dictionary_bidirectional_requires_hierarchical`) -- an attribute sets `bidirectional` without `hierarchical` ## Structural vs. alterable properties diff --git a/packages/clickhouse/src/create-dictionary-parser.ts b/packages/clickhouse/src/create-dictionary-parser.ts index 55f7755d..84d70033 100644 --- a/packages/clickhouse/src/create-dictionary-parser.ts +++ b/packages/clickhouse/src/create-dictionary-parser.ts @@ -6,11 +6,19 @@ export interface ParsedDictionaryAttribute { default?: string | number expression?: string hierarchical?: boolean + bidirectional?: boolean injective?: boolean isObjectId?: boolean } -const MODIFIER_KEYWORDS = ['DEFAULT', 'EXPRESSION', 'HIERARCHICAL', 'INJECTIVE', 'IS_OBJECT_ID'] +const MODIFIER_KEYWORDS = [ + 'DEFAULT', + 'EXPRESSION', + 'HIERARCHICAL', + 'BIDIRECTIONAL', + 'INJECTIVE', + 'IS_OBJECT_ID', +] function extractBalancedParenGroup( text: string, @@ -161,6 +169,11 @@ function applyAttributeModifiers(attribute: ParsedDictionaryAttribute, tail: str remaining = remaining.replace(/^HIERARCHICAL\b/i, '').trim() continue } + if (/^BIDIRECTIONAL\b/i.test(remaining)) { + attribute.bidirectional = true + remaining = remaining.replace(/^BIDIRECTIONAL\b/i, '').trim() + continue + } if (/^INJECTIVE\b/i.test(remaining)) { attribute.injective = true remaining = remaining.replace(/^INJECTIVE\b/i, '').trim() @@ -234,3 +247,37 @@ export function parseCommentFromCreateDictionaryQuery(query: string | undefined) if (!match?.[1]) return undefined return match[1].replace(/\\'/g, "'").replace(/''/g, "'") } + +function stripBackticks(value: string): string { + const trimmed = value.trim() + return trimmed.startsWith('`') && trimmed.endsWith('`') ? trimmed.slice(1, -1) : trimmed +} + +export function parseDictionaryRangeFromCreateDictionaryQuery( + query: string | undefined +): { min: string; max: string } | undefined { + if (!query) return undefined + const body = extractKeywordParenBody(query, 'RANGE') + if (!body) return undefined + const match = /^MIN\s+(\S+)\s+MAX\s+(\S+)$/i.exec(body.trim()) + if (!match?.[1] || !match[2]) return undefined + return { min: stripBackticks(match[1]), max: stripBackticks(match[2]) } +} + +export function parseDictionarySettingsFromCreateDictionaryQuery( + query: string | undefined +): Record | undefined { + if (!query) return undefined + const body = extractKeywordParenBody(query, 'SETTINGS') + if (!body) return undefined + const out: Record = {} + for (const part of splitTopLevelComma(body)) { + const eq = part.indexOf('=') + if (eq === -1) continue + const key = part.slice(0, eq).trim() + if (!key) continue + const { value } = consumeQuotedOrBareValue(part.slice(eq + 1).trim()) + out[key] = value + } + return Object.keys(out).length > 0 ? out : undefined +} diff --git a/packages/clickhouse/src/index.test.ts b/packages/clickhouse/src/index.test.ts index 20541312..f99c1edb 100644 --- a/packages/clickhouse/src/index.test.ts +++ b/packages/clickhouse/src/index.test.ts @@ -13,6 +13,8 @@ import { parseCommentFromCreateDictionaryQuery, parseDictionaryAttributesFromCreateDictionaryQuery, parseDictionaryPrimaryKeyFromCreateDictionaryQuery, + parseDictionaryRangeFromCreateDictionaryQuery, + parseDictionarySettingsFromCreateDictionaryQuery, parseEngineFromCreateTableQuery, parseLayoutFromCreateDictionaryQuery, parseLifetimeFromCreateDictionaryQuery, @@ -553,4 +555,41 @@ LIFETIME(300)` { name: 'full_name', type: 'String', expression: "concat(a, ' ', b)" }, ]) }) + + test('parses RANGE, SETTINGS, and BIDIRECTIONAL modifier', () => { + const rangeQuery = `CREATE DICTIONARY default.rates_dict +( + \`id\` UInt64, + \`parent_id\` UInt64 HIERARCHICAL BIDIRECTIONAL, + \`start_date\` DateTime, + \`end_date\` DateTime +) +PRIMARY KEY id +SOURCE(HTTP(url 'http://example.com/rates' format 'TSV')) +LIFETIME(MIN 0 MAX 300) +LAYOUT(RANGE_HASHED()) +RANGE(MIN start_date MAX end_date) +SETTINGS(dictionary_use_async_executor = 1, max_threads = 8)` + expect(parseDictionaryAttributesFromCreateDictionaryQuery(rangeQuery)).toEqual([ + { name: 'id', type: 'UInt64' }, + { name: 'parent_id', type: 'UInt64', hierarchical: true, bidirectional: true }, + { name: 'start_date', type: 'DateTime' }, + { name: 'end_date', type: 'DateTime' }, + ]) + expect(parseDictionaryRangeFromCreateDictionaryQuery(rangeQuery)).toEqual({ + min: 'start_date', + max: 'end_date', + }) + expect(parseDictionarySettingsFromCreateDictionaryQuery(rangeQuery)).toEqual({ + dictionary_use_async_executor: 1, + max_threads: 8, + }) + }) + + test('RANGE and SETTINGS return undefined when absent', () => { + expect(parseDictionaryRangeFromCreateDictionaryQuery(query)).toBeUndefined() + expect(parseDictionarySettingsFromCreateDictionaryQuery(query)).toBeUndefined() + expect(parseDictionaryRangeFromCreateDictionaryQuery(undefined)).toBeUndefined() + expect(parseDictionarySettingsFromCreateDictionaryQuery(undefined)).toBeUndefined() + }) }) diff --git a/packages/clickhouse/src/index.ts b/packages/clickhouse/src/index.ts index 8a3390b6..ca3d6270 100644 --- a/packages/clickhouse/src/index.ts +++ b/packages/clickhouse/src/index.ts @@ -163,6 +163,8 @@ export { parseCommentFromCreateDictionaryQuery, parseDictionaryAttributesFromCreateDictionaryQuery, parseDictionaryPrimaryKeyFromCreateDictionaryQuery, + parseDictionaryRangeFromCreateDictionaryQuery, + parseDictionarySettingsFromCreateDictionaryQuery, parseLayoutFromCreateDictionaryQuery, parseLifetimeFromCreateDictionaryQuery, parseSourceFromCreateDictionaryQuery, diff --git a/packages/core/src/canonical.ts b/packages/core/src/canonical.ts index c838770b..ee54820e 100644 --- a/packages/core/src/canonical.ts +++ b/packages/core/src/canonical.ts @@ -180,6 +180,10 @@ function canonicalizeDictionaryAttribute(attribute: DictionaryAttribute): Dictio } function canonicalizeDictionary(def: DictionaryDefinition): DictionaryDefinition { + const settings = def.settings + ? Object.fromEntries(Object.entries(def.settings).sort(([a], [b]) => a.localeCompare(b))) + : undefined + return { ...def, database: def.database.trim(), @@ -195,6 +199,10 @@ function canonicalizeDictionary(def: DictionaryDefinition): DictionaryDefinition source: normalizeSQLFragment(def.source), layout: normalizeSQLFragment(def.layout), lifetime: normalizeSQLFragment(def.lifetime), + range: def.range + ? { min: def.range.min.trim(), max: def.range.max.trim() } + : undefined, + settings: settings && Object.keys(settings).length > 0 ? settings : undefined, comment: def.comment?.trim(), } } diff --git a/packages/core/src/index.test.ts b/packages/core/src/index.test.ts index 7f1fbe78..56d3da62 100644 --- a/packages/core/src/index.test.ts +++ b/packages/core/src/index.test.ts @@ -1783,6 +1783,54 @@ describe('@chkit/core dictionaries', () => { expect(plan.operations).toHaveLength(0) }) + test('renders RANGE, SETTINGS, and BIDIRECTIONAL clauses', () => { + const dict = dictionary({ + ...baseDictionary, + attributes: [ + ...baseDictionary.attributes, + { name: 'parent_id', type: 'UInt64', hierarchical: true, bidirectional: true }, + { name: 'start_date', type: 'DateTime' }, + { name: 'end_date', type: 'DateTime' }, + ], + layout: 'RANGE_HASHED()', + range: { min: 'start_date', max: 'end_date' }, + settings: { dictionary_use_async_executor: 1, max_threads: 8 }, + }) + const sql = toCreateSQL(dict) + expect(sql).toContain('`parent_id` UInt64 HIERARCHICAL BIDIRECTIONAL') + expect(sql).toContain('RANGE(MIN `start_date` MAX `end_date`)') + expect(sql).toContain('SETTINGS(dictionary_use_async_executor = 1, max_threads = 8)') + }) + + test('validation: range references missing attribute', () => { + const issues = validateDefinitions([ + dictionary({ ...baseDictionary, range: { min: 'not_an_attribute', max: 'name' } }), + ]) + expect(issues.map((i) => i.code)).toContain('dictionary_range_missing_attribute') + }) + + test('validation: bidirectional requires hierarchical', () => { + const issues = validateDefinitions([ + dictionary({ + ...baseDictionary, + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'parent_id', type: 'UInt64', bidirectional: true }, + ], + }), + ]) + expect(issues.map((i) => i.code)).toContain('dictionary_bidirectional_requires_hierarchical') + }) + + test('binary diff: adding settings produces a single CREATE OR REPLACE DICTIONARY op', () => { + const oldDefs = [dictionary({ ...baseDictionary })] + const newDefs = [dictionary({ ...baseDictionary, settings: { max_threads: 4 } })] + const plan = planDiff(oldDefs, newDefs) + expect(plan.operations).toHaveLength(1) + expect(plan.operations[0]?.type).toBe('create_dictionary') + expect(plan.operations[0]?.sql).toContain('SETTINGS(max_threads = 4)') + }) + test('create ordering: create_dictionary ranks after create_table', () => { const usersTable = table({ database: 'app', diff --git a/packages/core/src/model-types.ts b/packages/core/src/model-types.ts index 49043065..6ee7c4af 100644 --- a/packages/core/src/model-types.ts +++ b/packages/core/src/model-types.ts @@ -164,6 +164,8 @@ export interface DictionaryAttribute { /** EXPRESSION — computed from source columns. Mutually exclusive with default. */ expression?: string hierarchical?: boolean + /** Enables bidirectional parent/child lookups. Only valid alongside hierarchical. */ + bidirectional?: boolean injective?: boolean isObjectId?: boolean } @@ -181,6 +183,10 @@ export interface DictionaryDefinition { layout: string /** Raw LIFETIME(...) body, e.g. `300` / `MIN 300 MAX 360`. */ lifetime: string + /** RANGE(MIN ... MAX ...) — required by RANGE_HASHED / COMPLEX_KEY_RANGE_HASHED layouts. */ + range?: { min: string; max: string } + /** Raw SETTINGS(...) key/value pairs. */ + settings?: Record comment?: string } @@ -360,6 +366,8 @@ export type ValidationIssueCode = | 'dictionary_missing_layout' | 'dictionary_missing_lifetime' | 'dictionary_attribute_default_expression_exclusive' + | 'dictionary_range_missing_attribute' + | 'dictionary_bidirectional_requires_hierarchical' export interface ValidationIssue { code: ValidationIssueCode diff --git a/packages/core/src/sql-validation.e2e.test.ts b/packages/core/src/sql-validation.e2e.test.ts index 92038af6..5eb8fd9a 100644 --- a/packages/core/src/sql-validation.e2e.test.ts +++ b/packages/core/src/sql-validation.e2e.test.ts @@ -938,6 +938,34 @@ ORDER BY (\`id\`, toDate(\`created_at\`))` const def = dictionary({ ...baseDictionary }) await assertValidSQL(client, renderDictionarySQL(def, true)) }) + + test('dictionary with RANGE_HASHED layout, RANGE, and SETTINGS', async () => { + const def = dictionary({ + ...baseDictionary, + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'start_date', type: 'Date' }, + { name: 'end_date', type: 'Date' }, + { name: 'value', type: 'String' }, + ], + layout: 'RANGE_HASHED()', + range: { min: 'start_date', max: 'end_date' }, + settings: { dictionary_use_async_executor: 1, max_threads: 4 }, + }) + await assertValidSQL(client, toCreateSQL(def)) + }) + + test('dictionary with a BIDIRECTIONAL hierarchical attribute', async () => { + const def = dictionary({ + ...baseDictionary, + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'parent_id', type: 'UInt64', hierarchical: true, bidirectional: true }, + { name: 'name', type: 'String' }, + ], + }) + await assertValidSQL(client, toCreateSQL(def)) + }) }) // ========================================================================= diff --git a/packages/core/src/sql.ts b/packages/core/src/sql.ts index 867a131e..32dcb9fb 100644 --- a/packages/core/src/sql.ts +++ b/packages/core/src/sql.ts @@ -152,21 +152,41 @@ function renderDictionaryAttribute(attr: DictionaryAttribute): string { if (attr.expression !== undefined) out += ` EXPRESSION ${attr.expression}` else if (attr.default !== undefined) out += ` DEFAULT ${renderDefault(attr.default)}` if (attr.hierarchical) out += ' HIERARCHICAL' + if (attr.bidirectional) out += ' BIDIRECTIONAL' if (attr.injective) out += ' INJECTIVE' if (attr.isObjectId) out += ' IS_OBJECT_ID' return out } +function renderDictionaryRangeColumn(column: string, columnNames: Set): string { + return columnNames.has(column) || isPlainColumnReference(column) ? `\`${column}\`` : column +} + +function renderDictionarySettings(settings: Record): string { + return Object.entries(settings) + .map(([k, v]) => `${k} = ${typeof v === 'string' ? `'${v.replace(/'/g, "''")}'` : v}`) + .join(', ') +} + export function renderDictionarySQL(def: DictionaryDefinition, replace = false): string { const verb = replace ? 'CREATE OR REPLACE DICTIONARY' : 'CREATE DICTIONARY IF NOT EXISTS' const attrs = def.attributes.map(renderDictionaryAttribute).join(',\n ') - const pk = renderKeyClauseColumns(def.primaryKey, new Set(def.attributes.map((a) => a.name))) + const columnNames = new Set(def.attributes.map((a) => a.name)) + const pk = renderKeyClauseColumns(def.primaryKey, columnNames) const clauses = [ `PRIMARY KEY ${pk}`, `SOURCE(${def.source})`, `LAYOUT(${def.layout})`, `LIFETIME(${def.lifetime})`, ] + if (def.range) { + clauses.push( + `RANGE(MIN ${renderDictionaryRangeColumn(def.range.min, columnNames)} MAX ${renderDictionaryRangeColumn(def.range.max, columnNames)})` + ) + } + if (def.settings && Object.keys(def.settings).length > 0) { + clauses.push(`SETTINGS(${renderDictionarySettings(def.settings)})`) + } if (def.comment) clauses.push(`COMMENT '${def.comment.replace(/'/g, "''")}'`) return `${verb} ${def.database}.${def.name}\n(\n ${attrs}\n)\n${clauses.join('\n')};` } diff --git a/packages/core/src/validate.ts b/packages/core/src/validate.ts index 67afcabb..4e68a636 100644 --- a/packages/core/src/validate.ts +++ b/packages/core/src/validate.ts @@ -247,6 +247,15 @@ function validateDictionaryDefinition(def: DictionaryDefinition, issues: Validat `Dictionary ${def.database}.${def.name} attribute "${attribute.name}" sets both "default" and "expression"; choose one` ) } + + if (attribute.bidirectional && !attribute.hierarchical) { + pushValidationIssue( + issues, + def, + 'dictionary_bidirectional_requires_hierarchical', + `Dictionary ${def.database}.${def.name} attribute "${attribute.name}" sets "bidirectional" without "hierarchical"; bidirectional only applies to hierarchical attributes` + ) + } } if (def.primaryKey.length === 0) { @@ -295,6 +304,19 @@ function validateDictionaryDefinition(def: DictionaryDefinition, issues: Validat `Dictionary ${def.database}.${def.name} requires a non-empty "lifetime"` ) } + + if (def.range) { + for (const column of [def.range.min, def.range.max]) { + if (!attributeSet.has(column)) { + pushValidationIssue( + issues, + def, + 'dictionary_range_missing_attribute', + `Dictionary ${def.database}.${def.name} range references missing attribute "${column}"` + ) + } + } + } } export function validateDefinitions(definitions: SchemaDefinition[]): ValidationIssue[] { diff --git a/packages/plugin-pull/src/index.test.ts b/packages/plugin-pull/src/index.test.ts index 2d0a3fad..9807d267 100644 --- a/packages/plugin-pull/src/index.test.ts +++ b/packages/plugin-pull/src/index.test.ts @@ -295,6 +295,34 @@ export default schema(app_events, app_events_view, analytics_daily_mv) expect(content).toContain('export default schema(app_users_dict)') }) + test('renders a dictionary with range, settings, and a bidirectional attribute', () => { + const content = renderSchemaFile([ + { + kind: 'dictionary', + database: 'app', + name: 'rates_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'parent_id', type: 'UInt64', hierarchical: true, bidirectional: true }, + { name: 'start_date', type: 'DateTime' }, + { name: 'end_date', type: 'DateTime' }, + ], + primaryKey: ['id'], + source: "HTTP(url 'http://example.com/rates' format 'TSV')", + layout: 'RANGE_HASHED()', + lifetime: '300', + range: { min: 'start_date', max: 'end_date' }, + settings: { dictionary_use_async_executor: 1, max_threads: 8 }, + }, + ]) + + expect(content).toContain('{ name: "parent_id", type: "UInt64", hierarchical: true, bidirectional: true }') + expect(content).toContain('range: { min: "start_date", max: "end_date" }') + expect(content).toContain('settings: {') + expect(content).toContain('dictionary_use_async_executor: 1') + expect(content).toContain('max_threads: 8') + }) + test('renders refreshable materialized view with full refresh block', () => { const content = renderSchemaFile([ { @@ -700,6 +728,42 @@ LIFETIME(MIN 0 MAX 300)`, }) }) + test('mapSystemTableRowToDefinition parses a dictionary row with RANGE and SETTINGS', () => { + const definition = __testUtils.mapSystemTableRowToDefinition({ + database: 'app', + name: 'rates_dict', + engine: 'Dictionary', + create_table_query: `CREATE DICTIONARY app.rates_dict +( + \`id\` UInt64, + \`start_date\` DateTime, + \`end_date\` DateTime +) +PRIMARY KEY id +SOURCE(HTTP(url 'http://example.com/rates' format 'TSV')) +LIFETIME(MIN 0 MAX 300) +LAYOUT(RANGE_HASHED()) +RANGE(MIN start_date MAX end_date) +SETTINGS(dictionary_use_async_executor = 1)`, + }) + expect(definition).toEqual({ + kind: 'dictionary', + database: 'app', + name: 'rates_dict', + attributes: [ + { name: 'id', type: 'UInt64' }, + { name: 'start_date', type: 'DateTime' }, + { name: 'end_date', type: 'DateTime' }, + ], + primaryKey: ['id'], + source: "HTTP(url 'http://example.com/rates' format 'TSV')", + layout: 'RANGE_HASHED()', + lifetime: 'MIN 0 MAX 300', + range: { min: 'start_date', max: 'end_date' }, + settings: { dictionary_use_async_executor: 1 }, + }) + }) + test('mapSystemTableRowToDefinition returns null for a dictionary with an unparsable query', () => { const definition = __testUtils.mapSystemTableRowToDefinition({ database: 'app', diff --git a/packages/plugin-pull/src/index.ts b/packages/plugin-pull/src/index.ts index 5f0f2e82..c61ab7c9 100644 --- a/packages/plugin-pull/src/index.ts +++ b/packages/plugin-pull/src/index.ts @@ -360,6 +360,8 @@ function mapIntrospectedObjectToDefinition(introspected: IntrospectedObject): Sc source: introspected.source, layout: introspected.layout, lifetime: introspected.lifetime, + ...(introspected.range ? { range: introspected.range } : {}), + ...(introspected.settings ? { settings: introspected.settings } : {}), ...(introspected.comment ? { comment: introspected.comment } : {}), } } diff --git a/packages/plugin-pull/src/render-schema.ts b/packages/plugin-pull/src/render-schema.ts index 974e865b..3c3085f1 100644 --- a/packages/plugin-pull/src/render-schema.ts +++ b/packages/plugin-pull/src/render-schema.ts @@ -141,6 +141,14 @@ function renderDictionaryDefinition(definition: DictionaryDefinition, variableNa lines.push(` source: ${renderString(definition.source)},`) lines.push(` layout: ${renderString(definition.layout)},`) lines.push(` lifetime: ${renderString(definition.lifetime)},`) + if (definition.range) { + lines.push( + ` range: { min: ${renderString(definition.range.min)}, max: ${renderString(definition.range.max)} },` + ) + } + if (definition.settings && Object.keys(definition.settings).length > 0) { + lines.push(...renderSettingsLines(definition.settings, ' ')) + } if (definition.comment) { lines.push(` comment: ${renderString(definition.comment)},`) } @@ -156,6 +164,7 @@ function renderDictionaryAttribute(attribute: DictionaryAttribute): string { parts.push(`default: ${renderLiteral(attribute.default)}`) } if (attribute.hierarchical) parts.push('hierarchical: true') + if (attribute.bidirectional) parts.push('bidirectional: true') if (attribute.injective) parts.push('injective: true') if (attribute.isObjectId) parts.push('isObjectId: true') return `{ ${parts.join(', ')} }` diff --git a/packages/plugin-pull/src/view-parser.ts b/packages/plugin-pull/src/view-parser.ts index 554d4841..3fb16c0f 100644 --- a/packages/plugin-pull/src/view-parser.ts +++ b/packages/plugin-pull/src/view-parser.ts @@ -3,6 +3,8 @@ import { parseCommentFromCreateDictionaryQuery, parseDictionaryAttributesFromCreateDictionaryQuery, parseDictionaryPrimaryKeyFromCreateDictionaryQuery, + parseDictionaryRangeFromCreateDictionaryQuery, + parseDictionarySettingsFromCreateDictionaryQuery, parseLayoutFromCreateDictionaryQuery, parseLifetimeFromCreateDictionaryQuery, parseSourceFromCreateDictionaryQuery, @@ -31,7 +33,16 @@ export type IntrospectedObject = >) | ({ kind: 'dictionary' } & Pick< DictionaryDefinition, - 'database' | 'name' | 'attributes' | 'primaryKey' | 'source' | 'layout' | 'lifetime' | 'comment' + | 'database' + | 'name' + | 'attributes' + | 'primaryKey' + | 'source' + | 'layout' + | 'lifetime' + | 'range' + | 'settings' + | 'comment' >) | IntrospectedTable @@ -48,6 +59,8 @@ function mapDictionaryRowToDefinition( return null } const comment = parseCommentFromCreateDictionaryQuery(query) + const range = parseDictionaryRangeFromCreateDictionaryQuery(query) + const settings = parseDictionarySettingsFromCreateDictionaryQuery(query) return { kind: 'dictionary', database: row.database, @@ -57,6 +70,8 @@ function mapDictionaryRowToDefinition( source, layout, lifetime, + ...(range ? { range } : {}), + ...(settings ? { settings } : {}), ...(comment ? { comment } : {}), } } From c1aa3c3cd83fd62a40fbd19b21c0cb7abb79fc3d Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Wed, 15 Jul 2026 14:04:20 +0200 Subject: [PATCH 08/15] feat(core,cli): rename dictionaries via RENAME DICTIONARY, fix ON CLUSTER on replace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dictionary renamedFrom was dead API surface — nothing read it, so a renamed dictionary always diffed as drop_dictionary + create_dictionary. Wired it into the same schema/CLI rename-mapping pipeline tables already use (remap-before-diff + post-hoc operation injection), added a matching --rename-dictionary flag, and confirmed RENAME DICTIONARY IF EXISTS is accepted and idempotent against a live ClickHouse instance. Also fixes a related gap found in the process: ON CLUSTER injection matched on the literal "CREATE DICTIONARY" prefix, so it silently never applied to CREATE OR REPLACE DICTIONARY — meaning every structural change to a dictionary (there is no ALTER DICTIONARY) went unreplicated in cluster mode. Co-Authored-By: Claude Sonnet 5 --- apps/docs/src/content/docs/cli/generate.md | 13 +- .../src/content/docs/schema/dsl-reference.md | 19 +- packages/cli/src/commands/generate/command.ts | 23 +- .../src/commands/generate/plan-pipeline.ts | 55 ++++- .../src/commands/generate/rename-mappings.ts | 201 ++++++++++++++++-- packages/cli/src/test/generate.e2e.test.ts | 69 ++++++ packages/core/src/model-types.ts | 1 + packages/core/src/on-cluster.test.ts | 18 ++ packages/core/src/on-cluster.ts | 17 +- 9 files changed, 382 insertions(+), 34 deletions(-) diff --git a/apps/docs/src/content/docs/cli/generate.md b/apps/docs/src/content/docs/cli/generate.md index 36b190b8..a8d549f9 100644 --- a/apps/docs/src/content/docs/cli/generate.md +++ b/apps/docs/src/content/docs/cli/generate.md @@ -21,6 +21,7 @@ chkit generate [flags] | `--migration-id ` | string | — | Escape hatch: override the default timestamp migration prefix | | `--rename-table ` | string | — | Explicit table rename: `old_db.old_table=new_db.new_table` | | `--rename-column ` | string | — | Explicit column rename: `db.table.old_column=new_column` | +| `--rename-dictionary ` | string | — | Explicit dictionary rename: `old_db.old_dict=new_db.new_dict` | | `--table ` | string | — | Scope operations to matching tables | | `--dryrun` | boolean | `false` | Print the plan without writing any files | | `--empty` | boolean | `false` | Scaffold a blank manual migration without diffing the schema | @@ -63,10 +64,12 @@ An empty match set emits a warning and produces no output. chkit detects potential renames through two mechanisms: 1. **Schema metadata** — set `renamedFrom` on your schema definition -2. **CLI flags** — `--rename-table old_db.old_table=new_db.new_table` and `--rename-column db.table.old_col=new_col` +2. **CLI flags** — `--rename-table old_db.old_table=new_db.new_table`, `--rename-column db.table.old_col=new_col`, and `--rename-dictionary old_db.old_dict=new_db.new_dict` CLI flags take priority when both sources specify a mapping for the same object. Rename flags accept comma-separated values for multiple mappings. +A dictionary rename emits a single `RENAME DICTIONARY IF EXISTS ... TO ...` statement instead of a `drop_dictionary` + `create_dictionary` pair — see [Dictionary rename](/schema/dsl-reference/#dictionary-rename). + Validation errors are raised for conflicting, chained, or cyclic rename mappings. ### Dryrun mode @@ -79,7 +82,7 @@ Plans for objects in a database lead with a `create_database` operation (`CREATE With `--empty`, the command skips the schema diff entirely and writes a blank, timestamped migration stub for you to hand-edit. Use it for DDL that chkit does not model — raw `INSERT`/backfill statements, `OPTIMIZE`, manual dictionary reloads, or one-off data fixes. -The stub carries the standard migration header (with `operation-count: 0`) plus a placeholder comment. The snapshot is left untouched, so an empty migration never absorbs pending schema drift. The `--name` and `--migration-id` flags apply; without `--name`, the file defaults to `manual`. Schema-diff flags (`--table`, `--rename-table`, `--rename-column`, `--dryrun`) are not used in empty mode. +The stub carries the standard migration header (with `operation-count: 0`) plus a placeholder comment. The snapshot is left untouched, so an empty migration never absorbs pending schema drift. The `--name` and `--migration-id` flags apply; without `--name`, the file defaults to `manual`. Schema-diff flags (`--table`, `--rename-table`, `--rename-column`, `--rename-dictionary`, `--dryrun`) are not used in empty mode. `chkit migrate` picks the file up like any other migration and applies it in filename order. Write your SQL into the stub *before* applying it — editing a migration after it has run triggers a checksum mismatch. @@ -129,6 +132,12 @@ chkit generate --rename-table old_db.users=new_db.accounts chkit generate --rename-column analytics.events.old_name=new_name ``` +**Explicit dictionary rename:** + +```sh +chkit generate --rename-dictionary old_db.old_dict=new_db.new_dict +``` + ## Exit codes | Code | Meaning | diff --git a/apps/docs/src/content/docs/schema/dsl-reference.md b/apps/docs/src/content/docs/schema/dsl-reference.md index 7ceb4568..845e1bec 100644 --- a/apps/docs/src/content/docs/schema/dsl-reference.md +++ b/apps/docs/src/content/docs/schema/dsl-reference.md @@ -432,7 +432,7 @@ ClickHouse redacts inline passwords back to `[HIDDEN]` on introspection (`SHOW C ### No `ALTER DICTIONARY` -ClickHouse has no `ALTER DICTIONARY` — every structural change to a dictionary is rendered as a single `CREATE OR REPLACE DICTIONARY` statement (atomic, dependency-safe). See [Structural vs. alterable properties](#structural-vs-alterable-properties). +ClickHouse has no `ALTER DICTIONARY` — every structural change to a dictionary is rendered as a single `CREATE OR REPLACE DICTIONARY` statement (atomic, dependency-safe). See [Structural vs. alterable properties](#structural-vs-alterable-properties). A pure rename (`renamedFrom` with no other change) is the one exception — it renders as `RENAME DICTIONARY`, not a replace; see [Dictionary rename](#dictionary-rename). ## Type system reference @@ -483,7 +483,22 @@ columns: [ ] ``` -Both table and column renames can be overridden by CLI flags `--rename-table` and `--rename-column`. +### Dictionary rename + +Set `renamedFrom` on a dictionary definition to rename a dictionary. This emits a single `RENAME DICTIONARY IF EXISTS ... TO ...` statement instead of a `drop_dictionary` + `create_dictionary` pair: + +```ts +const lookupDict = dictionary({ + database: 'app', + name: 'lookup_dict', // new name + renamedFrom: { name: 'users_dict' }, // old name + // ... +}) +``` + +The `database` field in `renamedFrom` is optional and defaults to the dictionary's current database. + +Table, column, and dictionary renames can all be overridden by CLI flags: `--rename-table`, `--rename-column`, and `--rename-dictionary`. ## Plugin configuration diff --git a/packages/cli/src/commands/generate/command.ts b/packages/cli/src/commands/generate/command.ts index 270e66f4..9e633fa2 100644 --- a/packages/cli/src/commands/generate/command.ts +++ b/packages/cli/src/commands/generate/command.ts @@ -15,21 +15,28 @@ import { tableKeysFromDefinitions, } from '../../runtime/table-scope.js' import { + applyExplicitDictionaryRenames, applyExplicitTableRenames, applySelectedRenameSuggestions, assertCliColumnMappingsResolvable, buildExplicitColumnRenameSuggestions, } from './plan-pipeline.js' import { + assertCliDictionaryMappingsResolvable, assertCliTableMappingsResolvable, assertNoConflictingColumnMappings, + assertNoConflictingDictionaryMappings, assertNoConflictingTableMappings, collectSchemaRenameMappings, mergeColumnMappings, + mergeDictionaryMappings, mergeTableMappings, parseRenameColumnMappings, + parseRenameDictionaryMappings, parseRenameTableMappings, + remapOldDefinitionsForDictionaryRenames, remapOldDefinitionsForTableRenames, + resolveActiveDictionaryMappings, resolveActiveTableMappings, } from './rename-mappings.js' import { emitGenerateApplyOutput, emitGenerateEmptyOutput, emitGeneratePlanOutput } from './output.js' @@ -40,6 +47,7 @@ const GENERATE_FLAGS = defineFlags([ { name: '--migration-id', type: 'string', description: 'Override the default timestamp migration prefix', placeholder: '' }, { name: '--rename-table', type: 'string[]', description: 'Explicit table rename mapping', placeholder: '' }, { name: '--rename-column', type: 'string[]', description: 'Explicit column rename mapping', placeholder: '' }, + { name: '--rename-dictionary', type: 'string[]', description: 'Explicit dictionary rename mapping', placeholder: '' }, { name: '--dryrun', type: 'boolean', description: 'Print plan without writing artifacts' }, { name: '--empty', type: 'boolean', description: 'Scaffold a blank manual migration (no schema diff, snapshot untouched)' }, ] as const) @@ -99,11 +107,14 @@ async function cmdGenerate(ctx: import('../../plugins.js').ChxPluginCommandConte const renameTableValues = f['--rename-table'] ?? [] const renameColumnValues = f['--rename-column'] ?? [] + const renameDictionaryValues = f['--rename-dictionary'] ?? [] const cliTableMappings = parseRenameTableMappings(renameTableValues) const cliColumnMappings = parseRenameColumnMappings(renameColumnValues) + const cliDictionaryMappings = parseRenameDictionaryMappings(renameDictionaryValues) const schemaMappings = collectSchemaRenameMappings(definitions) const tableMappings = mergeTableMappings(schemaMappings.tableMappings, cliTableMappings) const columnMappings = mergeColumnMappings(schemaMappings.columnMappings, cliColumnMappings) + const dictionaryMappings = mergeDictionaryMappings(schemaMappings.dictionaryMappings, cliDictionaryMappings) const { migrationsDir, metaDir } = dirs const previousDefinitions = (await readSnapshot(metaDir))?.definitions ?? [] @@ -131,12 +142,19 @@ async function cmdGenerate(ctx: import('../../plugins.js').ChxPluginCommandConte assertNoConflictingTableMappings(tableMappings) assertNoConflictingColumnMappings(columnMappings) + assertNoConflictingDictionaryMappings(dictionaryMappings) assertCliTableMappingsResolvable(cliTableMappings, previousDefinitions, definitions) + assertCliDictionaryMappingsResolvable(cliDictionaryMappings, previousDefinitions, definitions) const activeTableMappings = resolveActiveTableMappings(previousDefinitions, definitions, tableMappings) - const remappedPreviousDefinitions = remapOldDefinitionsForTableRenames( + const activeDictionaryMappings = resolveActiveDictionaryMappings( previousDefinitions, - activeTableMappings + definitions, + dictionaryMappings + ) + const remappedPreviousDefinitions = remapOldDefinitionsForDictionaryRenames( + remapOldDefinitionsForTableRenames(previousDefinitions, activeTableMappings), + activeDictionaryMappings ) debug('generate', `previous snapshot: ${previousDefinitions.length} definitions, current: ${definitions.length} definitions`) @@ -161,6 +179,7 @@ async function cmdGenerate(ctx: import('../../plugins.js').ChxPluginCommandConte } plan = applyExplicitTableRenames(plan, activeTableMappings) + plan = applyExplicitDictionaryRenames(plan, activeDictionaryMappings) assertCliColumnMappingsResolvable(cliColumnMappings, plan, definitions) plan = applySelectedRenameSuggestions(plan, buildExplicitColumnRenameSuggestions(plan, columnMappings)) diff --git a/packages/cli/src/commands/generate/plan-pipeline.ts b/packages/cli/src/commands/generate/plan-pipeline.ts index c98b3090..56f8a660 100644 --- a/packages/cli/src/commands/generate/plan-pipeline.ts +++ b/packages/cli/src/commands/generate/plan-pipeline.ts @@ -6,7 +6,7 @@ import type { SchemaDefinition, } from '@chkit/core' -import type { ColumnRenameMapping, TableRenameMapping } from './rename-mappings.js' +import type { ColumnRenameMapping, DictionaryRenameMapping, TableRenameMapping } from './rename-mappings.js' export function applySelectedRenameSuggestions( plan: MigrationPlan, @@ -104,6 +104,57 @@ export function applyExplicitTableRenames( } } +export function applyExplicitDictionaryRenames( + plan: MigrationPlan, + mappings: DictionaryRenameMapping[] +): MigrationPlan { + if (mappings.length === 0) return plan + + const operationKeysToRemove = new Set() + const extraOperations: MigrationOperation[] = [] + const createDatabaseOps = new Set( + plan.operations.filter((operation) => operation.type === 'create_database').map((operation) => operation.key) + ) + + for (const mapping of mappings) { + operationKeysToRemove.add(`dictionary:${mapping.oldDatabase}.${mapping.oldName}`) + operationKeysToRemove.add(`dictionary:${mapping.newDatabase}.${mapping.newName}`) + if (mapping.oldDatabase !== mapping.newDatabase) { + const dbKey = `database:${mapping.newDatabase}` + if (!createDatabaseOps.has(dbKey)) { + extraOperations.push({ + type: 'create_database', + key: dbKey, + risk: 'safe', + sql: `CREATE DATABASE IF NOT EXISTS ${mapping.newDatabase};`, + }) + createDatabaseOps.add(dbKey) + } + } + extraOperations.push({ + type: 'rename_dictionary', + key: `dictionary:${mapping.newDatabase}.${mapping.newName}:rename_dictionary`, + risk: 'caution', + sql: `RENAME DICTIONARY IF EXISTS ${mapping.oldDatabase}.${mapping.oldName} TO ${mapping.newDatabase}.${mapping.newName};`, + }) + } + + const operations = [ + ...plan.operations.filter((operation) => !operationKeysToRemove.has(operation.key)), + ...extraOperations, + ].sort((a, b) => { + const rankOrder = rankOperation(a) - rankOperation(b) + if (rankOrder !== 0) return rankOrder + return a.key.localeCompare(b.key) + }) + + return { + operations, + riskSummary: summarizeRisk(operations), + renameSuggestions: plan.renameSuggestions, + } +} + export function buildExplicitColumnRenameSuggestions( plan: MigrationPlan, mappings: ColumnRenameMapping[] @@ -167,7 +218,7 @@ export function assertCliColumnMappingsResolvable( function rankOperation(op: MigrationOperation): number { if (op.type.startsWith('drop_')) return 0 if (op.type === 'create_database') return 1 - if (op.type === 'alter_table_rename_table') return 2 + if (op.type === 'alter_table_rename_table' || op.type === 'rename_dictionary') return 2 if (op.type.startsWith('alter_')) return 3 if (op.type === 'create_table') return 4 if (op.type === 'create_view') return 5 diff --git a/packages/cli/src/commands/generate/rename-mappings.ts b/packages/cli/src/commands/generate/rename-mappings.ts index 45fa4ec7..04f6883e 100644 --- a/packages/cli/src/commands/generate/rename-mappings.ts +++ b/packages/cli/src/commands/generate/rename-mappings.ts @@ -1,4 +1,4 @@ -import type { SchemaDefinition, TableDefinition } from '@chkit/core' +import type { DictionaryDefinition, SchemaDefinition, TableDefinition } from '@chkit/core' export interface TableRenameMapping { oldDatabase: string @@ -8,6 +8,14 @@ export interface TableRenameMapping { source: 'cli' | 'schema' } +export interface DictionaryRenameMapping { + oldDatabase: string + oldName: string + newDatabase: string + newName: string + source: 'cli' | 'schema' +} + export interface ColumnRenameMapping { database: string table: string @@ -24,8 +32,28 @@ export function parseRenameTableMappings(values: string[]): TableRenameMapping[] `Invalid --rename-table mapping "${mapping}". Expected format: old_db.old_table=new_db.new_table` ) } - const from = parseQualifiedTable(fromRaw) - const to = parseQualifiedTable(toRaw) + const from = parseQualifiedName(fromRaw) + const to = parseQualifiedName(toRaw) + return { + oldDatabase: from.database, + oldName: from.name, + newDatabase: to.database, + newName: to.name, + source: 'cli', + } + }) +} + +export function parseRenameDictionaryMappings(values: string[]): DictionaryRenameMapping[] { + return values.map((mapping) => { + const [fromRaw, toRaw, ...rest] = mapping.split('=').map((part) => part.trim()) + if (!fromRaw || !toRaw || rest.length > 0) { + throw new Error( + `Invalid --rename-dictionary mapping "${mapping}". Expected format: old_db.old_dict=new_db.new_dict` + ) + } + const from = parseQualifiedName(fromRaw) + const to = parseQualifiedName(toRaw) return { oldDatabase: from.database, oldName: from.name, @@ -63,14 +91,38 @@ export function parseRenameColumnMappings(values: string[]): ColumnRenameMapping export function collectSchemaRenameMappings( definitions: SchemaDefinition[] -): { tableMappings: TableRenameMapping[]; columnMappings: ColumnRenameMapping[] } { +): { + tableMappings: TableRenameMapping[] + columnMappings: ColumnRenameMapping[] + dictionaryMappings: DictionaryRenameMapping[] +} { const tableMappings: TableRenameMapping[] = [] const columnMappings: ColumnRenameMapping[] = [] + const dictionaryMappings: DictionaryRenameMapping[] = [] for (const definition of definitions) { - if (definition.kind !== 'table') continue - if (definition.renamedFrom) { - tableMappings.push({ + if (definition.kind === 'table') { + if (definition.renamedFrom) { + tableMappings.push({ + oldDatabase: definition.renamedFrom.database ?? definition.database, + oldName: definition.renamedFrom.name, + newDatabase: definition.database, + newName: definition.name, + source: 'schema', + }) + } + for (const column of definition.columns) { + if (!column.renamedFrom) continue + columnMappings.push({ + database: definition.database, + table: definition.name, + from: column.renamedFrom, + to: column.name, + source: 'schema', + }) + } + } else if (definition.kind === 'dictionary' && definition.renamedFrom) { + dictionaryMappings.push({ oldDatabase: definition.renamedFrom.database ?? definition.database, oldName: definition.renamedFrom.name, newDatabase: definition.database, @@ -78,19 +130,9 @@ export function collectSchemaRenameMappings( source: 'schema', }) } - for (const column of definition.columns) { - if (!column.renamedFrom) continue - columnMappings.push({ - database: definition.database, - table: definition.name, - from: column.renamedFrom, - to: column.name, - source: 'schema', - }) - } } - return { tableMappings, columnMappings } + return { tableMappings, columnMappings, dictionaryMappings } } export function mergeTableMappings( @@ -115,6 +157,28 @@ export function mergeTableMappings( return merged } +export function mergeDictionaryMappings( + schemaMappings: DictionaryRenameMapping[], + cliMappings: DictionaryRenameMapping[] +): DictionaryRenameMapping[] { + const merged = [...schemaMappings] + for (const cliMapping of cliMappings) { + const cliOldKey = `${cliMapping.oldDatabase}.${cliMapping.oldName}` + const cliNewKey = `${cliMapping.newDatabase}.${cliMapping.newName}` + for (let i = merged.length - 1; i >= 0; i -= 1) { + const entry = merged[i] + if (!entry) continue + const oldKey = `${entry.oldDatabase}.${entry.oldName}` + const newKey = `${entry.newDatabase}.${entry.newName}` + if (oldKey === cliOldKey || newKey === cliNewKey) { + merged.splice(i, 1) + } + } + merged.push(cliMapping) + } + return merged +} + export function mergeColumnMappings( schemaMappings: ColumnRenameMapping[], cliMappings: ColumnRenameMapping[] @@ -149,6 +213,18 @@ export function resolveActiveTableMappings( ) } +export function resolveActiveDictionaryMappings( + previousDefinitions: SchemaDefinition[], + nextDefinitions: SchemaDefinition[], + mappings: DictionaryRenameMapping[] +): DictionaryRenameMapping[] { + return mappings.filter( + (mapping) => + dictionaryExists(previousDefinitions, mapping.oldDatabase, mapping.oldName) && + dictionaryExists(nextDefinitions, mapping.newDatabase, mapping.newName) + ) +} + export function assertNoConflictingTableMappings(mappings: TableRenameMapping[]): void { const byOld = new Map() const byNew = new Map() @@ -178,6 +254,35 @@ export function assertNoConflictingTableMappings(mappings: TableRenameMapping[]) } } +export function assertNoConflictingDictionaryMappings(mappings: DictionaryRenameMapping[]): void { + const byOld = new Map() + const byNew = new Map() + + for (const mapping of mappings) { + const oldKey = `${mapping.oldDatabase}.${mapping.oldName}` + const newKey = `${mapping.newDatabase}.${mapping.newName}` + const existingOld = byOld.get(oldKey) + if (existingOld && (existingOld.newDatabase !== mapping.newDatabase || existingOld.newName !== mapping.newName)) { + throw new Error(`Conflicting dictionary rename source mapping for "${oldKey}".`) + } + byOld.set(oldKey, mapping) + + const existingNew = byNew.get(newKey) + if (existingNew && (existingNew.oldDatabase !== mapping.oldDatabase || existingNew.oldName !== mapping.oldName)) { + throw new Error(`Conflicting dictionary rename target mapping for "${newKey}".`) + } + byNew.set(newKey, mapping) + } + + for (const key of byOld.keys()) { + if (byNew.has(key)) { + throw new Error( + `Unsupported chained or cyclic dictionary rename mapping involving "${key}". Use direct one-step mappings only.` + ) + } + } +} + export function assertNoConflictingColumnMappings(mappings: ColumnRenameMapping[]): void { const byFrom = new Map() const byTo = new Map() @@ -224,6 +329,31 @@ export function assertCliTableMappingsResolvable( } } +export function assertCliDictionaryMappingsResolvable( + cliMappings: DictionaryRenameMapping[], + previousDefinitions: SchemaDefinition[], + nextDefinitions: SchemaDefinition[] +): void { + for (const mapping of cliMappings) { + const hasOld = dictionaryExists(previousDefinitions, mapping.oldDatabase, mapping.oldName) + const hasNew = dictionaryExists(nextDefinitions, mapping.newDatabase, mapping.newName) + if (hasOld && hasNew) continue + if (!hasOld && !hasNew) { + throw new Error( + `--rename-dictionary mapping "${mapping.oldDatabase}.${mapping.oldName}=${mapping.newDatabase}.${mapping.newName}" is invalid: source dictionary is missing from previous snapshot and target dictionary is missing from current schema.` + ) + } + if (!hasOld) { + throw new Error( + `--rename-dictionary mapping "${mapping.oldDatabase}.${mapping.oldName}=${mapping.newDatabase}.${mapping.newName}" is invalid: source dictionary is missing from previous snapshot.` + ) + } + throw new Error( + `--rename-dictionary mapping "${mapping.oldDatabase}.${mapping.oldName}=${mapping.newDatabase}.${mapping.newName}" is invalid: target dictionary is missing from current schema.` + ) + } +} + export function remapOldDefinitionsForTableRenames( previousDefinitions: SchemaDefinition[], mappings: TableRenameMapping[] @@ -248,10 +378,34 @@ export function remapOldDefinitionsForTableRenames( }) } -function parseQualifiedTable(input: string): { database: string; name: string } { +export function remapOldDefinitionsForDictionaryRenames( + previousDefinitions: SchemaDefinition[], + mappings: DictionaryRenameMapping[] +): SchemaDefinition[] { + if (mappings.length === 0) return previousDefinitions + + const mappingByOld = new Map() + for (const mapping of mappings) { + mappingByOld.set(`${mapping.oldDatabase}.${mapping.oldName}`, mapping) + } + + return previousDefinitions.map((definition) => { + if (definition.kind !== 'dictionary') return definition + const mapping = mappingByOld.get(`${definition.database}.${definition.name}`) + if (!mapping) return definition + const remapped: DictionaryDefinition = { + ...definition, + database: mapping.newDatabase, + name: mapping.newName, + } + return remapped + }) +} + +function parseQualifiedName(input: string): { database: string; name: string } { const [database, name, ...rest] = input.split('.').map((part) => part.trim()) if (!database || !name || rest.length > 0) { - throw new Error(`Invalid table reference "${input}". Expected format: database.table`) + throw new Error(`Invalid object reference "${input}". Expected format: database.name`) } return { database, name } } @@ -261,3 +415,10 @@ function tableExists(definitions: SchemaDefinition[], database: string, name: st (definition) => definition.kind === 'table' && definition.database === database && definition.name === name ) } + +function dictionaryExists(definitions: SchemaDefinition[], database: string, name: string): boolean { + return definitions.some( + (definition) => + definition.kind === 'dictionary' && definition.database === database && definition.name === name + ) +} diff --git a/packages/cli/src/test/generate.e2e.test.ts b/packages/cli/src/test/generate.e2e.test.ts index b3baf4b8..8cfe5914 100644 --- a/packages/cli/src/test/generate.e2e.test.ts +++ b/packages/cli/src/test/generate.e2e.test.ts @@ -170,6 +170,75 @@ describe('@chkit/cli generate e2e', () => { } }) + test('generate --dryrun with --rename-dictionary emits RENAME DICTIONARY, not drop+create', async () => { + const fixture = await createFixture() + try { + await writeFile( + fixture.schemaPath, + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\nconst users = table({\n database: 'app',\n name: 'users',\n columns: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n engine: 'MergeTree()',\n primaryKey: ['id'],\n orderBy: ['id'],\n})\n\nconst usersDict = dictionary({\n database: 'app',\n name: 'users_dict',\n attributes: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n primaryKey: ['id'],\n source: "FILE(path '/dev/null' format 'CSV')",\n layout: 'FLAT()',\n lifetime: '300',\n})\n\nexport default schema(users, usersDict)\n`, + 'utf8' + ) + runCli(['generate', '--config', fixture.configPath, '--name', 'init', '--json']) + + await writeFile( + fixture.schemaPath, + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\nconst users = table({\n database: 'app',\n name: 'users',\n columns: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n engine: 'MergeTree()',\n primaryKey: ['id'],\n orderBy: ['id'],\n})\n\nconst lookupDict = dictionary({\n database: 'app',\n name: 'lookup_dict',\n attributes: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n primaryKey: ['id'],\n source: "FILE(path '/dev/null' format 'CSV')",\n layout: 'FLAT()',\n lifetime: '300',\n})\n\nexport default schema(users, lookupDict)\n`, + 'utf8' + ) + + const result = runCli([ + 'generate', + '--config', + fixture.configPath, + '--dryrun', + '--rename-dictionary', + 'app.users_dict=app.lookup_dict', + '--json', + ]) + expect(result.exitCode).toBe(0) + const payload = JSON.parse(result.stdout) as { + operations: Array<{ type: string; sql: string }> + } + expect(payload.operations.some((operation) => operation.type === 'rename_dictionary')).toBe(true) + expect( + payload.operations.find((operation) => operation.type === 'rename_dictionary')?.sql + ).toBe('RENAME DICTIONARY IF EXISTS app.users_dict TO app.lookup_dict;') + expect(payload.operations.some((operation) => operation.type === 'drop_dictionary')).toBe(false) + expect(payload.operations.some((operation) => operation.type === 'create_dictionary')).toBe(false) + } finally { + await rm(fixture.dir, { recursive: true, force: true }) + } + }) + + test('schema renamedFrom metadata emits explicit rename dictionary operation', async () => { + const fixture = await createFixture() + try { + await writeFile( + fixture.schemaPath, + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\nconst users = table({\n database: 'app',\n name: 'users',\n columns: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n engine: 'MergeTree()',\n primaryKey: ['id'],\n orderBy: ['id'],\n})\n\nconst usersDict = dictionary({\n database: 'app',\n name: 'users_dict',\n attributes: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n primaryKey: ['id'],\n source: "FILE(path '/dev/null' format 'CSV')",\n layout: 'FLAT()',\n lifetime: '300',\n})\n\nexport default schema(users, usersDict)\n`, + 'utf8' + ) + runCli(['generate', '--config', fixture.configPath, '--name', 'init', '--json']) + + await writeFile( + fixture.schemaPath, + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\nconst users = table({\n database: 'app',\n name: 'users',\n columns: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n engine: 'MergeTree()',\n primaryKey: ['id'],\n orderBy: ['id'],\n})\n\nconst lookupDict = dictionary({\n database: 'app',\n name: 'lookup_dict',\n renamedFrom: { name: 'users_dict' },\n attributes: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n primaryKey: ['id'],\n source: "FILE(path '/dev/null' format 'CSV')",\n layout: 'FLAT()',\n lifetime: '300',\n})\n\nexport default schema(users, lookupDict)\n`, + 'utf8' + ) + + const result = runCli(['generate', '--config', fixture.configPath, '--dryrun', '--json']) + expect(result.exitCode).toBe(0) + const payload = JSON.parse(result.stdout) as { + operations: Array<{ type: string }> + } + expect(payload.operations.some((operation) => operation.type === 'rename_dictionary')).toBe(true) + expect(payload.operations.some((operation) => operation.type === 'drop_dictionary')).toBe(false) + expect(payload.operations.some((operation) => operation.type === 'create_dictionary')).toBe(false) + } finally { + await rm(fixture.dir, { recursive: true, force: true }) + } + }) + test('cli --rename-table overrides conflicting schema renamedFrom metadata', async () => { const fixture = await createFixture() try { diff --git a/packages/core/src/model-types.ts b/packages/core/src/model-types.ts index 6ee7c4af..97b58b57 100644 --- a/packages/core/src/model-types.ts +++ b/packages/core/src/model-types.ts @@ -318,6 +318,7 @@ export type MigrationOperationType = | 'alter_table_modify_ttl' | 'create_dictionary' | 'drop_dictionary' + | 'rename_dictionary' export interface MigrationOperation { type: MigrationOperationType diff --git a/packages/core/src/on-cluster.test.ts b/packages/core/src/on-cluster.test.ts index 745cbc68..163771b1 100644 --- a/packages/core/src/on-cluster.test.ts +++ b/packages/core/src/on-cluster.test.ts @@ -94,6 +94,24 @@ describe('applyOnClusterToPlan', () => { expect(applyOnClusterToPlan(plan, 'c').operations[0]?.sql).toBe('INSERT INTO db.t SELECT 1;') }) + test('injects ON CLUSTER for dictionary create-or-replace, drop, and rename', () => { + const plan = planOf([ + op('create_dictionary', 'CREATE DICTIONARY IF NOT EXISTS db.d\n(\n `id` UInt64\n)\nPRIMARY KEY `id`\nSOURCE(NULL())\nLAYOUT(FLAT())\nLIFETIME(0);'), + op('create_dictionary', 'CREATE OR REPLACE DICTIONARY db.d\n(\n `id` UInt64\n)\nPRIMARY KEY `id`\nSOURCE(NULL())\nLAYOUT(FLAT())\nLIFETIME(0);'), + op('drop_dictionary', 'DROP DICTIONARY IF EXISTS db.d;'), + op('rename_dictionary', 'RENAME DICTIONARY IF EXISTS db.old TO db.new;'), + ]) + + const sql = applyOnClusterToPlan(plan, 'c').operations.map((o) => o.sql) + + expect(sql).toEqual([ + "CREATE DICTIONARY IF NOT EXISTS db.d ON CLUSTER 'c'\n(\n `id` UInt64\n)\nPRIMARY KEY `id`\nSOURCE(NULL())\nLAYOUT(FLAT())\nLIFETIME(0);", + "CREATE OR REPLACE DICTIONARY db.d ON CLUSTER 'c'\n(\n `id` UInt64\n)\nPRIMARY KEY `id`\nSOURCE(NULL())\nLAYOUT(FLAT())\nLIFETIME(0);", + "DROP DICTIONARY IF EXISTS db.d ON CLUSTER 'c';", + "RENAME DICTIONARY IF EXISTS db.old TO db.new ON CLUSTER 'c';", + ]) + }) + // These statement shapes are not emitted by chkit yet; the anchors exist as a // forward-compatible safety net so injection already works if a future command // starts producing them. Placements confirmed against the ClickHouse reference. diff --git a/packages/core/src/on-cluster.ts b/packages/core/src/on-cluster.ts index 9199ac9d..80e1b33e 100644 --- a/packages/core/src/on-cluster.ts +++ b/packages/core/src/on-cluster.ts @@ -21,14 +21,19 @@ const ON_CLUSTER_ANCHORS = [ 'CREATE VIEW', 'CREATE MATERIALIZED VIEW', 'CREATE DATABASE', + 'CREATE DICTIONARY', + // A structural dictionary change renders as `CREATE OR REPLACE DICTIONARY` + // (there is no `ALTER DICTIONARY`), which does NOT share the `CREATE + // DICTIONARY` prefix — it needs its own anchor or ON CLUSTER injection + // silently no-ops for every dictionary replace. + 'CREATE OR REPLACE DICTIONARY', 'ALTER TABLE', 'DROP TABLE', 'DROP VIEW', + 'DROP DICTIONARY', // --- Not emitted by chkit yet; kept as a forward-compatible safety net --- - 'CREATE DICTIONARY', 'CREATE FUNCTION', 'DROP DATABASE', - 'DROP DICTIONARY', 'ATTACH TABLE', 'DETACH TABLE', 'TRUNCATE TABLE', @@ -37,13 +42,13 @@ const ON_CLUSTER_ANCHORS = [ // Statements where `ON CLUSTER` goes at the very END, after the full object // list — not after the first name. RENAME and EXCHANGE take multiple object -// references (`a TO b`, `a AND b`), so the clause can only be appended. Only -// `RENAME TABLE` is emitted by chkit today; the rest are the same-family -// safety net described above. +// references (`a TO b`, `a AND b`), so the clause can only be appended. +// `RENAME TABLE` and `RENAME DICTIONARY` are emitted by chkit today; the rest +// are the same-family forward-compatible safety net described above. const ON_CLUSTER_TRAILING_ANCHORS = [ 'RENAME TABLE', - 'RENAME DATABASE', 'RENAME DICTIONARY', + 'RENAME DATABASE', 'EXCHANGE TABLES', 'EXCHANGE DICTIONARIES', ] as const From 0bd7409a175424ee5446e7ff95537b8f4878daa0 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Wed, 15 Jul 2026 14:06:43 +0200 Subject: [PATCH 09/15] chore: add changeset for the dictionary schema primitive Covers the full dictionary() feature shipped on this branch: DSL/validation/ SQL/planner support, RANGE/SETTINGS/BIDIRECTIONAL, pull introspection, codegen, destructive-op safety, rename support, and the ON CLUSTER fix. Co-Authored-By: Claude Sonnet 5 --- .changeset/dictionary-schema-primitive.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/dictionary-schema-primitive.md diff --git a/.changeset/dictionary-schema-primitive.md b/.changeset/dictionary-schema-primitive.md new file mode 100644 index 00000000..4b531a8d --- /dev/null +++ b/.changeset/dictionary-schema-primitive.md @@ -0,0 +1,16 @@ +--- +"chkit": patch +"@chkit/core": patch +"@chkit/clickhouse": patch +"@chkit/plugin-pull": patch +"@chkit/plugin-codegen": patch +--- + +Add `dictionary()` as a first-class ClickHouse schema primitive, mirroring `materializedView()` across the full lifecycle: DSL authoring, validation, canonicalization, SQL rendering, migration planning/diff, drift, `check`, destructive-op safety, `pull` introspection, and `codegen` typed interfaces. + +- `dictionary({ database, name, attributes, primaryKey, source, layout, lifetime, range?, settings?, comment? })` — attributes support `default`/`expression` (mutually exclusive), `hierarchical`, `bidirectional` (requires `hierarchical`), `injective`, and `isObjectId`. `range: { min, max }` renders `RANGE(MIN ... MAX ...)` for `RANGE_HASHED`/`COMPLEX_KEY_RANGE_HASHED` layouts, and `settings` renders `SETTINGS(...)`. +- ClickHouse has no `ALTER DICTIONARY`, so any structural change plans a single atomic `CREATE OR REPLACE DICTIONARY`. Dropping a dictionary is treated as destructive and blocked without `--allow-destructive`. +- Set `renamedFrom` on a dictionary (or pass `--rename-dictionary old_db.old=new_db.new` to `chkit generate`) to rename a dictionary via `RENAME DICTIONARY IF EXISTS ... TO ...` instead of a destructive drop + create. +- `chkit pull` introspects live dictionaries (including `RANGE`/`SETTINGS` and all attribute modifiers) into typed schema files, preserving ClickHouse's `[HIDDEN]` password redaction on `SOURCE(...)` credentials; the planner masks passwords before diffing so a live `[HIDDEN]` value never shows as perpetual drift. +- `codegen` generates a typed interface (and optional Zod schema) for each dictionary from its `attributes`, always included regardless of `includeViews`. +- `ON CLUSTER` mode now correctly stamps every dictionary DDL statement, including `CREATE OR REPLACE DICTIONARY` and `RENAME DICTIONARY`. From 6b83f697131adb614b7f6d6e676a74acf2ae2cc0 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Thu, 23 Jul 2026 10:16:17 +0200 Subject: [PATCH 10/15] feat(cli): warn on plain-text dictionary passwords and undetected password drift chkit generate now flags two SOURCE(...) password gaps: a literal password about to land in the generated migration file, and a password-only change that produces no operation because chkit masks passwords before diffing. --- .changeset/dictionary-password-warnings.md | 5 ++ apps/docs/src/content/docs/cli/generate.md | 13 ++++- packages/cli/src/commands/generate/command.ts | 11 +++- .../generate/dictionary-password-warnings.ts | 53 ++++++++++++++++++ packages/cli/src/commands/generate/output.ts | 16 +++++- packages/cli/src/test/generate.e2e.test.ts | 56 +++++++++++++++++++ 6 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 .changeset/dictionary-password-warnings.md create mode 100644 packages/cli/src/commands/generate/dictionary-password-warnings.ts diff --git a/.changeset/dictionary-password-warnings.md b/.changeset/dictionary-password-warnings.md new file mode 100644 index 00000000..7fc8e7ea --- /dev/null +++ b/.changeset/dictionary-password-warnings.md @@ -0,0 +1,5 @@ +--- +"chkit": patch +--- + +`chkit generate` now warns when a dictionary's `SOURCE(...)` contains a plain-text password: once when it's about to be written into the generated migration SQL file, and once when a password-only change won't produce a migration (chkit masks passwords before diffing, so that change is otherwise silent). Warnings print to the console and are included as a `warnings` array in `--json` output for both plan and apply modes. diff --git a/apps/docs/src/content/docs/cli/generate.md b/apps/docs/src/content/docs/cli/generate.md index a8d549f9..97a8e400 100644 --- a/apps/docs/src/content/docs/cli/generate.md +++ b/apps/docs/src/content/docs/cli/generate.md @@ -72,6 +72,13 @@ A dictionary rename emits a single `RENAME DICTIONARY IF EXISTS ... TO ...` stat Validation errors are raised for conflicting, chained, or cyclic rename mappings. +### Dictionary password warnings + +A dictionary's `SOURCE(...)` clause is a raw string (see [Credentials in `source`](/schema/dsl-reference/#credentials-in-source)), so any credentials it embeds are written verbatim into the generated migration SQL — ClickHouse has no DDL-level secret substitution. `generate` surfaces two warnings for this, printed to the console and included as a `warnings` array in `--json` output: + +- **Plain-text password**: a dictionary being created or replaced this run has a literal `password '...'` in its `SOURCE(...)` — it will land in the committed migration file as plain text. +- **Undetected password change**: an existing dictionary's `SOURCE(...)` password changed but nothing else did. chkit masks passwords before diffing, which means a password-only change produces no operation and no migration. Apply the new password directly against ClickHouse, or scaffold a manual statement with `chkit generate --empty`. + ### Dryrun mode With `--dryrun`, the command prints the migration plan (operations with risk levels and SQL) without writing any files. Useful for previewing changes before committing. @@ -161,7 +168,8 @@ chkit generate --rename-dictionary old_db.old_dict=new_db.new_dict { "type": "create_database", "key": "database:default", "risk": "safe", "sql": "CREATE DATABASE IF NOT EXISTS default;" }, { "type": "create_table", "key": "default.users", "risk": "safe", "sql": "CREATE TABLE ..." } ], - "renameSuggestions": [] + "renameSuggestions": [], + "warnings": [] } ``` @@ -176,7 +184,8 @@ chkit generate --rename-dictionary old_db.old_dict=new_db.new_dict "snapshotFile": "./chkit/meta/snapshot.json", "definitionCount": 3, "operationCount": 2, - "riskSummary": { "safe": 2, "caution": 0, "danger": 0 } + "riskSummary": { "safe": 2, "caution": 0, "danger": 0 }, + "warnings": [] } ``` diff --git a/packages/cli/src/commands/generate/command.ts b/packages/cli/src/commands/generate/command.ts index 9e633fa2..b9350baa 100644 --- a/packages/cli/src/commands/generate/command.ts +++ b/packages/cli/src/commands/generate/command.ts @@ -14,6 +14,7 @@ import { resolveTableScope, tableKeysFromDefinitions, } from '../../runtime/table-scope.js' +import { detectDictionaryPasswordWarnings } from './dictionary-password-warnings.js' import { applyExplicitDictionaryRenames, applyExplicitTableRenames, @@ -203,8 +204,14 @@ async function cmdGenerate(ctx: import('../../plugins.js').ChxPluginCommandConte // post-pass, after all plan transforms (renames, plugins, scope filtering). plan = applyOnClusterToPlan(plan, config.clickhouse?.cluster) + const dictionaryPasswordWarnings = detectDictionaryPasswordWarnings({ + plan, + definitions, + previousDefinitions: remappedPreviousDefinitions, + }) + if (planMode) { - emitGeneratePlanOutput(plan, jsonMode, resolvedScope) + emitGeneratePlanOutput(plan, jsonMode, resolvedScope, dictionaryPasswordWarnings) return 0 } @@ -246,6 +253,6 @@ async function cmdGenerate(ctx: import('../../plugins.js').ChxPluginCommandConte } } - emitGenerateApplyOutput(result, artifactDefinitions, plan, jsonMode, resolvedScope) + emitGenerateApplyOutput(result, artifactDefinitions, plan, jsonMode, resolvedScope, dictionaryPasswordWarnings) return 0 } diff --git a/packages/cli/src/commands/generate/dictionary-password-warnings.ts b/packages/cli/src/commands/generate/dictionary-password-warnings.ts new file mode 100644 index 00000000..db215e0a --- /dev/null +++ b/packages/cli/src/commands/generate/dictionary-password-warnings.ts @@ -0,0 +1,53 @@ +import type { DictionaryDefinition, MigrationPlan, SchemaDefinition } from '@chkit/core' + +const PASSWORD_ANY_RE = /password\s+'(?:[^'\\]|\\.)*'/i +const PASSWORD_LITERAL_RE = /password\s+'(?!\[HIDDEN\])(?:[^'\\]|\\.)*'/i + +export function detectDictionaryPasswordWarnings(input: { + plan: MigrationPlan + definitions: SchemaDefinition[] + previousDefinitions: SchemaDefinition[] +}): string[] { + const previousDictionariesByKey = new Map( + input.previousDefinitions + .filter((def): def is DictionaryDefinition => def.kind === 'dictionary') + .map((def) => [`dictionary:${def.database}.${def.name}`, def]) + ) + + return [ + ...plaintextPasswordWarnings(input.plan), + ...undetectedPasswordChangeWarnings(input.definitions, previousDictionariesByKey, input.plan), + ] +} + +function plaintextPasswordWarnings(plan: MigrationPlan): string[] { + return plan.operations + .filter((op) => op.type === 'create_dictionary' && PASSWORD_LITERAL_RE.test(op.sql)) + .map( + (op) => + `Dictionary "${op.key.slice('dictionary:'.length)}" has a password in its SOURCE(...) — it will be written in plain text to the generated migration SQL file.` + ) +} + +function undetectedPasswordChangeWarnings( + definitions: SchemaDefinition[], + previousDictionariesByKey: Map, + plan: MigrationPlan +): string[] { + const warnings: string[] = [] + + for (const def of definitions) { + if (def.kind !== 'dictionary' || !PASSWORD_ANY_RE.test(def.source)) continue + + const key = `dictionary:${def.database}.${def.name}` + const previous = previousDictionariesByKey.get(key) + if (!previous || previous.source === def.source) continue + if (plan.operations.some((op) => op.key === key)) continue + + warnings.push( + `Dictionary "${def.database}.${def.name}" SOURCE(...) password changed, but chkit masks passwords before diffing, so no migration was generated for this change — apply the new password directly against ClickHouse (or via "chkit generate --empty").` + ) + } + + return warnings +} diff --git a/packages/cli/src/commands/generate/output.ts b/packages/cli/src/commands/generate/output.ts index 06ea50d1..a8c56cc3 100644 --- a/packages/cli/src/commands/generate/output.ts +++ b/packages/cli/src/commands/generate/output.ts @@ -11,6 +11,7 @@ interface GeneratePlanPayload { riskSummary: MigrationPlan['riskSummary'] operations: MigrationPlan['operations'] renameSuggestions: MigrationPlan['renameSuggestions'] + warnings: string[] } interface GenerateApplyPayload { @@ -20,6 +21,7 @@ interface GenerateApplyPayload { definitionCount: number operationCount: number riskSummary: MigrationPlan['riskSummary'] + warnings: string[] } interface GenerateEmptyPayload { @@ -42,7 +44,12 @@ export function emitGenerateEmptyOutput(result: { migrationFile: string }, jsonM console.log('Snapshot unchanged. Add your SQL to the file, then run "chkit migrate".') } -export function emitGeneratePlanOutput(plan: MigrationPlan, jsonMode: boolean, scope: TableScope): void { +export function emitGeneratePlanOutput( + plan: MigrationPlan, + jsonMode: boolean, + scope: TableScope, + warnings: string[] = [] +): void { const payload: GeneratePlanPayload = { scope, mode: 'plan', @@ -50,6 +57,7 @@ export function emitGeneratePlanOutput(plan: MigrationPlan, jsonMode: boolean, s riskSummary: plan.riskSummary, operations: plan.operations, renameSuggestions: plan.renameSuggestions, + warnings, } if (jsonMode) { @@ -62,6 +70,7 @@ export function emitGeneratePlanOutput(plan: MigrationPlan, jsonMode: boolean, s `Risk summary: safe=${payload.riskSummary.safe}, caution=${payload.riskSummary.caution}, danger=${payload.riskSummary.danger}` ) for (const line of summarizePlan(payload.operations)) console.log(`- ${line}`) + for (const warning of payload.warnings) console.warn(`Warning: ${warning}`) if (payload.renameSuggestions.length > 0) { console.log('\nRename suggestions (review and confirm manually):') for (const suggestion of payload.renameSuggestions) { @@ -79,7 +88,8 @@ export function emitGenerateApplyOutput( definitions: SchemaDefinition[], plan: MigrationPlan, jsonMode: boolean, - scope: TableScope + scope: TableScope, + warnings: string[] = [] ): void { const payload: GenerateApplyPayload = { scope, @@ -88,6 +98,7 @@ export function emitGenerateApplyOutput( definitionCount: definitions.length, operationCount: plan.operations.length, riskSummary: plan.riskSummary, + warnings, } if (jsonMode) { @@ -112,4 +123,5 @@ export function emitGenerateApplyOutput( console.log( `Risk summary: safe=${plan.riskSummary.safe}, caution=${plan.riskSummary.caution}, danger=${plan.riskSummary.danger}` ) + for (const warning of payload.warnings) console.warn(`Warning: ${warning}`) } diff --git a/packages/cli/src/test/generate.e2e.test.ts b/packages/cli/src/test/generate.e2e.test.ts index 8cfe5914..cb31b8ea 100644 --- a/packages/cli/src/test/generate.e2e.test.ts +++ b/packages/cli/src/test/generate.e2e.test.ts @@ -463,6 +463,7 @@ describe('@chkit/cli generate e2e', () => { 'schemaVersion', 'scope', 'snapshotFile', + 'warnings', ]) } finally { await rm(fixture.dir, { recursive: true, force: true }) @@ -551,4 +552,59 @@ describe('@chkit/cli generate e2e', () => { await rm(fixture.dir, { recursive: true, force: true }) } }) + + test('generate --dryrun warns when a new dictionary SOURCE(...) has a plain-text password', async () => { + const fixture = await createFixture() + try { + await writeFile( + fixture.schemaPath, + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\nconst users = table({\n database: 'app',\n name: 'users',\n columns: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n engine: 'MergeTree()',\n primaryKey: ['id'],\n orderBy: ['id'],\n})\n\nconst usersDict = dictionary({\n database: 'app',\n name: 'users_dict',\n attributes: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n primaryKey: ['id'],\n source: "MYSQL(host 'db' user 'root' password 'secret' table 'users')",\n layout: 'FLAT()',\n lifetime: '300',\n})\n\nexport default schema(users, usersDict)\n`, + 'utf8' + ) + + const result = runCli(['generate', '--config', fixture.configPath, '--dryrun', '--json']) + expect(result.exitCode).toBe(0) + const payload = JSON.parse(result.stdout) as { warnings: string[] } + expect( + payload.warnings.some( + (warning) => warning.includes('app.users_dict') && warning.includes('plain text') + ) + ).toBe(true) + } finally { + await rm(fixture.dir, { recursive: true, force: true }) + } + }) + + test('generate --dryrun warns when only a dictionary SOURCE(...) password changed', async () => { + const fixture = await createFixture() + try { + await writeFile( + fixture.schemaPath, + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\nconst users = table({\n database: 'app',\n name: 'users',\n columns: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n engine: 'MergeTree()',\n primaryKey: ['id'],\n orderBy: ['id'],\n})\n\nconst usersDict = dictionary({\n database: 'app',\n name: 'users_dict',\n attributes: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n primaryKey: ['id'],\n source: "MYSQL(host 'db' user 'root' password 'old-secret' table 'users')",\n layout: 'FLAT()',\n lifetime: '300',\n})\n\nexport default schema(users, usersDict)\n`, + 'utf8' + ) + runCli(['generate', '--config', fixture.configPath, '--name', 'init', '--json']) + + await writeFile( + fixture.schemaPath, + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\nconst users = table({\n database: 'app',\n name: 'users',\n columns: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n engine: 'MergeTree()',\n primaryKey: ['id'],\n orderBy: ['id'],\n})\n\nconst usersDict = dictionary({\n database: 'app',\n name: 'users_dict',\n attributes: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n primaryKey: ['id'],\n source: "MYSQL(host 'db' user 'root' password 'new-secret' table 'users')",\n layout: 'FLAT()',\n lifetime: '300',\n})\n\nexport default schema(users, usersDict)\n`, + 'utf8' + ) + + const result = runCli(['generate', '--config', fixture.configPath, '--dryrun', '--json']) + expect(result.exitCode).toBe(0) + const payload = JSON.parse(result.stdout) as { + operationCount: number + warnings: string[] + } + expect(payload.operationCount).toBe(0) + expect( + payload.warnings.some( + (warning) => warning.includes('app.users_dict') && warning.includes('masks passwords') + ) + ).toBe(true) + } finally { + await rm(fixture.dir, { recursive: true, force: true }) + } + }) }) From 1536ab2756cbc16535deac4b516a9d495ae8a5f2 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Mon, 27 Jul 2026 09:54:19 +0200 Subject: [PATCH 11/15] fix(core): diff dictionary password changes instead of masking them unconditionally `diffDictionary` masked every SOURCE(...) password before comparing old vs new, so a password-only change was silently dropped and never produced a migration. Now only a source still carrying ClickHouse's [HIDDEN] introspection placeholder is excluded from the diff; a real password change diffs and migrates like any other field. Replaces the now-obsolete "undetected password change" warning in `chkit generate` with a new warning in `chkit pull`, printed when an introspected dictionary's password comes back as [HIDDEN] and chkit can't recover it. Co-Authored-By: Claude Sonnet 5 --- .changeset/dictionary-password-warnings.md | 6 +- apps/docs/src/content/docs/cli/generate.md | 7 ++- apps/docs/src/content/docs/plugins/pull.md | 4 +- .../src/content/docs/schema/dsl-reference.md | 4 +- packages/cli/src/commands/generate/command.ts | 6 +- .../generate/dictionary-password-warnings.ts | 45 +------------- packages/cli/src/test/generate.e2e.test.ts | 28 ++++++++- packages/core/src/index.test.ts | 31 +++++++++- packages/core/src/planner.ts | 24 ++++---- packages/plugin-pull/src/index.test.ts | 58 +++++++++++++++++++ packages/plugin-pull/src/index.ts | 17 ++++++ 11 files changed, 160 insertions(+), 70 deletions(-) diff --git a/.changeset/dictionary-password-warnings.md b/.changeset/dictionary-password-warnings.md index 7fc8e7ea..ad07689c 100644 --- a/.changeset/dictionary-password-warnings.md +++ b/.changeset/dictionary-password-warnings.md @@ -1,5 +1,9 @@ --- "chkit": patch +"@chkit/core": patch +"@chkit/plugin-pull": patch --- -`chkit generate` now warns when a dictionary's `SOURCE(...)` contains a plain-text password: once when it's about to be written into the generated migration SQL file, and once when a password-only change won't produce a migration (chkit masks passwords before diffing, so that change is otherwise silent). Warnings print to the console and are included as a `warnings` array in `--json` output for both plan and apply modes. +Fixed a bug where a dictionary's `SOURCE(...)` password change alone never produced a migration — the diff engine unconditionally masked passwords before comparing, so a password rotation was silently dropped. Password changes now diff and migrate like any other field change. + +`chkit generate` warns when a dictionary being created or replaced has a literal password in its `SOURCE(...)` — it's about to be written into the migration SQL file as plain text. `chkit pull` warns when an introspected dictionary's password comes back from ClickHouse as `[HIDDEN]` — chkit can't recover the real value, so that dictionary's `source` is excluded from future diffs until it's replaced. Both warnings print to the console and are included as a `warnings` array in `--json` output. diff --git a/apps/docs/src/content/docs/cli/generate.md b/apps/docs/src/content/docs/cli/generate.md index 97a8e400..a40fc016 100644 --- a/apps/docs/src/content/docs/cli/generate.md +++ b/apps/docs/src/content/docs/cli/generate.md @@ -74,10 +74,11 @@ Validation errors are raised for conflicting, chained, or cyclic rename mappings ### Dictionary password warnings -A dictionary's `SOURCE(...)` clause is a raw string (see [Credentials in `source`](/schema/dsl-reference/#credentials-in-source)), so any credentials it embeds are written verbatim into the generated migration SQL — ClickHouse has no DDL-level secret substitution. `generate` surfaces two warnings for this, printed to the console and included as a `warnings` array in `--json` output: +A dictionary's `SOURCE(...)` clause is a raw string (see [Credentials in `source`](/schema/dsl-reference/#credentials-in-source)), so any credentials it embeds are written verbatim into the generated migration SQL — ClickHouse has no DDL-level secret substitution. A password change is a real diff like any other field change and produces a `CREATE OR REPLACE DICTIONARY` migration. -- **Plain-text password**: a dictionary being created or replaced this run has a literal `password '...'` in its `SOURCE(...)` — it will land in the committed migration file as plain text. -- **Undetected password change**: an existing dictionary's `SOURCE(...)` password changed but nothing else did. chkit masks passwords before diffing, which means a password-only change produces no operation and no migration. Apply the new password directly against ClickHouse, or scaffold a manual statement with `chkit generate --empty`. +`generate` warns when a dictionary being created or replaced this run has a literal `password '...'` in its `SOURCE(...)` — it will land in the committed migration file as plain text. This prints to the console and is included as a `warnings` array in `--json` output. + +The one exception: a dictionary whose `source` still carries ClickHouse's `[HIDDEN]` introspection placeholder (written by [`chkit pull`](/plugins/pull/#credential-handling-hidden-passwords) when it can't recover the real password) never produces a migration on its own — chkit doesn't know the real value, so it can't safely diff or render it. Replace `[HIDDEN]` with a real credential in the schema file first. ### Dryrun mode diff --git a/apps/docs/src/content/docs/plugins/pull.md b/apps/docs/src/content/docs/plugins/pull.md index aad3a1f5..9e30b8c2 100644 --- a/apps/docs/src/content/docs/plugins/pull.md +++ b/apps/docs/src/content/docs/plugins/pull.md @@ -109,7 +109,7 @@ Tables may also include `uniqueKey`, `ttl`, `settings`, `indexes`, and `projecti ## Credential handling (`[HIDDEN]` passwords) -ClickHouse redacts inline `SOURCE(...)` passwords to `[HIDDEN]` on introspection (`system.tables.create_table_query`, `SHOW CREATE DICTIONARY`) and does not offer a way to recover the original value via `pull` — chkit does not attempt `format_display_secrets_in_show_and_select`. When a pulled dictionary's `source` contains `[HIDDEN]`, the generated file emits it verbatim with a leading comment: +ClickHouse redacts inline `SOURCE(...)` passwords to `[HIDDEN]` on introspection (`system.tables.create_table_query`, `SHOW CREATE DICTIONARY`) and does not offer a way to recover the original value via `pull` — chkit does not attempt `format_display_secrets_in_show_and_select`. When a pulled dictionary's `source` contains `[HIDDEN]`, `chkit pull` prints a console warning (and includes it in a `warnings` array in `--json` output), and the generated file emits the source verbatim with a leading comment: ```ts // NOTE: password redacted by ClickHouse — replace '[HIDDEN]' with your credential (e.g. process.env.X). @@ -135,7 +135,7 @@ source: `MYSQL(host 'db' port 3306 user 'reader' password '${process.env.MYSQL_P For round-trip fidelity without a manual edit, use [named collections](https://clickhouse.com/docs/operations/named-collections) on the ClickHouse side instead of an inline password — chkit does not require this, but it's the ClickHouse-native way to avoid the redaction entirely. -Because chkit masks the `password '...'` token before comparing `source` for drift, a live `[HIDDEN]` value is never reported as perpetual drift against your real secret once you've filled it in. +Because a `source` still carrying `[HIDDEN]` is excluded from the diff entirely (see [Credentials in `source`](/schema/dsl-reference/#credentials-in-source)), `chkit generate` won't produce a migration for that dictionary's `source` until you replace the placeholder with a real value. ## Current limits diff --git a/apps/docs/src/content/docs/schema/dsl-reference.md b/apps/docs/src/content/docs/schema/dsl-reference.md index 845e1bec..7a38a39b 100644 --- a/apps/docs/src/content/docs/schema/dsl-reference.md +++ b/apps/docs/src/content/docs/schema/dsl-reference.md @@ -428,7 +428,7 @@ Inline credentials in `source` (e.g. a MySQL/PostgreSQL `password '...'`) should source: `MYSQL(host 'db' password '${process.env.MYSQL_PASSWORD}' ...)`, ``` -ClickHouse redacts inline passwords back to `[HIDDEN]` on introspection (`SHOW CREATE DICTIONARY`, `system.dictionaries`). chkit masks the `password '...'` token before comparing `source` for drift/diff purposes, so a live `[HIDDEN]` value is never reported as perpetual drift against your real secret — see [Pull: credential handling](/plugins/pull/#credential-handling-hidden-passwords). +ClickHouse redacts inline passwords back to `[HIDDEN]` on introspection (`SHOW CREATE DICTIONARY`, `system.dictionaries`). A real password change diffs and migrates like any other field change. The one exception is a `source` that still carries the literal `[HIDDEN]` placeholder written by `chkit pull` — chkit never knows the real value in that case, so it excludes `source` from the diff entirely rather than risk rendering `[HIDDEN]` into DDL — see [Pull: credential handling](/plugins/pull/#credential-handling-hidden-passwords). ### No `ALTER DICTIONARY` @@ -561,7 +561,7 @@ When a property changes, chkit determines whether the table can be altered in pl Views and materialized views always use drop + recreate. -Dictionaries have no ALTER at all: any change to `attributes`, `primaryKey`, `layout`, `lifetime`, or `comment` renders as a single `CREATE OR REPLACE DICTIONARY` (`risk=caution`). A `source` change is compared with the password masked (see [Credentials in `source`](#credentials-in-source)), so only a real credential/connection change triggers a replace. Removing a dictionary from schema emits `DROP DICTIONARY` (`risk=danger`, requires `--allow-destructive`). +Dictionaries have no ALTER at all: any change to `attributes`, `primaryKey`, `layout`, `lifetime`, `source` (including a password change), or `comment` renders as a single `CREATE OR REPLACE DICTIONARY` (`risk=caution`) — except a `source` still carrying the `[HIDDEN]` introspection placeholder, which is excluded from the diff entirely (see [Credentials in `source`](#credentials-in-source)). Removing a dictionary from schema emits `DROP DICTIONARY` (`risk=danger`, requires `--allow-destructive`). :::danger Changing a structural property on an existing table generates a `DROP TABLE` followed by `CREATE TABLE` — **all rows are permanently deleted and the table is recreated empty**. The data is not copied over. The drop is classified `risk=danger` (blocked without `--allow-destructive`) and `chkit migrate` flags it with the distinct `table_recreate_data_loss` warning. To preserve data, migrate by hand instead: create a new table with the desired structure, `INSERT INTO new SELECT ... FROM old`, then swap names and drop the old table. diff --git a/packages/cli/src/commands/generate/command.ts b/packages/cli/src/commands/generate/command.ts index b9350baa..5bdbdbeb 100644 --- a/packages/cli/src/commands/generate/command.ts +++ b/packages/cli/src/commands/generate/command.ts @@ -204,11 +204,7 @@ async function cmdGenerate(ctx: import('../../plugins.js').ChxPluginCommandConte // post-pass, after all plan transforms (renames, plugins, scope filtering). plan = applyOnClusterToPlan(plan, config.clickhouse?.cluster) - const dictionaryPasswordWarnings = detectDictionaryPasswordWarnings({ - plan, - definitions, - previousDefinitions: remappedPreviousDefinitions, - }) + const dictionaryPasswordWarnings = detectDictionaryPasswordWarnings(plan) if (planMode) { emitGeneratePlanOutput(plan, jsonMode, resolvedScope, dictionaryPasswordWarnings) diff --git a/packages/cli/src/commands/generate/dictionary-password-warnings.ts b/packages/cli/src/commands/generate/dictionary-password-warnings.ts index db215e0a..1d5d99fb 100644 --- a/packages/cli/src/commands/generate/dictionary-password-warnings.ts +++ b/packages/cli/src/commands/generate/dictionary-password-warnings.ts @@ -1,26 +1,8 @@ -import type { DictionaryDefinition, MigrationPlan, SchemaDefinition } from '@chkit/core' +import type { MigrationPlan } from '@chkit/core' -const PASSWORD_ANY_RE = /password\s+'(?:[^'\\]|\\.)*'/i const PASSWORD_LITERAL_RE = /password\s+'(?!\[HIDDEN\])(?:[^'\\]|\\.)*'/i -export function detectDictionaryPasswordWarnings(input: { - plan: MigrationPlan - definitions: SchemaDefinition[] - previousDefinitions: SchemaDefinition[] -}): string[] { - const previousDictionariesByKey = new Map( - input.previousDefinitions - .filter((def): def is DictionaryDefinition => def.kind === 'dictionary') - .map((def) => [`dictionary:${def.database}.${def.name}`, def]) - ) - - return [ - ...plaintextPasswordWarnings(input.plan), - ...undetectedPasswordChangeWarnings(input.definitions, previousDictionariesByKey, input.plan), - ] -} - -function plaintextPasswordWarnings(plan: MigrationPlan): string[] { +export function detectDictionaryPasswordWarnings(plan: MigrationPlan): string[] { return plan.operations .filter((op) => op.type === 'create_dictionary' && PASSWORD_LITERAL_RE.test(op.sql)) .map( @@ -28,26 +10,3 @@ function plaintextPasswordWarnings(plan: MigrationPlan): string[] { `Dictionary "${op.key.slice('dictionary:'.length)}" has a password in its SOURCE(...) — it will be written in plain text to the generated migration SQL file.` ) } - -function undetectedPasswordChangeWarnings( - definitions: SchemaDefinition[], - previousDictionariesByKey: Map, - plan: MigrationPlan -): string[] { - const warnings: string[] = [] - - for (const def of definitions) { - if (def.kind !== 'dictionary' || !PASSWORD_ANY_RE.test(def.source)) continue - - const key = `dictionary:${def.database}.${def.name}` - const previous = previousDictionariesByKey.get(key) - if (!previous || previous.source === def.source) continue - if (plan.operations.some((op) => op.key === key)) continue - - warnings.push( - `Dictionary "${def.database}.${def.name}" SOURCE(...) password changed, but chkit masks passwords before diffing, so no migration was generated for this change — apply the new password directly against ClickHouse (or via "chkit generate --empty").` - ) - } - - return warnings -} diff --git a/packages/cli/src/test/generate.e2e.test.ts b/packages/cli/src/test/generate.e2e.test.ts index cb31b8ea..00cf1d22 100644 --- a/packages/cli/src/test/generate.e2e.test.ts +++ b/packages/cli/src/test/generate.e2e.test.ts @@ -575,7 +575,7 @@ describe('@chkit/cli generate e2e', () => { } }) - test('generate --dryrun warns when only a dictionary SOURCE(...) password changed', async () => { + test('generate --dryrun generates a migration when only a dictionary SOURCE(...) password changed', async () => { const fixture = await createFixture() try { await writeFile( @@ -595,16 +595,38 @@ describe('@chkit/cli generate e2e', () => { expect(result.exitCode).toBe(0) const payload = JSON.parse(result.stdout) as { operationCount: number + operations: Array<{ type: string; sql: string }> warnings: string[] } - expect(payload.operationCount).toBe(0) + expect(payload.operationCount).toBe(1) + expect(payload.operations[0]?.type).toBe('create_dictionary') + expect(payload.operations[0]?.sql).toContain("password 'new-secret'") expect( payload.warnings.some( - (warning) => warning.includes('app.users_dict') && warning.includes('masks passwords') + (warning) => warning.includes('app.users_dict') && warning.includes('plain text') ) ).toBe(true) } finally { await rm(fixture.dir, { recursive: true, force: true }) } }) + + test('generate --dryrun does not diff a dictionary SOURCE(...) still carrying the [HIDDEN] placeholder', async () => { + const fixture = await createFixture() + try { + await writeFile( + fixture.schemaPath, + `import { schema, table, dictionary } from '${CORE_ENTRY}'\n\nconst users = table({\n database: 'app',\n name: 'users',\n columns: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n engine: 'MergeTree()',\n primaryKey: ['id'],\n orderBy: ['id'],\n})\n\nconst usersDict = dictionary({\n database: 'app',\n name: 'users_dict',\n attributes: [\n { name: 'id', type: 'UInt64' },\n { name: 'email', type: 'String' },\n ],\n primaryKey: ['id'],\n source: "MYSQL(host 'db' user 'root' password '[HIDDEN]' table 'users')",\n layout: 'FLAT()',\n lifetime: '300',\n})\n\nexport default schema(users, usersDict)\n`, + 'utf8' + ) + runCli(['generate', '--config', fixture.configPath, '--name', 'init', '--json']) + + const result = runCli(['generate', '--config', fixture.configPath, '--dryrun', '--json']) + expect(result.exitCode).toBe(0) + const payload = JSON.parse(result.stdout) as { operationCount: number } + expect(payload.operationCount).toBe(0) + } finally { + await rm(fixture.dir, { recursive: true, force: true }) + } + }) }) diff --git a/packages/core/src/index.test.ts b/packages/core/src/index.test.ts index 56d3da62..406b32f7 100644 --- a/packages/core/src/index.test.ts +++ b/packages/core/src/index.test.ts @@ -1771,7 +1771,7 @@ describe('@chkit/core dictionaries', () => { expect(plan.operations[0]?.risk).toBe('danger') }) - test('binary diff: source differing only by password produces no diff', () => { + test('binary diff: a real password change produces a CREATE OR REPLACE DICTIONARY op', () => { const oldDefs = [dictionary({ ...baseDictionary })] const newDefs = [ dictionary({ @@ -1780,9 +1780,38 @@ describe('@chkit/core dictionaries', () => { }), ] const plan = planDiff(oldDefs, newDefs) + expect(plan.operations).toHaveLength(1) + expect(plan.operations[0]?.type).toBe('create_dictionary') + expect(plan.operations[0]?.sql).toContain("password 'a-different-secret'") + }) + + test('binary diff: a [HIDDEN] source placeholder never drives a diff on its own', () => { + const oldDefs = [dictionary({ ...baseDictionary })] + const newDefs = [ + dictionary({ + ...baseDictionary, + source: `MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')`, + }), + ] + const plan = planDiff(oldDefs, newDefs) expect(plan.operations).toHaveLength(0) }) + test('binary diff: a [HIDDEN] source placeholder does not suppress unrelated field changes', () => { + const oldDefs = [dictionary({ ...baseDictionary })] + const newDefs = [ + dictionary({ + ...baseDictionary, + source: `MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')`, + layout: 'COMPLEX_KEY_HASHED()', + }), + ] + const plan = planDiff(oldDefs, newDefs) + expect(plan.operations).toHaveLength(1) + expect(plan.operations[0]?.type).toBe('create_dictionary') + expect(plan.operations[0]?.sql).toContain("password '[HIDDEN]'") + }) + test('renders RANGE, SETTINGS, and BIDIRECTIONAL clauses', () => { const dict = dictionary({ ...baseDictionary, diff --git a/packages/core/src/planner.ts b/packages/core/src/planner.ts index 9ea8b36a..5689bd8f 100644 --- a/packages/core/src/planner.ts +++ b/packages/core/src/planner.ts @@ -316,26 +316,30 @@ function diffMaterializedView( return [] } -// Redacts inline SOURCE() credentials for comparison only, mirroring what -// ClickHouse itself does on introspection (`[HIDDEN]`). A definition whose -// only difference is the password must not diff every run — never used to -// render DDL, so the real secret still reaches ClickHouse. -function maskDictionarySecrets(source: string): string { - return source.replace(/password\s+'(?:[^'\\]|\\.)*'/gi, "password '[HIDDEN]'") +// `chkit pull` writes ClickHouse's own introspection placeholder +// (`password '[HIDDEN]'`) into `source` when it can't recover a dictionary's +// real credential. That placeholder must never drive a diff — rendering it +// would deploy the literal string "[HIDDEN]" as the password. A real +// password value, by contrast, is fully known to chkit (it's a plain string +// in the schema file) and a change to it is a genuine diff like any other. +function dictionarySourceIsHidden(source: string): boolean { + return source.includes('[HIDDEN]') } -function dictionaryComparisonShape(def: DictionaryDefinition) { +function dictionaryComparisonShape(def: DictionaryDefinition, omitSource: boolean) { + if (!omitSource) return def const { source, ...rest } = def - return { ...rest, source: maskDictionarySecrets(source) } + return rest } function diffDictionary( oldDef: DictionaryDefinition, newDef: DictionaryDefinition ): MigrationOperation[] { + const omitSource = dictionarySourceIsHidden(newDef.source) const unchanged = - JSON.stringify(dictionaryComparisonShape(oldDef)) === - JSON.stringify(dictionaryComparisonShape(newDef)) + JSON.stringify(dictionaryComparisonShape(oldDef, omitSource)) === + JSON.stringify(dictionaryComparisonShape(newDef, omitSource)) if (unchanged) return [] return [ diff --git a/packages/plugin-pull/src/index.test.ts b/packages/plugin-pull/src/index.test.ts index 9807d267..839718d7 100644 --- a/packages/plugin-pull/src/index.test.ts +++ b/packages/plugin-pull/src/index.test.ts @@ -500,6 +500,64 @@ describe('@chkit/plugin-pull schema command', () => { expect(payload.content).toContain('const app_users_mv = materializedView({') }) + test('warns when an introspected dictionary SOURCE(...) password is [HIDDEN]', async () => { + const plugin = createPullPlugin({ + databases: ['app'], + introspect: async () => [ + { + kind: 'dictionary', + database: 'app', + name: 'users_dict', + attributes: [{ name: 'id', type: 'UInt64' }], + primaryKey: ['id'], + source: "MYSQL(host 'db' port 3306 user 'reader' password '[HIDDEN]' db 'app' table 'users')", + layout: 'HASHED()', + lifetime: '300', + }, + ], + }) + + const command = plugin.commands[0] + if (!command) throw new Error('missing command') + + const logs: unknown[] = [] + const code = await command.run({ + args: [], + flags: { '--dryrun': true }, + jsonMode: true, + options: PullSchema.parse({ databases: ['app'] }), + rawOptions: { databases: ['app'] }, + configPath: '/tmp/clickhouse.config.ts', + config: { + schema: ['./schema.ts'], + outDir: './chkit', + migrationsDir: './chkit/migrations', + metaDir: './chkit/meta', + plugins: [], + check: { failOnPending: true, failOnChecksumMismatch: true, failOnDrift: true }, + safety: { allowDestructive: false }, + clickhouse: { + url: 'http://localhost:8123', + username: 'default', + password: '', + database: 'default', + secure: false, + }, + }, + print(value) { + logs.push(value) + }, + }) + + expect(code).toBe(0) + const payload = logs[0] as { warnings: string[] } + expect( + payload.warnings.some( + (warning) => warning.includes('app.users_dict') && warning.includes('[HIDDEN]') + ) + ).toBe(true) + }) + test('writes schema file and fails on existing file without --force', async () => { const dir = await mkdtemp(join(tmpdir(), 'chkit-plugin-pull-')) const outFile = join(dir, 'schema.ts') diff --git a/packages/plugin-pull/src/index.ts b/packages/plugin-pull/src/index.ts index c61ab7c9..2166b1f8 100644 --- a/packages/plugin-pull/src/index.ts +++ b/packages/plugin-pull/src/index.ts @@ -133,6 +133,19 @@ interface PullSchemaResult { databases: string[] skippedObjects: Array<{ kind: string; count: number }> content: string + warnings: string[] +} + +// ClickHouse redacts a dictionary's SOURCE(...) password to `[HIDDEN]` on +// introspection and offers no way to recover the real value via pull — flag +// it immediately so the placeholder doesn't sit unnoticed in the schema file. +function dictionaryHiddenPasswordWarnings(definitions: SchemaDefinition[]): string[] { + return definitions + .filter((def) => def.kind === 'dictionary' && def.source.includes('[HIDDEN]')) + .map( + (def) => + `Dictionary "${def.database}.${def.name}" SOURCE(...) password was redacted by ClickHouse to '[HIDDEN]' — chkit could not recover the real value. Replace it in the generated schema file before this dictionary's source can be diffed or migrated.` + ) } function stringArrayFlag(value: string | string[] | boolean | undefined): string[] | undefined { @@ -208,6 +221,7 @@ export function createPullPlugin(options: PullPluginOptions = {}): PullPlugin { databases: pulled.databases, skippedObjects: pulled.skippedObjects, dryrun, + warnings: pulled.warnings, ...(dryrun ? { content: pulled.content } : {}), } @@ -226,6 +240,7 @@ export function createPullPlugin(options: PullPluginOptions = {}): PullPlugin { `Pulled ${pulled.definitionCount} objects from ${pulled.databases.join(', ') || '(none)'} to ${pulled.outFile}` ) } + for (const warning of pulled.warnings) console.warn(`Warning: ${warning}`) return 0 }, }), @@ -295,6 +310,7 @@ async function pullSchema(input: { const content = renderSchemaFile(definitions) const tableCount = definitions.filter((definition) => definition.kind === 'table').length const skippedObjects = summarizeSkippedObjects(objects, definitions, selectedDatabases) + const warnings = dictionaryHiddenPasswordWarnings(definitions) return { outFile, @@ -303,6 +319,7 @@ async function pullSchema(input: { databases: selectedDatabases, skippedObjects, content, + warnings, } } From 821c399d479437e8ca7e43047f895ac3d685a1e7 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Mon, 27 Jul 2026 10:11:19 +0200 Subject: [PATCH 12/15] chore: consolidate dictionary changesets into one feature entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dictionary-password-warnings.md described its diffing fix as "fixed a bug," but that behavior never shipped in a release — it only existed within this same unreleased branch. Fold it into the single dictionary-schema-primitive changeset so the changelog reads as one coherent feature description instead of a feature followed by a fix to itself. Co-Authored-By: Claude Sonnet 5 --- .changeset/dictionary-password-warnings.md | 9 --------- .changeset/dictionary-schema-primitive.md | 2 +- 2 files changed, 1 insertion(+), 10 deletions(-) delete mode 100644 .changeset/dictionary-password-warnings.md diff --git a/.changeset/dictionary-password-warnings.md b/.changeset/dictionary-password-warnings.md deleted file mode 100644 index ad07689c..00000000 --- a/.changeset/dictionary-password-warnings.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -"chkit": patch -"@chkit/core": patch -"@chkit/plugin-pull": patch ---- - -Fixed a bug where a dictionary's `SOURCE(...)` password change alone never produced a migration — the diff engine unconditionally masked passwords before comparing, so a password rotation was silently dropped. Password changes now diff and migrate like any other field change. - -`chkit generate` warns when a dictionary being created or replaced has a literal password in its `SOURCE(...)` — it's about to be written into the migration SQL file as plain text. `chkit pull` warns when an introspected dictionary's password comes back from ClickHouse as `[HIDDEN]` — chkit can't recover the real value, so that dictionary's `source` is excluded from future diffs until it's replaced. Both warnings print to the console and are included as a `warnings` array in `--json` output. diff --git a/.changeset/dictionary-schema-primitive.md b/.changeset/dictionary-schema-primitive.md index 4b531a8d..988a9f2e 100644 --- a/.changeset/dictionary-schema-primitive.md +++ b/.changeset/dictionary-schema-primitive.md @@ -11,6 +11,6 @@ Add `dictionary()` as a first-class ClickHouse schema primitive, mirroring `mate - `dictionary({ database, name, attributes, primaryKey, source, layout, lifetime, range?, settings?, comment? })` — attributes support `default`/`expression` (mutually exclusive), `hierarchical`, `bidirectional` (requires `hierarchical`), `injective`, and `isObjectId`. `range: { min, max }` renders `RANGE(MIN ... MAX ...)` for `RANGE_HASHED`/`COMPLEX_KEY_RANGE_HASHED` layouts, and `settings` renders `SETTINGS(...)`. - ClickHouse has no `ALTER DICTIONARY`, so any structural change plans a single atomic `CREATE OR REPLACE DICTIONARY`. Dropping a dictionary is treated as destructive and blocked without `--allow-destructive`. - Set `renamedFrom` on a dictionary (or pass `--rename-dictionary old_db.old=new_db.new` to `chkit generate`) to rename a dictionary via `RENAME DICTIONARY IF EXISTS ... TO ...` instead of a destructive drop + create. -- `chkit pull` introspects live dictionaries (including `RANGE`/`SETTINGS` and all attribute modifiers) into typed schema files, preserving ClickHouse's `[HIDDEN]` password redaction on `SOURCE(...)` credentials; the planner masks passwords before diffing so a live `[HIDDEN]` value never shows as perpetual drift. +- `chkit pull` introspects live dictionaries (including `RANGE`/`SETTINGS` and all attribute modifiers) into typed schema files, preserving ClickHouse's `[HIDDEN]` password redaction on `SOURCE(...)` credentials. A `SOURCE(...)` password change diffs and migrates like any other field change; `chkit generate` warns when a literal password is about to be written into migration SQL as plain text, and `chkit pull` warns when an introspected password comes back as `[HIDDEN]` (chkit can't recover the real value, so that dictionary's `source` is excluded from future diffs until it's replaced). Both warnings print to the console and are included as a `warnings` array in `--json` output. - `codegen` generates a typed interface (and optional Zod schema) for each dictionary from its `attributes`, always included regardless of `includeViews`. - `ON CLUSTER` mode now correctly stamps every dictionary DDL statement, including `CREATE OR REPLACE DICTIONARY` and `RENAME DICTIONARY`. From 99321bbec4ccca20f5ce26ef3eef8c2de27062b7 Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Mon, 27 Jul 2026 10:14:12 +0200 Subject: [PATCH 13/15] docs(changeset): reword ON CLUSTER dictionary line to avoid implying a prior bug Same issue as the previous changeset cleanup: "now correctly stamps" reads as a fix to previously-shipped behavior, but ON CLUSTER support for dictionaries never shipped before this branch. State the behavior plainly instead. Co-Authored-By: Claude Sonnet 5 --- .changeset/dictionary-schema-primitive.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/dictionary-schema-primitive.md b/.changeset/dictionary-schema-primitive.md index 988a9f2e..f3502425 100644 --- a/.changeset/dictionary-schema-primitive.md +++ b/.changeset/dictionary-schema-primitive.md @@ -13,4 +13,4 @@ Add `dictionary()` as a first-class ClickHouse schema primitive, mirroring `mate - Set `renamedFrom` on a dictionary (or pass `--rename-dictionary old_db.old=new_db.new` to `chkit generate`) to rename a dictionary via `RENAME DICTIONARY IF EXISTS ... TO ...` instead of a destructive drop + create. - `chkit pull` introspects live dictionaries (including `RANGE`/`SETTINGS` and all attribute modifiers) into typed schema files, preserving ClickHouse's `[HIDDEN]` password redaction on `SOURCE(...)` credentials. A `SOURCE(...)` password change diffs and migrates like any other field change; `chkit generate` warns when a literal password is about to be written into migration SQL as plain text, and `chkit pull` warns when an introspected password comes back as `[HIDDEN]` (chkit can't recover the real value, so that dictionary's `source` is excluded from future diffs until it's replaced). Both warnings print to the console and are included as a `warnings` array in `--json` output. - `codegen` generates a typed interface (and optional Zod schema) for each dictionary from its `attributes`, always included regardless of `includeViews`. -- `ON CLUSTER` mode now correctly stamps every dictionary DDL statement, including `CREATE OR REPLACE DICTIONARY` and `RENAME DICTIONARY`. +- `ON CLUSTER` mode stamps `ON CLUSTER ` onto every dictionary DDL statement, including `CREATE OR REPLACE DICTIONARY` and `RENAME DICTIONARY`. From 37875a23faa4404a1d74801855f3ac33e49bc30c Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Mon, 27 Jul 2026 10:31:10 +0200 Subject: [PATCH 14/15] feat(plugin-pull): warn when an introspected dictionary password is plain text ClickHouse's [HIDDEN] redaction on SOURCE(...) can be turned off server-side (display_secrets_in_show_and_select + the displaySecretsInShowAndSelect grant), in which case pull receives the real password and writes it verbatim into the schema file with no other signal. Detect the non-[HIDDEN] case the same way the existing warning detects [HIDDEN], and mention the setting as an escape hatch in the [HIDDEN] warning too. Verified against a local ClickHouse instance with both settings enabled. Co-Authored-By: Claude Sonnet 5 --- .changeset/dictionary-schema-primitive.md | 2 +- apps/docs/src/content/docs/plugins/pull.md | 8 ++- packages/plugin-pull/src/index.test.ts | 58 ++++++++++++++++++++++ packages/plugin-pull/src/index.ts | 35 +++++++++---- 4 files changed, 92 insertions(+), 11 deletions(-) diff --git a/.changeset/dictionary-schema-primitive.md b/.changeset/dictionary-schema-primitive.md index f3502425..a88d20a8 100644 --- a/.changeset/dictionary-schema-primitive.md +++ b/.changeset/dictionary-schema-primitive.md @@ -11,6 +11,6 @@ Add `dictionary()` as a first-class ClickHouse schema primitive, mirroring `mate - `dictionary({ database, name, attributes, primaryKey, source, layout, lifetime, range?, settings?, comment? })` — attributes support `default`/`expression` (mutually exclusive), `hierarchical`, `bidirectional` (requires `hierarchical`), `injective`, and `isObjectId`. `range: { min, max }` renders `RANGE(MIN ... MAX ...)` for `RANGE_HASHED`/`COMPLEX_KEY_RANGE_HASHED` layouts, and `settings` renders `SETTINGS(...)`. - ClickHouse has no `ALTER DICTIONARY`, so any structural change plans a single atomic `CREATE OR REPLACE DICTIONARY`. Dropping a dictionary is treated as destructive and blocked without `--allow-destructive`. - Set `renamedFrom` on a dictionary (or pass `--rename-dictionary old_db.old=new_db.new` to `chkit generate`) to rename a dictionary via `RENAME DICTIONARY IF EXISTS ... TO ...` instead of a destructive drop + create. -- `chkit pull` introspects live dictionaries (including `RANGE`/`SETTINGS` and all attribute modifiers) into typed schema files, preserving ClickHouse's `[HIDDEN]` password redaction on `SOURCE(...)` credentials. A `SOURCE(...)` password change diffs and migrates like any other field change; `chkit generate` warns when a literal password is about to be written into migration SQL as plain text, and `chkit pull` warns when an introspected password comes back as `[HIDDEN]` (chkit can't recover the real value, so that dictionary's `source` is excluded from future diffs until it's replaced). Both warnings print to the console and are included as a `warnings` array in `--json` output. +- `chkit pull` introspects live dictionaries (including `RANGE`/`SETTINGS` and all attribute modifiers) into typed schema files, preserving ClickHouse's `[HIDDEN]` password redaction on `SOURCE(...)` credentials. A `SOURCE(...)` password change diffs and migrates like any other field change; `chkit generate` warns when a literal password is about to be written into migration SQL as plain text. `chkit pull` warns in two cases: when an introspected password comes back as `[HIDDEN]` (chkit can't recover the real value, so that dictionary's `source` is excluded from future diffs until it's replaced), and when ClickHouse is configured to reveal real passwords on introspection (`display_secrets_in_show_and_select` + `displaySecretsInShowAndSelect`), since that writes a plain-text credential into the generated schema file with no other indication. All warnings print to the console and are included as a `warnings` array in `--json` output. - `codegen` generates a typed interface (and optional Zod schema) for each dictionary from its `attributes`, always included regardless of `includeViews`. - `ON CLUSTER` mode stamps `ON CLUSTER ` onto every dictionary DDL statement, including `CREATE OR REPLACE DICTIONARY` and `RENAME DICTIONARY`. diff --git a/apps/docs/src/content/docs/plugins/pull.md b/apps/docs/src/content/docs/plugins/pull.md index 9e30b8c2..ec2b93a4 100644 --- a/apps/docs/src/content/docs/plugins/pull.md +++ b/apps/docs/src/content/docs/plugins/pull.md @@ -109,7 +109,7 @@ Tables may also include `uniqueKey`, `ttl`, `settings`, `indexes`, and `projecti ## Credential handling (`[HIDDEN]` passwords) -ClickHouse redacts inline `SOURCE(...)` passwords to `[HIDDEN]` on introspection (`system.tables.create_table_query`, `SHOW CREATE DICTIONARY`) and does not offer a way to recover the original value via `pull` — chkit does not attempt `format_display_secrets_in_show_and_select`. When a pulled dictionary's `source` contains `[HIDDEN]`, `chkit pull` prints a console warning (and includes it in a `warnings` array in `--json` output), and the generated file emits the source verbatim with a leading comment: +By default, ClickHouse redacts inline `SOURCE(...)` passwords to `[HIDDEN]` on introspection (`system.tables.create_table_query`, `SHOW CREATE DICTIONARY`), and chkit does not attempt to work around that. When a pulled dictionary's `source` contains `[HIDDEN]`, `chkit pull` prints a console warning (and includes it in a `warnings` array in `--json` output), and the generated file emits the source verbatim with a leading comment: ```ts // NOTE: password redacted by ClickHouse — replace '[HIDDEN]' with your credential (e.g. process.env.X). @@ -137,6 +137,12 @@ For round-trip fidelity without a manual edit, use [named collections](https://c Because a `source` still carrying `[HIDDEN]` is excluded from the diff entirely (see [Credentials in `source`](/schema/dsl-reference/#credentials-in-source)), `chkit generate` won't produce a migration for that dictionary's `source` until you replace the placeholder with a real value. +### Recovering the real password instead + +ClickHouse can be configured to skip the redaction and hand back the real password on introspection: enable the server-side `display_secrets_in_show_and_select` setting and grant the connecting user `displaySecretsInShowAndSelect`. The `[HIDDEN]` warning mentions this escape hatch. + +If you do this, be aware of the consequence: `chkit pull` has no way to detect that this is happening, or to re-redact the value on your behalf — it just copies through whatever ClickHouse returns. The real password lands in the generated schema file in plain text, same as any dictionary you'd author by hand with an inline credential. `chkit pull` detects this case too and warns that a plain-text password was written to the file, so you don't discover it by accident later. + ## Current limits - Materialized views without a `TO` clause are skipped. diff --git a/packages/plugin-pull/src/index.test.ts b/packages/plugin-pull/src/index.test.ts index 839718d7..5e51a912 100644 --- a/packages/plugin-pull/src/index.test.ts +++ b/packages/plugin-pull/src/index.test.ts @@ -558,6 +558,64 @@ describe('@chkit/plugin-pull schema command', () => { ).toBe(true) }) + test('warns when an introspected dictionary SOURCE(...) password is plain text', async () => { + const plugin = createPullPlugin({ + databases: ['app'], + introspect: async () => [ + { + kind: 'dictionary', + database: 'app', + name: 'users_dict', + attributes: [{ name: 'id', type: 'UInt64' }], + primaryKey: ['id'], + source: "MYSQL(host 'db' port 3306 user 'reader' password 'super-secret-pw' db 'app' table 'users')", + layout: 'HASHED()', + lifetime: '300', + }, + ], + }) + + const command = plugin.commands[0] + if (!command) throw new Error('missing command') + + const logs: unknown[] = [] + const code = await command.run({ + args: [], + flags: { '--dryrun': true }, + jsonMode: true, + options: PullSchema.parse({ databases: ['app'] }), + rawOptions: { databases: ['app'] }, + configPath: '/tmp/clickhouse.config.ts', + config: { + schema: ['./schema.ts'], + outDir: './chkit', + migrationsDir: './chkit/migrations', + metaDir: './chkit/meta', + plugins: [], + check: { failOnPending: true, failOnChecksumMismatch: true, failOnDrift: true }, + safety: { allowDestructive: false }, + clickhouse: { + url: 'http://localhost:8123', + username: 'default', + password: '', + database: 'default', + secure: false, + }, + }, + print(value) { + logs.push(value) + }, + }) + + expect(code).toBe(0) + const payload = logs[0] as { warnings: string[] } + expect( + payload.warnings.some( + (warning) => warning.includes('app.users_dict') && warning.includes('plain-text password') + ) + ).toBe(true) + }) + test('writes schema file and fails on existing file without --force', async () => { const dir = await mkdtemp(join(tmpdir(), 'chkit-plugin-pull-')) const outFile = join(dir, 'schema.ts') diff --git a/packages/plugin-pull/src/index.ts b/packages/plugin-pull/src/index.ts index 2166b1f8..5636b288 100644 --- a/packages/plugin-pull/src/index.ts +++ b/packages/plugin-pull/src/index.ts @@ -13,6 +13,7 @@ import { type ChxInlinePluginRegistration, createPluginRunner, defineFlags, + type DictionaryDefinition, type FlagMapping, normalizeEngine, type ResolvedChxConfig, @@ -136,16 +137,32 @@ interface PullSchemaResult { warnings: string[] } +const PASSWORD_LITERAL_RE = /password\s+'(?!\[HIDDEN\])(?:[^'\\]|\\.)*'/i + // ClickHouse redacts a dictionary's SOURCE(...) password to `[HIDDEN]` on -// introspection and offers no way to recover the real value via pull — flag -// it immediately so the placeholder doesn't sit unnoticed in the schema file. -function dictionaryHiddenPasswordWarnings(definitions: SchemaDefinition[]): string[] { +// introspection by default, and offers no way to recover the real value via +// pull — flag it so the placeholder doesn't sit unnoticed in the schema +// file. That redaction can also be turned off server-side (server config +// `display_secrets_in_show_and_select` plus the `displaySecretsInShowAndSelect` +// grant on the connecting user), in which case ClickHouse hands chkit the +// real password and it's written verbatim into the schema file — chkit has +// no way to detect or opt out of that, so flag it too. +function dictionaryPasswordWarnings(definitions: SchemaDefinition[]): string[] { return definitions - .filter((def) => def.kind === 'dictionary' && def.source.includes('[HIDDEN]')) - .map( - (def) => - `Dictionary "${def.database}.${def.name}" SOURCE(...) password was redacted by ClickHouse to '[HIDDEN]' — chkit could not recover the real value. Replace it in the generated schema file before this dictionary's source can be diffed or migrated.` - ) + .filter((def): def is DictionaryDefinition => def.kind === 'dictionary') + .flatMap((def) => { + if (def.source.includes('[HIDDEN]')) { + return [ + `Dictionary "${def.database}.${def.name}" SOURCE(...) password was redacted by ClickHouse to '[HIDDEN]' — chkit could not recover the real value. Replace it in the generated schema file before this dictionary's source can be diffed or migrated. To have ClickHouse reveal real passwords on introspection instead, grant the connecting user "displaySecretsInShowAndSelect" and enable the server-side "display_secrets_in_show_and_select" setting.`, + ] + } + if (PASSWORD_LITERAL_RE.test(def.source)) { + return [ + `Dictionary "${def.database}.${def.name}" SOURCE(...) has a plain-text password — ClickHouse returned the real credential on introspection and it was written verbatim into the generated schema file.`, + ] + } + return [] + }) } function stringArrayFlag(value: string | string[] | boolean | undefined): string[] | undefined { @@ -310,7 +327,7 @@ async function pullSchema(input: { const content = renderSchemaFile(definitions) const tableCount = definitions.filter((definition) => definition.kind === 'table').length const skippedObjects = summarizeSkippedObjects(objects, definitions, selectedDatabases) - const warnings = dictionaryHiddenPasswordWarnings(definitions) + const warnings = dictionaryPasswordWarnings(definitions) return { outFile, From 385fc966ff759bf5eaa799afb91f3afd704f6abc Mon Sep 17 00:00:00 2001 From: Jose Enrique Date: Mon, 27 Jul 2026 15:50:10 +0200 Subject: [PATCH 15/15] fix(clickhouse): anchor dictionary keyword parsing past the attribute list; wait on system.dictionaries for create_dictionary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues from PR #191 review: - create-dictionary-parser.ts searched the whole query text for SOURCE/LAYOUT/LIFETIME/RANGE/SETTINGS, the same bug class just fixed for create-table-parser.ts (#190). RANGE in particular collides with ClickHouse's real range() array function — a dictionary with `EXPRESSION ...range(1, 10)...` on one attribute and a genuine RANGE(MIN ... MAX ...) clause would silently lose that clause on pull. Anchor all of it to the region after the attribute list, like the table parser does. - waitForDDLPropagation's create_dictionary case waited on system.tables (waitForTable) instead of the already-added waitForDictionary (system.dictionaries), so chkit migrate could return before a dictionary had actually finished loading — particularly relevant on replicated/clustered targets. Co-Authored-By: Claude Sonnet 5 --- .../src/create-dictionary-parser.ts | 50 +++++++++++++++---- packages/clickhouse/src/ddl-propagation.ts | 2 +- packages/clickhouse/src/index.test.ts | 35 +++++++++++++ 3 files changed, 75 insertions(+), 12 deletions(-) diff --git a/packages/clickhouse/src/create-dictionary-parser.ts b/packages/clickhouse/src/create-dictionary-parser.ts index 84d70033..02a9bf7f 100644 --- a/packages/clickhouse/src/create-dictionary-parser.ts +++ b/packages/clickhouse/src/create-dictionary-parser.ts @@ -53,24 +53,52 @@ function extractBalancedParenGroup( return undefined } -function extractCreateDictionaryBody(query: string | undefined): string | undefined { - if (!query) return undefined +/** + * Positions of the parens that open and close the attribute list — the close + * being the one right before the dictionary's `PRIMARY KEY ...` clause. + * Returns undefined when there is no balanced attribute list (e.g. a query we + * can't parse). + */ +function findAttributeListBounds(query: string): { open: number; close: number } | undefined { const nameMatch = query.match( /\bCREATE\s+(?:OR\s+REPLACE\s+)?DICTIONARY\s+(?:IF\s+NOT\s+EXISTS\s+)?[`\w.]+\s*/i ) if (!nameMatch || nameMatch.index === undefined) return undefined const start = nameMatch.index + nameMatch[0].length + const openIndex = query.indexOf('(', start) + if (openIndex === -1) return undefined const group = extractBalancedParenGroup(query, start) - if (!group) return undefined - const trimmed = group.content.trim() - return trimmed.length > 0 ? trimmed : undefined + return group ? { open: openIndex, close: group.endIndex - 1 } : undefined +} + +function extractCreateDictionaryBody(query: string | undefined): string | undefined { + if (!query) return undefined + const bounds = findAttributeListBounds(query) + if (!bounds) return undefined + const body = query.slice(bounds.open + 1, bounds.close).trim() + return body.length > 0 ? body : undefined +} + +/** + * Everything after the attribute list: `PRIMARY KEY ... SOURCE(...) LAYOUT(...) + * ...`. This is where dictionary-level clauses live. Table-level keywords like + * SOURCE/RANGE can also appear inside an attribute's EXPRESSION (`range()` is a + * real ClickHouse array function) — searching the whole query would risk + * matching one of those instead of the real clause. Falls back to the whole + * query when the attribute list can't be located, preserving behaviour for + * unparseable inputs. + */ +function extractDictionaryOptions(query: string): string { + const bounds = findAttributeListBounds(query) + return bounds ? query.slice(bounds.close + 1) : query } function extractKeywordParenBody(query: string, keyword: string): string | undefined { - const match = new RegExp(`\\b${keyword}\\s*\\(`, 'i').exec(query) + const options = extractDictionaryOptions(query) + const match = new RegExp(`\\b${keyword}\\s*\\(`, 'i').exec(options) if (!match || match.index === undefined) return undefined const openIndex = match.index + match[0].length - 1 - const group = extractBalancedParenGroup(query, openIndex) + const group = extractBalancedParenGroup(options, openIndex) return group ? group.content.trim() : undefined } @@ -215,10 +243,10 @@ export function parseDictionaryPrimaryKeyFromCreateDictionaryQuery( query: string | undefined ): string[] { if (!query) return [] - const match = - /\)\s*PRIMARY\s+KEY\s+([\s\S]*?)(?:\bSOURCE\s*\(|\bLAYOUT\s*\(|\bLIFETIME\s*\(|\bCOMMENT\b|;|$)/i.exec( - query - ) + const options = extractDictionaryOptions(query) + const match = /^\s*PRIMARY\s+KEY\s+([\s\S]*?)(?:\bSOURCE\s*\(|\bLAYOUT\s*\(|\bLIFETIME\s*\(|\bCOMMENT\b|;|$)/i.exec( + options + ) const raw = match?.[1]?.trim() if (!raw) return [] return splitTopLevelComma(raw) diff --git a/packages/clickhouse/src/ddl-propagation.ts b/packages/clickhouse/src/ddl-propagation.ts index 180ad2da..a2e652fe 100644 --- a/packages/clickhouse/src/ddl-propagation.ts +++ b/packages/clickhouse/src/ddl-propagation.ts @@ -146,7 +146,7 @@ export async function waitForDDLPropagation( return waitForView(executor, parsed.database, parsed.table) case 'create_dictionary': - return waitForTable(executor, parsed.database, parsed.table) + return waitForDictionary(executor, parsed.database, parsed.table) case 'alter_table_add_column': case 'alter_table_modify_column': diff --git a/packages/clickhouse/src/index.test.ts b/packages/clickhouse/src/index.test.ts index 5100fffb..c6c45a39 100644 --- a/packages/clickhouse/src/index.test.ts +++ b/packages/clickhouse/src/index.test.ts @@ -669,4 +669,39 @@ SETTINGS(dictionary_use_async_executor = 1, max_threads = 8)` expect(parseDictionaryRangeFromCreateDictionaryQuery(undefined)).toBeUndefined() expect(parseDictionarySettingsFromCreateDictionaryQuery(undefined)).toBeUndefined() }) + + test('an EXPRESSION calling the range() array function does not shadow the real RANGE clause', () => { + // `range(...)` is a real ClickHouse array function, so it can legitimately + // appear inside an attribute's EXPRESSION. The parser must not mistake it + // for the dictionary-level RANGE(MIN ... MAX ...) clause. + const shadowedRangeQuery = `CREATE DICTIONARY default.rates_dict +( + \`id\` UInt64, + \`buckets\` String EXPRESSION arrayStringConcat(arrayMap(x -> toString(x), range(1, 10)), ','), + \`start_date\` DateTime, + \`end_date\` DateTime +) +PRIMARY KEY id +SOURCE(HTTP(url 'http://example.com/rates' format 'TSV')) +LAYOUT(RANGE_HASHED()) +LIFETIME(MIN 0 MAX 300) +RANGE(MIN start_date MAX end_date)` + expect(parseDictionaryAttributesFromCreateDictionaryQuery(shadowedRangeQuery)).toEqual([ + { name: 'id', type: 'UInt64' }, + { + name: 'buckets', + type: 'String', + expression: "arrayStringConcat(arrayMap(x -> toString(x), range(1, 10)), ',')", + }, + { name: 'start_date', type: 'DateTime' }, + { name: 'end_date', type: 'DateTime' }, + ]) + expect(parseDictionaryRangeFromCreateDictionaryQuery(shadowedRangeQuery)).toEqual({ + min: 'start_date', + max: 'end_date', + }) + expect(parseSourceFromCreateDictionaryQuery(shadowedRangeQuery)).toBe( + "HTTP(url 'http://example.com/rates' format 'TSV')" + ) + }) })