fix: distinct warning for destructive table recreate (#23)

.changeset/recreate-warning.md

@@ -0,0 +1,5 @@
+---
+"chkit": patch
+---
+
+`chkit migrate` now flags a destructive table **recreate** (a `DROP TABLE` + `CREATE TABLE` caused by changing `engine`, `orderBy`, `primaryKey`, `partitionBy`, or `uniqueKey`) with a distinct `table_recreate_data_loss` warning instead of the generic `drop_table_data_loss`. The warning spells out that all rows are permanently deleted and the table is recreated empty, and recommends migrating via a temporary table to preserve data. Documented in the migrate and schema DSL reference pages.

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

@@ -53,6 +53,7 @@ Each destructive operation includes a warning code, reason, impact description,
 | Warning code | Operation |
 |-------------|-----------|
 | `drop_table_data_loss` | `DROP TABLE` |
+| `table_recreate_data_loss` | `DROP TABLE` + `CREATE TABLE` recreate from a structural change (`engine` / `orderBy` / `primaryKey` / `partitionBy` / `uniqueKey`) — all rows lost, table recreated empty |
 | `drop_column_irreversible` | `DROP COLUMN` |
 | `drop_view_dependency_break` | `DROP VIEW` / `DROP MATERIALIZED VIEW` |
 | `destructive_operation_review_required` | Other destructive operations |

apps/docs/src/content/docs/schema/dsl-reference.md

@@ -425,3 +425,7 @@ When a property changes, chkit determines whether the table can be altered in pl
 **Alterable** (ALTER in place): columns, indexes, projections, settings, TTL, comment
 
 Views and materialized views always use drop + recreate.
+
+:::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.
+:::

packages/cli/src/runtime/safety-markers.ts

@@ -98,12 +98,26 @@ export function extractMigrationOperationSummaries(sql: string): MigrationOperat
   return summaries
 }
 
-function describeDestructiveOperation(type: string): {
+function describeDestructiveOperation(
+  type: string,
+  opts: { recreate?: boolean } = {}
+): {
   warningCode: string
   reason: string
   impact: string
   recommendation: string
 } {
+  if (type === 'drop_table' && opts.recreate) {
+    return {
+      warningCode: 'table_recreate_data_loss',
+      reason:
+        'A change to engine, ORDER BY, PRIMARY KEY, PARTITION BY, or UNIQUE KEY can only be applied by dropping and recreating this table.',
+      impact:
+        'ALL ROWS are permanently deleted — the table is recreated empty and existing data is NOT copied over.',
+      recommendation:
+        'Back up the data first, or migrate via a temporary table (rename, INSERT ... SELECT, then drop) before approving.',
+    }
+  }
   if (type === 'drop_table') {
     return {
       warningCode: 'drop_table_data_loss',
@@ -141,12 +155,22 @@ export function collectDestructiveOperationMarkers(
   migration: string,
   sql: string
 ): DestructiveOperationMarker[] {
+  // A table recreate emits a drop_table + create_table for the SAME key in the
+  // same migration (an engine/ORDER BY/PRIMARY KEY/PARTITION BY/UNIQUE KEY
+  // change). Flag those drops with a louder, distinct warning than a plain drop
+  // so the user knows all rows are lost and the table is recreated empty (#23).
+  const createdTableKeys = new Set(
+    extractMigrationOperationSummaries(sql)
+      .filter((op) => op.type === 'create_table')
+      .map((op) => op.key)
+  )
   return extractDestructiveOperationSummaries(sql).map((summary) => {
     const parsed = parseOperationLine(summary, null)
     const type = parsed?.type ?? 'unknown'
     const key = parsed?.key ?? 'unknown'
     const risk = parsed?.risk ?? 'danger'
-    const detail = describeDestructiveOperation(type)
+    const recreate = type === 'drop_table' && createdTableKeys.has(key)
+    const detail = describeDestructiveOperation(type, { recreate })
     return {
       migration,
       type,

packages/cli/src/test/recreate-warning.e2e.test.ts

@@ -0,0 +1,97 @@
+import { mkdtemp, rm, writeFile } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+
+import { describe, expect, test } from 'bun:test'
+
+import {
+  CORE_ENTRY,
+  createJournalTableName,
+  createLiveExecutor,
+  createPrefix,
+  getRequiredEnv,
+  quoteIdent,
+  runCli,
+  waitForTable,
+} from './e2e-testkit.js'
+
+/**
+ * #23: a structural change (engine / ORDER BY / PRIMARY KEY / PARTITION BY /
+ * UNIQUE KEY) generates a DROP + CREATE that destroys all rows. The drop used
+ * to surface the same generic `drop_table_data_loss` warning as a deliberate
+ * drop. It now carries the distinct, louder `table_recreate_data_loss` warning.
+ * This run is BLOCKED (no --allow-destructive), so no data is actually dropped.
+ */
+describe('@chkit/cli table-recreate warning e2e (#23)', () => {
+  test('a structural change surfaces the distinct table_recreate_data_loss warning', async () => {
+    const liveEnv = getRequiredEnv()
+    const executor = createLiveExecutor(liveEnv)
+    const database = liveEnv.clickhouseDatabase
+    const journalTable = createJournalTableName('recreate')
+    const prefix = createPrefix('recreate')
+    const tableName = `${prefix}_t`
+    const cliEnv = { CHKIT_JOURNAL_TABLE: journalTable, CI: '1' }
+
+    const dir = await mkdtemp(join(tmpdir(), 'chkit-recreate-'))
+    const config = join(dir, 'clickhouse.config.ts')
+    const schemaPath = join(dir, 'schema.ts')
+
+    const renderSchema = (engine: string): string =>
+      `import { schema, table } from '${CORE_ENTRY}'\n\n` +
+      `export default schema(table({\n` +
+      `  database: '${database}',\n` +
+      `  name: '${tableName}',\n` +
+      `  columns: [{ name: 'id', type: 'UInt64' }],\n` +
+      `  engine: '${engine}',\n` +
+      `  primaryKey: ['id'],\n` +
+      `  orderBy: ['id'],\n` +
+      `}))\n`
+
+    try {
+      await writeFile(schemaPath, renderSchema('MergeTree()'), 'utf8')
+      await writeFile(
+        config,
+        `export default {\n` +
+          `  schema: '${schemaPath}',\n` +
+          `  outDir: '${join(dir, 'chkit')}',\n` +
+          `  migrationsDir: '${join(dir, 'chkit/migrations')}',\n` +
+          `  metaDir: '${join(dir, 'chkit/meta')}',\n` +
+          `  clickhouse: {\n` +
+          `    url: '${liveEnv.clickhouseUrl}',\n` +
+          `    username: '${liveEnv.clickhouseUser}',\n` +
+          `    password: '${liveEnv.clickhousePassword}',\n` +
+          `    database: '${database}',\n` +
+          `  },\n}\n`,
+        'utf8',
+      )
+
+      const genInit = runCli(dir, ['generate', '--config', config, '--name', 'init', '--migration-id', '20990101000000', '--json'], cliEnv)
+      expect(genInit.exitCode).toBe(0)
+      const migrateInit = runCli(dir, ['migrate', '--config', config, '--execute', '--json'], cliEnv)
+      expect(migrateInit.exitCode).toBe(0)
+      await waitForTable(executor, database, tableName)
+
+      // Structural change: engine MergeTree -> ReplacingMergeTree forces a
+      // DROP + CREATE recreate of the existing table.
+      await writeFile(schemaPath, renderSchema('ReplacingMergeTree()'), 'utf8')
+      const genRecreate = runCli(dir, ['generate', '--config', config, '--name', 'recreate', '--migration-id', '20990101000001', '--json'], cliEnv)
+      expect(genRecreate.exitCode).toBe(0)
+
+      // Apply in CI without --allow-destructive: blocked (exit 3), and the
+      // blocked operation must carry the distinct recreate warning.
+      const migrate = runCli(dir, ['migrate', '--config', config, '--execute', '--json'], cliEnv)
+      expect(migrate.exitCode).toBe(3)
+      const payload = JSON.parse(migrate.stdout) as {
+        destructiveOperations?: Array<{ type: string; warningCode: string }>
+      }
+      const warningCodes = (payload.destructiveOperations ?? []).map((op) => op.warningCode)
+      expect(warningCodes).toContain('table_recreate_data_loss')
+      expect(warningCodes).not.toContain('drop_table_data_loss')
+    } finally {
+      await rm(dir, { recursive: true, force: true })
+      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()
+    }
+  }, 180_000)
+})

packages/cli/src/test/runtime/safety-markers.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, test } from 'bun:test'
 
 import {
+  collectDestructiveOperationMarkers,
   collectUnmarkedDestructiveStatements,
   extractExecutableStatements,
   extractMigrationOperationSummaries,
@@ -203,3 +204,47 @@ describe('scanDestructiveSqlStatements (defense-in-depth for unmarked SQL)', ()
     expect(marker?.summary).toContain('DROP COLUMN')
   })
 })
+
+describe('collectDestructiveOperationMarkers table-recreate warning (#23)', () => {
+  test('a drop+create of the same table gets the distinct recreate warning', () => {
+    // What the planner emits for an engine/ORDER BY/PRIMARY KEY/PARTITION BY/
+    // UNIQUE KEY change: drop_table then create_table for the SAME key.
+    const sql = [
+      '-- operation: drop_table key=table:default.events risk=danger',
+      'DROP TABLE IF EXISTS default.events;',
+      '-- operation: create_table key=table:default.events risk=safe',
+      'CREATE TABLE default.events (id UInt64) ENGINE = MergeTree ORDER BY id;',
+    ].join('\n')
+
+    const markers = collectDestructiveOperationMarkers('20260101_recreate.sql', sql)
+    expect(markers).toHaveLength(1)
+    const marker = markers[0]
+    expect(marker?.type).toBe('drop_table')
+    expect(marker?.warningCode).toBe('table_recreate_data_loss')
+    expect(marker?.impact).toContain('ALL ROWS')
+  })
+
+  test('a plain drop (no matching create) keeps the generic drop warning', () => {
+    const sql = [
+      '-- operation: drop_table key=table:default.old_events risk=danger',
+      'DROP TABLE IF EXISTS default.old_events;',
+    ].join('\n')
+
+    const markers = collectDestructiveOperationMarkers('20260101_drop.sql', sql)
+    expect(markers).toHaveLength(1)
+    expect(markers[0]?.warningCode).toBe('drop_table_data_loss')
+  })
+
+  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',
+      'DROP TABLE IF EXISTS default.old_events;',
+      '-- operation: create_table key=table:default.new_events risk=safe',
+      'CREATE TABLE default.new_events (id UInt64) ENGINE = MergeTree ORDER BY id;',
+    ].join('\n')
+
+    const markers = collectDestructiveOperationMarkers('20260101_swap.sql', sql)
+    expect(markers).toHaveLength(1)
+    expect(markers[0]?.warningCode).toBe('drop_table_data_loss')
+  })
+})