MemberJunction / MemberJunction/MJ

PostgreSQL counterpart of V202609031400 still carries the literal EntityField Sequences that #4378 removed from the T-SQL

Open
#4,385 0 comments 0 reactions 1 assignee Claimed by @SDesai-BC View on GitHub
bug priority: medium
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

Follow-up to #4378, deliberately left out of that PR. Filed so the dialects do not quietly diverge.

## What is divergent

#4378 replaced 12 literal `EntityField.Sequence` values in
`migrations/v6/V202609031400__v6.1.x__Conversation_Scoped_Skill_Activation.sql` with the apply-time expression CodeGen now emits:

```sql
(SELECT COALESCE(MAX([Sequence]), 0) + 1
FROM [${flyway:defaultSchema}].[EntityField]
WHERE [EntityID] = '')
```

Its counterpart, `migrations-pg/v6/V202609031400__v6.1.x__Conversation_Scoped_Skill_Activation.pg.sql`, **still has the literals** — e.g. sequences 1–8 in the `__mj."EntityField"` INSERTs around lines 291–340.

## Why it was not fixed in #4378

Three reasons, all still true:

1. **Nothing is blocked.** `changes.yml` filters to `^migrations/.*\.sql$`, so `migrations-pg/**` is not scanned. The release PR passed.
2. **Both dialects behave identically today.** The risk is latent in both — it needs a *second* migration touching the same entity — and neither has one. Verified on a from-scratch SQL Server build: `MJ: Conversation Skills` gets sequences `1..11` and `ActivationScope` gets 14, with or without the fix, because the repeatable renumber normalises either way.
3. **PG is converter territory.** `migrations/CLAUDE.md` is explicit that PG counterparts are deterministic transpilation run by the build engineer at release time, not hand-authored. That file is `--split --bake-codegen` output and says so at the top; regenerating it needs a live PG database seeded to the state immediately **before** this migration. Hand-editing 12 lines would have been exactly the drift the converter exists to prevent.

## What to do

Regenerate the counterpart with the converter as release-time toolchain work, not by hand:

```bash
mj migrate convert --file migrations/v6/V202609031400__v6.1.x__Conversation_Scoped_Skill_Activation.sql \
--split --bake-codegen
```

`--bake-codegen` needs `DB_PLATFORM=postgresql` + `PG_*` pointed at a live database seeded to the state just before this migration. Diff the result against the committed file before replacing it — the existing file carries baked CodeGen output whose header warns *"If the hand-written DDL above changes, re-run CodeGen and replace this entire generated section."*

Alternatively, fold it into the next release's PG conversion pass, which touches this folder anyway.

## Worth checking at the same time

The PG converter emitted literal sequences because the T-SQL it converted had them. Now that CodeGen emits the apply-time expression, confirm the converter translates that expression correctly to:

```sql
(SELECT COALESCE(MAX("Sequence"), 0) + 1 FROM __mj."EntityField" WHERE "EntityID" = '')
```

`V202609081111__Entity_SubtypeSelector.pg.sql` already contains one such expression (line 145), which suggests it does — but it is worth confirming rather than assuming, since every future migration will carry these.

## Acceptance

- `migrations-pg/v6/V202609031400__…pg.sql` carries apply-time expressions, not literals
- It was produced by the converter, not by hand
- `migrations/v6` ↔ `migrations-pg/v6` parity stays at 56/56

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.