MemberJunction / MemberJunction/MJ
Batched writes for existing rows: BulkUpdate and BulkDelete
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## The gap
MJ has no batched write path for rows that already exist — neither **update** nor **delete**. Both go one row at a time: `BaseEntity.Save()` is roughly nine serialized statements (load, existence check, the CRUD sproc, record-change tracking), and `DatabaseProviderBase.Delete(entity, options, user)` takes a single entity. On a calibrated bench that lands at **264 rows/min @25ms RTT**, matching live measurement (~250/min).
The thing that looks like it already solves this doesn't. `TransactionGroup` provides **atomicity, not batching**: both `SQLServerTransactionGroup.HandleSubmit` and `PostgreSQLTransactionGroup` loop `await query(item.Instruction)` once per item inside a single BEGIN/COMMIT. Same N round trips. A live experiment that enabled a 500-record transaction on a sync measured **34% slower**, confirming commit batching is not where the cost is.
That misconception is already in the tree. `user-management.component.ts:900` — `bulkDeleteUsers()` — assigns a `TransactionGroup` to each entity and submits, which reads as a bulk delete and is in fact N round trips with rollback semantics attached.
#4027 adds `BulkCreate(entities[])` as a provider capability — default implementation on `DatabaseProviderBase` loops `Save()` so semantics are identical, with SQL Server and PostgreSQL overriding it with the set-based path. That closes the gap for inserts only.
| operation | capability | status |
| --- | --- | --- |
| INSERT | `BulkCreate(entities[])` | #4027 |
| UPDATE | — | **nothing** |
| DELETE | — | **nothing** |
| READ | `RunView` | already batched — see below |
**Reads are not part of this.** `RunView` already returns N rows in one round trip, including as hydrated entity objects, so a batched read needs no new capability — a caller wanting many records by key uses `RunView` with an `IN` filter rather than a `Load()` per record. Where per-record reads exist (for example the sync engine's match reads) that is a caller choosing `Load()`, not a missing provider surface.
## Why it matters beyond one caller
Any operation that rewrites or removes rows already in a table pays the per-row price with nothing to opt into. The one that surfaced this is `CustomColumnPromoter`: promoting a custom column adds an empty column by migration, then must move every existing row's value out of the staging JSON into it, one `Save()` at a time. #4044 had to cap that sweep at 1000 rows per pass purely because the write path is slow — a bound that exists to work around a missing capability, not because the work needs bounding.
The same shape appears anywhere a backfill, repair, re-baseline, retention purge, or bulk status change touches existing rows.
## Proposal
`BulkUpdate(entities[], contextUser?)` and `BulkDelete(entities[], options?, contextUser?)` on `DatabaseProviderBase`, mirroring #4027 exactly:
- **Default implementations loop `Save()` / `Delete()`.** Semantics are preserved, every caller is correct by default, and a provider that doesn't override loses nothing.
- **SQL Server and PostgreSQL override** with a single round trip. Both dialects support `UPDATE … FROM (VALUES …) AS v(…)` joined on the primary key, and a keyset `DELETE … WHERE (pk) IN (…)`; a temp-table join is the fallback for very wide sets or when the parameter cap bites.
- **Callers opt in explicitly**, as the sync engine does for `BulkCreate` — this is not a silent change to `Save()` or `Delete()`.
### What the fast path costs, stated up front
The speed does not come from batching stored-procedure calls. It comes from **not calling them**. `SQLServerBulkCreate` does not invoke `spCreate`; it builds a `sql.Table` and bulk-inserts straight into `[schema].[table]`, and its header names the price: *no stored-procedure side effects, no Record Changes rows, no per-entity save events*. `BulkUpdate` and `BulkDelete` inherit exactly that trade, and it must be documented on each method rather than discovered.
Two wrinkles specific to these two operations:
- **`spUpdate` stamps `__mj_UpdatedAt`.** A set-based `UPDATE … FROM (VALUES …)` that bypasses the sproc must set it explicitly, or audit columns silently stop moving on every bulk-updated row.
- **Cascade handling lives in `BaseEntity._InnerDelete`,** not in the provider. A batched delete either reimplements cascades or refuses when they apply — an explicit refusal, never a silent difference.
Both keep the #4027 shape: the default implementation loops `Save()` / `Delete()` so callers are correct unless they deliberately opt in, and an ineligible batch (mixed entities, unsaved records, missing primary key, cascades) falls back to that loop rather than doing something surprising.
For a caller like the promoter's backfill, losing record-change rows is arguably the *right* outcome — moving a value out of a staging column into its real column is not a user edit, and 50,000 audit rows for it are noise. That is still a choice to make explicitly.
## Follow-on
Once these exist, `CustomColumnPromoter`'s two full-table walks (`spreadAndRebaseline` and `purgeStaleOverflowKeys`, both via `forEachOverflowRow`) move onto `BulkUpdate` and `MAX_PURGE_ROWS_PER_PASS` can go away. `bulkDeleteUsers` and the other UI loops that currently use a `TransactionGroup` for this move onto `BulkDelete`.
Contributor guide
Research direction
Start with DatabaseProviderBase and the #4027 BulkCreate capability, then compare SQLServerBulkCreate, SQLServerTransactionGroup.HandleSubmit, PostgreSQLTransactionGroup, and BaseEntity._InnerDelete. Review the CustomColumnPromoter walks and user-management.component.ts:900 for caller requirements. Done means explicit BulkUpdate and BulkDelete behavior, provider fast paths or safe fallback loops, documented side effects, timestamp handling, and cascade eligibility.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, sql, typescript
- Domain
- backend-api-design, databases
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100