MemberJunction / MemberJunction/MJ

CodeGen's EntityRelationship insert guard is keyed on a freshly-minted UUID, so its emitted SQL is unsafe to replay on another database

Open
#4,053 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

> **Corrected after filing.** The original text claimed CodeGen duplicates rows on its own re-runs. That is **not** true and I have rewritten this issue — CodeGen is idempotent against its own database. The real defect is narrower: the SQL it *emits* cannot protect itself when replayed elsewhere, which is exactly what Open Apps do with it.

## Summary

`ManageMetadataBase.buildInsertRelationshipSQL` emits each new `EntityRelationship` insert behind an `IF NOT EXISTS` guard keyed on a UUID **generated moments earlier in the same function call**. In CodeGen's own run that guard is redundant but harmless. In any *other* database the SQL is later replayed against, it is inert — it tests an id that host has never seen, so it can never match, and the insert fires unconditionally.

## Why CodeGen itself is fine

`manage-metadata.ts:3689-3712` reads the existing relationships live and only builds an insert when the pair has none:

```ts
const allRelationshipsResult = await this.runQuery(pool, sSQLRelationship);
const allRelationships = allRelationshipsResult.recordset;
...
const relationships = allRelationships.filter((r) =>
UUIDsEqual(r.EntityID, firstField.RelatedEntityID as string) &&
UUIDsEqual(r.RelatedEntityID, firstField.EntityID as string));

if (relationships.length === 0) {
for (const f of fkFields) batchSQL += this.buildInsertRelationshipSQL(f, md, relationshipCountMap);
} else {
batchSQL += this.buildEntityPairRelationshipSQL(relationships, fkFields, md, relationshipCountMap);
}
```

That upstream check is what prevents duplicates. **Running CodeGen repeatedly against one database does not duplicate relationships**, and this issue does not claim otherwise.

## The actual defect

`manage-metadata.ts:3769-3771`:

```ts
const newEntityRelationshipUUID = this.createNewUUID(); // minted on THIS run
const checkQuery = `SELECT 1 FROM ${this.qs(mj_core_schema(), 'EntityRelationship')} WHERE ${this.qi('ID')} = '${newEntityRelationshipUUID}'`;
const insertSQL = `INSERT INTO ${this.qs(mj_core_schema(), 'EntityRelationship')} (...) VALUES ('${newEntityRelationshipUUID}', ...)`;
```

wrapped by `SQLServerCodeGenProvider.conditionalInsertSQL` (`.../providers/sqlserver/SQLServerCodeGenProvider.ts:1446`) into `IF NOT EXISTS () BEGIN END`, giving output like:

```sql
/* Create Entity Relationship: MJ: Files -> : Form Uploads (One To Many via FileID) */
IF NOT EXISTS (SELECT 1 FROM [__mj].[EntityRelationship] WHERE [ID] = '87910a4f-de28-41c0-b9b2-8be59909cf70')
BEGIN
INSERT INTO [__mj].[EntityRelationship] ([ID], ...) VALUES ('87910a4f-de28-41c0-b9b2-8be59909cf70', ...)
END;
```

The guard asks *"has this exact row been inserted before"*. The question that makes an insert safe on a database you did not author against is *"does the relationship this describes already exist"* — under whatever id **that** host minted. Those coincide only where the id is shared, which replay is precisely the case that breaks.

So the guard is dead weight where it runs (CodeGen already knows the row is absent) and absent where it is needed (replay).

## Why this reaches shipped software

Open Apps paste CodeGen's SQL into their migrations — that is the documented workflow — so the ineffective guard ships and runs on installs and developer machines. A host that ran `mj codegen` before the migration has the relationship under its own id; the migration's guard misses and inserts a second row.

`__mj.EntityRelationship` has no unique constraint on `(EntityID, RelatedEntityID, RelatedEntityJoinField)`, so the duplicate lands silently. CodeGen then emits one `@FieldResolver` per row, so the *next* regeneration emits a duplicate identifier and the server package stops compiling (`TS2300` / `TS2393`), ending the run with `ERROR running one or more AFTER commands`. It is hard to attribute because the checked-in generated files predate the duplicate and still compile, so the break appears to belong to whatever branch happens to regenerate — we found it on a branch that changes no schema at all.

Audit of one app's shipped migrations: **51 statements** carry this ID-only guard, across 5 files, **4 of which are pasted CodeGen output**. The tables where it duplicates silently rather than failing loudly are exactly those without a natural-key unique constraint: `EntityRelationship`, `EntityFieldValue`, `EntitySetting`, `EntityPermission`. `Entity`, `EntityField` and `ApplicationEntity` do have one, which is why the same shape there fails loudly and never shipped broken.

## Suggested fix

Key the guard on the natural key so the emitted SQL is safe to replay:

```sql
IF NOT EXISTS (
SELECT 1 FROM [__mj].[EntityRelationship]
WHERE [EntityID] = '' AND [RelatedEntityID] = '' AND [RelatedEntityJoinField] = ''
)
```

`ManageMetadataBase` already emits the replay-safe shape for `EntityField` — `WHERE ID = '' OR (EntityID = ... AND Name = ...)` — so this is a consistency fix, not a new pattern.

Related, same change or nearby:

1. **Audit other emitted guards keyed on a just-minted `createNewUUID()`** — the same reasoning applies wherever generated SQL is expected to be replayable.
2. **Warn when two relationship rows resolve to one generated member name.** The identifier is built from the related entity plus the join field (not `Type`), so any two rows sharing that pair collide. Detecting it at generation turns a downstream `TS2300` into a message naming both rows.
3. Longer term, unique constraints on the natural keys of those four tables would make the class unrepresentable instead of leaving every writer to own idempotency by convention.

## Environment

MJ `6.1.0-edge.3`; emission site unchanged on `next` as of 2026-08-25.

Contributor guide

Open the contributing guide

Research direction

Start in manage-metadata.ts at lines 3689-3712 and 3769-3771, then read SQLServerCodeGenProvider.ts around line 1446 and compare the existing EntityField replay-safe guard. Change the emitted EntityRelationship guard to use its natural key rather than the freshly generated ID, and verify the generated SQL remains safe to replay against another database.

Written by the indexing model from the issue text.

Assessment

Tech stack
sql, typescript
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
57/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.