Skip to content

Remove the PostgreSQL savepoint resource-owner depth limit - #66

Merged
andinux merged 4 commits into
mainfrom
codex/postgres-savepoint-depth
Sep 22, 2026
Merged

andinux merged 4 commits into
mainfrom
codex/postgres-savepoint-depth

Conversation

@marcobambini

@marcobambini marcobambini commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

Applying a payload read from a PostgreSQL table at 126 or more user savepoints could fail with buffer pin ... is not owned by resource owner SubTransaction. The fixed 128-entry owner/context array silently stopped recording Cloudsync's additional internal subtransactions.

Replace the array with a stack of frames allocated in TopTransactionContext, keyed by subtransaction ID. Commit and rollback restore the caller's resource owner and memory context from a local copy. Subtransaction callbacks remove completed or externally aborted frames, and transaction callbacks clear the stack. Snapshot replacement remains limited to the outermost Cloudsync savepoint. PostgreSQL's own resource limits still apply, but Cloudsync no longer imposes the fixed depth cap.

Validation

  • Full suites on PostgreSQL 15.19, 17.11 and 18.6: 546 checks pass on each.
  • New regression/stress test at depths 1, 125, 126, 127, 128, 256, 1024 and 2048, using a heap scan as payload input.
  • Rollback, replay in the same transaction and commit at every depth.
  • 100 caught trigger failures at depth 256, followed by successful apply in the same backend.
  • Negative control: restoring the original implementation reproduces the resource-owner error at depth 126.

The test is included in full_test.sql; docs/internal/deep-savepoints.md explains the defect, lifetime handling and results.

This is one of three independent follow-ups to #64, now based on main after #64 and #65 were squash-merged. The PostgreSQL test is numbered 63 (63_deep_savepoints.sql), since test 62 came in with #65. These tests run against local PostgreSQL instances, not a deployed cloud server.

The cloud-integration reliability commit this branch used to carry was identical to the one merged with #65, so it was dropped in the rebase.

Validation after the rebase

Full PostgreSQL suite with ON_ERROR_STOP=on: 553 checks pass on standalone PostgreSQL 17 and 551 on Supabase (test 39's lock-contention part skips there by design), including all 25 checks of test 63. CI run 35743888194 passed on the rebased head 5ac466a.

Test 63 now reports a failed apply as [FAIL] with its SQLSTATE and recovers at the user savepoint, instead of stopping full_test.sql under ON_ERROR_STOP=on. Negative control against main's build: depths 1 and 125 pass, and every apply and reapply from depth 126 up is reported [FAIL] … SQLSTATE XX000 while the suite continues. The final caught-errors section at depth 256 still crashes an assertion-enabled server there (TRAP: failed Assert("portal->portalSnapshot == NULL")), which no psql script can recover from; the changelog entry mentions this effect of the old code.

CHANGELOG.md has an [Unreleased] entry for the fix.

marcobambini and others added 2 commits September 22, 2026 08:53
Test 62 is now 62_deferred_fk_caller_commit.sql, merged with #65.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@andinux
andinux changed the base branch from pg-fixes11092026 to main September 22, 2026 14:56
@andinux
andinux force-pushed the codex/postgres-savepoint-depth branch from 6ae57f4 to 5ac466a Compare September 22, 2026 14:56
andinux and others added 2 commits September 22, 2026 09:28
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing the suite

Under ON_ERROR_STOP=on a failing apply stopped full_test.sql with no
[FAIL] line. Each depth now records the SQLSTATE, recovers at the user
savepoint and reports [FAIL] per check, so the rest of the suite runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@andinux
andinux merged commit b195a31 into main Sep 22, 2026
38 checks passed
@andinux
andinux deleted the codex/postgres-savepoint-depth branch September 22, 2026 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants