MemberJunction / MemberJunction/MJ
Generated spCreate* procedures are not idempotent on an explicit @ID — re-running an Open App seed migration fails with a PK violation
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
The generated `spCreate*` procedures insert unconditionally. When an explicit `@ID` is supplied, they run a plain `INSERT` with no existence guard:
```sql
IF @ID IS NOT NULL
BEGIN
INSERT INTO [__mj].[Action] ( [ID], ... )
VALUES ( @ID, ... )
END
```
(verified in `migrations/v5/B202607091514__v5.46.x__Baseline.sql`, `spCreateAction`; the same shape is generated for every entity)
Open Apps seed core-schema metadata this way — an app's migration does `EXEC [__mj].spCreateAction @ID = '', ...` for each row it ships. So **re-running a seed migration against rows that already exist raises a primary-key violation** and the migration fails there.
## Why this matters now
Open App removal cannot reach these rows automatically. They are free-floating core metadata — `Action`, `ActionParam`, `ActionResultCode`, `AIAgent`, `AIPrompt`, `Query`, `Template` and friends — with no foreign-key path back to the app's own entities, so the engine's FK-graph teardown never visits them, and dropping the app's schema doesn't touch a different schema. The declared escape hatch is a `migrations.teardownDirectory`, which **no app currently ships**.
The result is a reinstall trap:
1. App installs and seeds N core rows with fixed GUIDs.
2. Install fails partway, or the app is removed. Its schema and entity metadata are cleaned; the fixed-GUID rows survive.
3. Reinstall recreates the schema and re-runs the migrations from a fresh history table.
4. The seed migration re-executes `EXEC spCreateX @ID=''` → **PK violation → reinstall fails.**
Measured on a real v5.51.0 instance: an app seeding ~220 such rows left **215** behind after a clean `mj app remove` (entity metadata and schema were removed correctly — the control returned exactly to baseline). Those 215 are all primed to collide on reinstall.
MemberJunction/MJ#3469 makes this reachable in one additional situation. Under the previous `per-run` default a failed install rolled the whole migration set back, so nothing survived to collide; under `per-migration` an earlier seed migration commits before a later one fails. The window widens from "post-migration failures" to "any failure after the seed migration commits." The PR warns about it, but a warning is not a fix.
## Requested change
Make the explicit-`@ID` path of the generated `spCreate*` procedures idempotent, so a seed migration can be re-executed safely. Options, roughly in order of preference:
1. **Guard the insert** — `IF NOT EXISTS (SELECT 1 FROM WHERE ID = @ID) INSERT ...`, returning the existing row otherwise. Smallest change, preserves current semantics for the auto-ID path.
2. **`MERGE` on ID** — additionally brings existing rows up to the supplied values, which is arguably what a re-run of a seed migration should do.
3. Leave `spCreate*` alone and generate a separate `spEnsure*`/upsert that seed migrations use instead — no change to existing callers, but every app has to adopt it.
Option 1 is the least invasive and fixes the reported failure. Option 2 is better if the intent is that a seed migration is declarative.
Worth noting this only removes the *collision*; it does not stop the rows from remaining. Making removal complete is a separate problem — the engine has no record of what a migration wrote, so today it can only delete what it can infer (the entity tree) plus whatever an app volunteers via `teardownDirectory`. Durable options discussed on #3469: install-time footprint capture, or an owning-app column on the core tables apps may seed. Neither exists today — `OpenAppID` appears only on `OpenAppDependency` and `OpenAppInstallHistory`, never on `Action`/`AIAgent`/`Query`/`Template`.
## Related
- MemberJunction/MJ#3469 (the transaction-mode and compensation change that surfaced this)
- MemberJunction/MJ#3451 (root-cause analysis)
- Apps affected today: MemberJunction/bizapps-sonar#17… see bizapps-sonar#51, bizapps-committees#17, BlueCypress/SaaS#101
Contributor guide
Research direction
Start with migrations/v5/B202607091514__v5.46.x__Baseline.sql and inspect the generated spCreateAction procedure and its explicit-@ID branch; compare the same shape across the other spCreate* procedures. Trace where these procedures are generated and how seed migrations invoke them. Done means re-running a seed migration with an existing fixed ID no longer raises a primary-key violation while the auto-ID path retains its current behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- sql
- Domain
- database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100