From 7aa969892a6cc9b85e79134bd7543304e21b9f80 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 13 Jul 2026 20:43:52 -0700 Subject: [PATCH] fix: count actually-inserted rows in the SQLite -> PostgreSQL migrator via RETURNING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit insertBatch read the driver wrapper's count (result.count ?? result.rowCount ?? rows.length), which reported 0 through drizzle's execute even when every row landed — migration reports showed 'inserted 0' for fully-migrated tables and the startup banner's migratedRows total was wrong. ON CONFLICT DO NOTHING RETURNING 1 yields exactly one row per row actually inserted, making the count driver-agnostic and correctly excluding conflict-skipped rows. Idempotency test now asserts first-run insertedRows == sourceRows and re-run insertedRows == 0. Co-Authored-By: Claude Fable 5 --- .changeset/pg-migrator-camelcase-tables.md | 2 +- .../__tests__/postgres/sqlite-migrator.test.ts | 11 +++++++++++ packages/core/src/postgres/sqlite-migrator.ts | 16 +++++++++++++--- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/.changeset/pg-migrator-camelcase-tables.md b/.changeset/pg-migrator-camelcase-tables.md index c613641354..5bb8579533 100644 --- a/.changeset/pg-migrator-camelcase-tables.md +++ b/.changeset/pg-migrator-camelcase-tables.md @@ -4,4 +4,4 @@ summary: Fix SQLite → PostgreSQL migration silently skipping legacy camelCase tables. category: fix -dev: The migrator snake_cased column names but matched TABLE names verbatim, so all 22 legacy camelCase SQLite tables (activityLog, runAuditEvents, mergeQueue, taskClaims, projectNodePathMappings, …) found no PostgreSQL counterpart and were silently skipped — surfacing as "Project/node path mapping not found" on engine start. TablePlan now carries a snake_cased pgTable used for all PostgreSQL-side operations. Re-run `fn db migrate` (idempotent) to top up databases migrated before this fix. +dev: The migrator snake_cased column names but matched TABLE names verbatim, so all 22 legacy camelCase SQLite tables (activityLog, runAuditEvents, mergeQueue, taskClaims, projectNodePathMappings, …) found no PostgreSQL counterpart and were silently skipped — surfacing as "Project/node path mapping not found" on engine start. TablePlan now carries a snake_cased pgTable used for all PostgreSQL-side operations. Re-run `fn db migrate` (idempotent) to top up databases migrated before this fix. Migration reports also now count inserted rows via RETURNING (previously "inserted 0" even when every row landed). diff --git a/packages/core/src/__tests__/postgres/sqlite-migrator.test.ts b/packages/core/src/__tests__/postgres/sqlite-migrator.test.ts index e750ff4707..a419fcf40e 100644 --- a/packages/core/src/__tests__/postgres/sqlite-migrator.test.ts +++ b/packages/core/src/__tests__/postgres/sqlite-migrator.test.ts @@ -546,12 +546,23 @@ pgDescribe("SQLite-to-PostgreSQL migrator", () => { const first = await migrateSqliteToPostgres(ctx!.db, sources); const firstCounts = new Map(first.tables.map((t) => [`${t.schema}.${t.table}`, t.targetRows])); + // FNXC:PostgresMigration 2026-07-13-21:05: + // insertedRows must report rows ACTUALLY inserted (RETURNING-based count): + // every copied row on the first run, zero on the idempotent re-run. The + // old driver-wrapper count read 0 even when every row landed. + for (const t of first.tables) { + if (!t.skipped) { + expect(t.insertedRows, `${t.schema}.${t.table} first-run insertedRows`).toBe(t.sourceRows); + } + } + // Second run — should be a clean re-sync (ON CONFLICT DO NOTHING). const second = await migrateSqliteToPostgres(ctx!.db, sources); for (const t of second.tables) { const key = `${t.schema}.${t.table}`; expect(t.targetRows, `${key} row count should be unchanged on re-run`).toBe(firstCounts.get(key)); expect(t.verified, `${key} should still verify`).toBe(true); + expect(t.insertedRows, `${key} re-run should insert nothing`).toBe(0); } }); diff --git a/packages/core/src/postgres/sqlite-migrator.ts b/packages/core/src/postgres/sqlite-migrator.ts index 8617595cbf..99b1da7b6d 100644 --- a/packages/core/src/postgres/sqlite-migrator.ts +++ b/packages/core/src/postgres/sqlite-migrator.ts @@ -729,12 +729,22 @@ async function insertBatch( )})`, ); + /* + FNXC:PostgresMigration 2026-07-13-21:05: + RETURNING 1 makes the inserted-row count driver-agnostic: the result carries + exactly one row per row actually inserted (conflicts return nothing). The + previous `result.count ?? result.rowCount ?? rows.length` read whatever the + driver wrapper exposed and reported 0 even when every row landed, so + migration reports showed "inserted 0" for fully-migrated tables and the + startup banner's migratedRows total was wrong. + */ const query = sql`INSERT INTO ${sql.raw(schemaQualifiedTable)} (${sql.raw(colList)})${sql.raw(overridingClause)} VALUES ${sql.join(valueRowsBuilt, sql`, `)} - ON CONFLICT DO NOTHING`; + ON CONFLICT DO NOTHING + RETURNING 1`; - const result = (await db.execute(query)) as unknown as { count?: number; rowCount?: number }; - return Number(result?.count ?? result?.rowCount ?? rows.length); + const result = (await db.execute(query)) as unknown as { length?: number }; + return Number(result?.length ?? 0); } /** Count rows in a PostgreSQL table. */