diff --git a/.changeset/mv-topo-order.md b/.changeset/mv-topo-order.md new file mode 100644 index 0000000..4a485b0 --- /dev/null +++ b/.changeset/mv-topo-order.md @@ -0,0 +1,6 @@ +--- +"@chkit/core": patch +"chkit": patch +--- + +Order materialized-view creates by their `refresh.dependsOn` edges. Creates within the same kind were tie-broken purely by name, so a refreshable materialized view declared `DEPENDS ON other_mv` whose name sorted before its dependency could be created first and fail. The planner now creates a `DEPENDS ON` target before the view that depends on it; independent views keep their stable alphabetical order. diff --git a/packages/core/src/planner-mv-order.test.ts b/packages/core/src/planner-mv-order.test.ts new file mode 100644 index 0000000..96f4c26 --- /dev/null +++ b/packages/core/src/planner-mv-order.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from 'bun:test' + +import { materializedView, planDiff } from './index' + +/** + * #41: create operations within the same kind were tie-broken purely by name. + * A refreshable materialized view declared `DEPENDS ON other_mv` must be + * created after its dependency, so ordering by name alone can place a dependent + * view before the view it depends on and fail. + */ +describe('planDiff materialized-view creation order (#41)', () => { + test('a DEPENDS ON target is created before the view that depends on it, even when names sort the other way', () => { + // `aaa_dependent` sorts before `zzz_base`, but it DEPENDS ON `zzz_base`, + // so a name-only order would create it first and fail. + const base = materializedView({ + database: 'default', + name: 'zzz_base', + to: { database: 'default', name: 'zzz_base_target' }, + as: 'SELECT id FROM default.source', + }) + const dependent = materializedView({ + database: 'default', + name: 'aaa_dependent', + to: { database: 'default', name: 'aaa_dependent_target' }, + refresh: { + every: '1 HOUR', + dependsOn: [{ database: 'default', name: 'zzz_base' }], + }, + as: 'SELECT id FROM default.zzz_base_target', + }) + + const plan = planDiff([], [dependent, base]) + const order = plan.operations + .filter((op) => op.type === 'create_materialized_view') + .map((op) => op.key) + + expect(order).toEqual([ + 'materialized_view:default.zzz_base', + 'materialized_view:default.aaa_dependent', + ]) + }) + + test('independent materialized views keep a stable alphabetical order', () => { + const first = materializedView({ + database: 'default', + name: 'mv_a', + to: { database: 'default', name: 'mv_a_target' }, + as: 'SELECT id FROM default.source', + }) + const second = materializedView({ + database: 'default', + name: 'mv_b', + to: { database: 'default', name: 'mv_b_target' }, + as: 'SELECT id FROM default.source', + }) + + const plan = planDiff([], [second, first]) + const order = plan.operations + .filter((op) => op.type === 'create_materialized_view') + .map((op) => op.key) + + expect(order).toEqual([ + 'materialized_view:default.mv_a', + 'materialized_view:default.mv_b', + ]) + }) +}) diff --git a/packages/core/src/planner.ts b/packages/core/src/planner.ts index a24ff42..526240a 100644 --- a/packages/core/src/planner.ts +++ b/packages/core/src/planner.ts @@ -431,6 +431,49 @@ function diffTables(oldDef: TableDefinition, newDef: TableDefinition): TableDiff } } +/** + * Creation depth of each refreshable materialized view, derived from its + * `refresh.dependsOn` edges (#41). A view declared `DEPENDS ON other_mv` must + * be created AFTER `other_mv` exists, so ordering create operations by + * ascending depth puts every dependency before the views that depend on it. + * Names alone are unsafe: a dependent MV whose name sorts first would otherwise + * be created first and fail. + */ +function materializedViewCreationDepth( + definitions: SchemaDefinition[], + byKey: Map +): Map { + const depth = new Map() + const visiting = new Set() + + const compute = (key: string): number => { + const cached = depth.get(key) + if (cached !== undefined) return cached + // A dependency cycle has no valid creation order; stop recursing and let + // the alphabetical tiebreak apply rather than looping forever. + if (visiting.has(key)) return 0 + const def = byKey.get(key) + if (!def || def.kind !== 'materialized_view') { + depth.set(key, 0) + return 0 + } + visiting.add(key) + let result = 0 + for (const dep of def.refresh?.dependsOn ?? []) { + const depKey = `materialized_view:${dep.database}.${dep.name}` + if (byKey.has(depKey)) result = Math.max(result, 1 + compute(depKey)) + } + visiting.delete(key) + depth.set(key, result) + return result + } + + for (const def of definitions) { + if (def.kind === 'materialized_view') compute(definitionKey(def)) + } + return depth +} + export function planDiff(oldDefinitions: SchemaDefinition[], newDefinitions: SchemaDefinition[]): MigrationPlan { const oldCanonical = canonicalizeDefinitions(oldDefinitions) const newCanonical = canonicalizeDefinitions(newDefinitions) @@ -494,6 +537,7 @@ export function planDiff(oldDefinitions: SchemaDefinition[], newDefinitions: Sch pushCreateDatabaseOperation(operations, database, 'safe') } + const mvCreationDepth = materializedViewCreationDepth(newCanonical, newMap) operations.sort((a, b) => { const rank = (op: MigrationOperation): number => { if (op.type.startsWith('drop_')) return 0 @@ -506,6 +550,12 @@ export function planDiff(oldDefinitions: SchemaDefinition[], newDefinitions: Sch } const rankOrder = rank(a) - rank(b) if (rankOrder !== 0) return rankOrder + // Within materialized-view creates, order a DEPENDS ON target before the + // view that depends on it (#41), then fall back to a stable name order. + if (a.type === 'create_materialized_view' && b.type === 'create_materialized_view') { + const depthOrder = (mvCreationDepth.get(a.key) ?? 0) - (mvCreationDepth.get(b.key) ?? 0) + if (depthOrder !== 0) return depthOrder + } return a.key.localeCompare(b.key) })