MemberJunction / MemberJunction/MJ
Proposal: first-class auto-increment for numeric fields (seed/increment/gap-free), with PG parity
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
cc @rkihm-bc for review · raised from @AN-BC
## Ask
Make auto-incrementing numeric values a **framework feature** rather than something each app hand-rolls: a declarable attribute on any numeric `EntityField` with configurable **starting value**, **increment interval**, and — critically — a **gap-free** option. CodeGen emits the right DDL and the entity layer assigns the value, on **both SQL Server and PostgreSQL**.
## Why
Every app that needs a human-facing sequential number is currently reinventing this. In bizapps-orders we needed gap-conscious document numbers for orders and payments (the order *is* the invoice, so auditors expect no gaps), and ended up hand-writing a counter table plus this:
```sql
DECLARE @seq TABLE (Seq INT);
UPDATE __mj_BizAppsOrders.OrderSequence WITH (UPDLOCK, HOLDLOCK)
SET NextSequenceNumber = NextSequenceNumber + 1
OUTPUT deleted.NextSequenceNumber INTO @seq(Seq)
WHERE ID = 1;
SELECT Seq FROM @seq;
```
Two things that pattern hides, which are exactly why this belongs in the framework:
1. **`UPDLOCK, HOLDLOCK` is not optional.** Without `UPDLOCK`, two concurrent readers take shared locks and deadlock on upgrade. Easy to get wrong, silent until you have concurrency.
2. **`OUTPUT ... INTO` is mandatory, not stylistic.** SQL Server rejects a *bare* `OUTPUT` clause on any table that has triggers — and MJ CodeGen puts an `__mj_UpdatedAt` trigger on **every** table. So the obvious `OUTPUT deleted.NextSequenceNumber` fails on every MJ schema. We hit this and had to diagnose it.
Anyone writing sequence logic in an MJ schema will hit both.
## Current state (checked, not assumed)
- **`EntityField.AutoIncrement: boolean` already exists** (`packages/MJCore/src/generic/entityInfo.ts:531`) — but it appears to only *record* that a column is IDENTITY. I found no code that acts on it beyond `IsSPParameter` tests, and no CodeGen path that emits IDENTITY *from* it.
- **PG conversion partly exists**: `CreateTableRule.convertConstraintsAndDefaults` maps `IDENTITY → GENERATED BY DEFAULT AS IDENTITY` (`packages/SQLConverter/src/rules/CreateTableRule.ts:329`).
### Bug in that conversion, worth fixing regardless of this proposal
The regex **discards the seed and increment**:
```js
sql = sql.replace(
/\bIDENTITY\s*\(\s*\d+\s*,\s*\d+\s*\)/gi,
'GENERATED BY DEFAULT AS IDENTITY'
);
```
`IDENTITY(1000, 5)` becomes a plain `GENERATED BY DEFAULT AS IDENTITY`, which starts at 1 and steps by 1. PG *can* express this — `GENERATED BY DEFAULT AS IDENTITY (START WITH 1000 INCREMENT BY 5)` — so the two dialects silently diverge today for any non-default seed/increment.
## Proposed shape
Extend the existing `AutoIncrement` attribute rather than adding a parallel concept:
| Attribute | Meaning |
|---|---|
| `AutoIncrement` | existing boolean, retained |
| `AutoIncrementSeed` | starting value (default 1) |
| `AutoIncrementStep` | increment interval (default 1) |
| `AutoIncrementStrategy` | `Identity` \| `Sequence` \| `GapFree` |
| `AutoIncrementScope` | optional field(s) the counter resets/partitions by (e.g. per-company, per-fiscal-year) |
**The strategies differ in a way that matters and should be an explicit choice, not a default:**
- **`Identity`** — SQL Server `IDENTITY(seed, step)` / PG `GENERATED ... (START WITH ... INCREMENT BY ...)`. Fastest. **Gaps on rollback** — the value is consumed before you know the row commits.
- **`Sequence`** — `CREATE SEQUENCE`. Portable, cached for speed, shareable across tables. **Gaps on restart** because of caching.
- **`GapFree`** — the counter-table pattern above. Serializes concurrent inserts on the counter row, so it is the slowest, but it is the *only* option that satisfies an auditor asking why invoice 1041 doesn't exist.
That trade-off is precisely why this shouldn't be one implicit behaviour: financial document numbers need `GapFree`, surrogate keys want `Identity`, and picking wrong is invisible until an audit or a load test.
`AutoIncrementScope` covers the very common real requirement of per-company or per-fiscal-year series (accounting already does this by hand for JE numbers: `JE-{CompanyCode}-{FY}-{seq}`).
## PostgreSQL parity — what needs doing
| Strategy | SQL Server | PostgreSQL |
|---|---|---|
| `Identity` | `IDENTITY(seed, step)` | `GENERATED BY DEFAULT AS IDENTITY (START WITH seed INCREMENT BY step)` — **converter must stop dropping seed/step** |
| `Sequence` | `CREATE SEQUENCE` + `NEXT VALUE FOR` | `CREATE SEQUENCE` + `nextval()` |
| `GapFree` | `UPDATE ... WITH (UPDLOCK, HOLDLOCK) ... OUTPUT ... INTO` | `UPDATE ... RETURNING` inside the transaction (PG's row lock on `UPDATE` gives the same serialization; no hint syntax, and no trigger restriction on `RETURNING`) |
The `GapFree` row differs enough between dialects that hand-porting it per app is exactly the kind of thing that will drift — a strong argument for generating it.
## Suggested scope
1. Fix the seed/increment loss in `CreateTableRule` (small, independent, worth doing now).
2. Add the metadata attributes + CodeGen DDL emission for `Identity` and `Sequence`.
3. Add `GapFree` with the generated helper, since that is the one with the sharp edges (`UPDLOCK/HOLDLOCK`, `OUTPUT ... INTO` vs `RETURNING`).
4. `AutoIncrementScope` last — it is the least common and the most design-sensitive.
Happy to contribute the bizapps-orders implementation as the reference for step 3; it is working and covered by live tests today.
Contributor guide
Research direction
Start with EntityField.AutoIncrement in packages/MJCore/src/generic/entityInfo.ts:531 and the conversion logic in packages/SQLConverter/src/rules/CreateTableRule.ts:329. Review the existing IsSPParameter tests and the proposed Identity, Sequence, and GapFree scope. Done means the agreed metadata and dialect-specific generation work, including preserving seed and increment values, is implemented for the selected scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, sql, typescript
- Domain
- backend-api-design, build-system, databases
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100