perf(pg-delta): opt in to batched transactional apply - #468
Conversation
High-latency apply was one RTT per action. Callers can now batch a transactional segment into one simple-protocol query; the default stays one statement per query so we can fall back if attribution mis-fires.
🦋 Changeset detectedLatest commit: 38f0524 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38f05249f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| function stripTrailingSemicolons(sql: string): string { | ||
| return sql.replace(/;+\s*$/, ""); |
There was a problem hiding this comment.
Remove this regex rewrite from the executor: the batch path now strips terminal semicolons and whitespace from every planner-rendered action before sending it, so apply no longer replays the action SQL verbatim. The repository explicitly prohibits regex-transforming SQL in engine replay paths; append batching delimiters without altering the original statement text instead.
AGENTS.md reference: AGENTS.md:L74-L78
Useful? React with 👍 / 👎.
| const completed = completedBeforeError(error); | ||
| if (completed === 0) return 0; | ||
| if (completed < batchLength) return completed; | ||
| return batchLength; |
There was a problem hiding this comment.
When a batch action contains multiple top-level statements and a later error has no PostgreSQL position, completedBeforeError counts CommandComplete messages rather than entries in ranges. Returning that count directly advances past the actual action slot; for example, a two-statement action followed by a failing action and another queued action reports the latter action's index and SQL. Track how many completions belong to each action, isolate such actions, or otherwise avoid treating completion count as a batch index.
Useful? React with 👍 / 👎.
| function utf8Bytes(text: string): number { | ||
| return Buffer.byteLength(text, "utf8"); | ||
| } |
There was a problem hiding this comment.
Replace the unimported Buffer global with TextEncoder or another runtime-neutral byte counter. partitionByBatchBounds() reaches this code whenever batchTransactional is enabled, but Buffer is not a standard Deno global, so the newly advertised public option can throw ReferenceError: Buffer is not defined before submitting its first batch under Deno. The package explicitly promises that this library is importable in Deno as well as Bun and Node.
AGENTS.md reference: AGENTS.md:L53-L55
Useful? React with 👍 / 👎.
| for (let i = segment.start; i < segment.end; i++) { | ||
| const action = actions[i]!; | ||
| assertActionSqlBatchable(action.sql); |
There was a problem hiding this comment.
When the default path encounters an empty or transaction-control action, this assertion runs only after BEGIN has succeeded and is outside every rollback handler. It therefore rejects apply() but reaches the outer finally with destroyClient === false, releasing a client that is still inside the transaction back into the shared pool; the next borrower can inherit that transaction and its session state. Validate every action before opening a transaction, or catch this failure and issue ROLLBACK before releasing the client.
Useful? React with 👍 / 👎.
1 participant
Summary
batchTransactional/--batch-transactionalsends each transactional apply segment as bounded simple-protocol batches (BEGIN+ preamble + actions) so high-latency links stop paying one RTT per action. Default remains one query per statement so we can fall back if batching mis-attributes a failure.COMMITstays its own round trip (inDoubt= lost COMMIT only).POSITIONthrough join offsets (parse-time and multi-statement actions), otherwise CommandComplete count. The joiner puts;on its own line so a trailing--cannot swallow the next statement.apply.ts(lock-table probe + this executor).Linked issue
Linear CLI-2296
Refs #462
open-for-contributionlabel (or I'm a Supabase maintainer).Checklist
bunx changeset)bun run format-and-lintandbun run check-typespassTest plan
cd packages/pg-delta && bun test src/apply/apply.test.ts src/apply/batch-query.test.tscd packages/pg-delta && bun test tests/apply-roundtrips.test.ts tests/apply-batch-failure.test.ts tests/apply-observer.test.ts--batch-transactionalcollapses a 200-action segment to O(1) RTTs plusCOMMIT