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

Open
#3,505 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.