MemberJunction / MemberJunction/MJ
SQLConverter: dedup mutates committed .pg.sql files; CTE-DELETE + sys.indexes-guard transpile gaps
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
Three `@memberjunction/sql-converter` (`mj migrate convert`) defects surfaced during the v5.48 PG conversion. The `/pg-migrate-experimental` runbook now *defends* against all three (see #3165), but the root fixes belong in the converter.
### 1. The dedup pass mutates COMMITTED `.pg.sql` files (highest priority)
Every `mj migrate convert` run ends with a global "EntityField Sequence deduplication" pass that rewrites `Sequence` values **inside already-committed `.pg.sql` files**, in place (prints `Fixed N sequence collision(s)` naming each file). Committed PG migrations are a deployed, immutable historical ledger — the converter must never touch them.
**Repro (v5.48):** converting `V202607142047__v5.48.x__Metadata_Sync.sql` reported 5 collisions and modified `V202604241700…`, `V202605190912…`, `V202605251204…`, `V202606051200…`, `V202606121723…` (5.30–5.41). The fresh-DB gate then applied cleanly *without* those edits — i.e. the dedup was spurious.
**Ask:** the dedup should only ever adjust sequences in the NEW migration being generated, never rewrite prior committed files. If a genuine cross-migration collision exists, fail loudly and point at the new migration.
### 2. CTE-targeted DELETE transpiles to invalid PG
`;WITH cte AS (...) DELETE FROM cte WHERE ...` (valid T-SQL) transpiles to `DELETE FROM "cte" ...`, which PG rejects (you can't DELETE from a CTE). Correct PG form is `DELETE FROM t USING () d WHERE t."ID" = d."ID" AND d.rn > 1`. Seen on `ListDetail_Index_Optimization` — had to hand-author the whole `.pg.sql`.
### 3. `IF EXISTS (SELECT 1 FROM sys.indexes …) BEGIN DROP INDEX … END` is dropped as an unhandled gap
The T-SQL idempotent-index-drop pattern lands as a `.needs-hand`/gap comment. The PG equivalent is trivial and could be templated: `DROP INDEX IF EXISTS __mj."IndexName";` (and `CREATE UNIQUE INDEX IF NOT EXISTS …`).
## Context
Discovered during the v5.48.0 release PG conversion. Runbook mitigations landed in #3165; this issue tracks the underlying `packages/SQLConverter` fixes (each warrants its own PR + regression test).
Contributor guide
Research direction
Start in packages/SQLConverter and reproduce the defects with mj migrate convert, using the /pg-migrate-experimental runbook and #3165 for the affected scenarios. Trace the deduplication pass and the CTE-DELETE and sys.indexes patterns, then add regression tests showing that only the new migration changes and that generated PostgreSQL is valid.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100