MemberJunction / MemberJunction/MJ

Proposal: first-class auto-increment for numeric fields (seed/increment/gap-free), with PG parity

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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.