Conversation
Version shims for PG19: RepOriginId->ReplOriginId, the replorigin session globals folded into replorigin_xact_state, index_beginscan flags argument, ShmemInitHash losing max_size, AssertVariableIsOfType->StaticAssertVariableIsOfType, and isQueryUsingTempRelation->query_uses_temp_object.
Guard or redirect the call sites affected by PG19: 1. Exposed server_message_level_options/composite_to_json 2. Flattened ReorderBufferTXN xact_time, JsonbInState, RepackStmt (was ClusterStmt), SignalRecoveryConflictWithVirtualXID, pgstat_fetch_entry/ get_database_name relocations 3. The wait_event.h include dropped from pgstat.h.
Add PG19 to the pgver matrices of pg-stable-test, spockbench and zodan_sync. pg-stable-test builds from the tip of REL_19_STABLE, as it does for every other branch. The Docker-based workflows resolve the newest REL_19_* tag, which is currently REL_19_BETA2.
List 19 as supported, note the version-substitution caveat in getting started, and add a 6.0.0 release-note highlight. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fix call sites that exist only on the v5_STABLE line (or are compiled unconditionally here, unlike main) for PostgreSQL 19 API changes: - spock_apply_heap.c: ExecInsertIndexTuples() now takes a flags bitmask and reordered arguments. - spock_failover_slots.c: ReplicationSlotCreate() grew a "repack" argument; ReplicationSlot.active_pid became active_proc (add a SlotIsActive() wrapper); rename the local get_database_oid() copy to spock_get_database_oid() since PG19 exports the core function from catalog/pg_database.h. - spock.c: log_min_messages became a per-backend-type array; index it with MyBackendType in the emit-log hook. - spock_output.c: include spock_compat.h for the AssertVariableIsOfType -> StaticAssertVariableIsOfType rename. - compat/19: restore the SPKstandard_ProcessUtility / SPKnext_ProcessUtility_hook macros still used by v5's spock_executor.c (main dropped its call sites, so its compat/19 lacks them).
The version-dependent DO block in spock--5.0.0.sql raised 'Unsupported PostgreSQL major version 19' during CREATE EXTENSION; the PG15-18 definitions work unchanged on PG19.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesPostgreSQL 19 support was added to CI matrices, documentation, SQL version checks, compatibility interfaces, runtime APIs, replication behavior, and protocol serialization. PostgreSQL 19 support
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 5 |
| Duplication | -2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
mason-sharp
reviewed
Jul 31, 2026
| ## Spock 5.0.11 | ||
|
|
||
| ### New Features | ||
| * **PostgreSQL 19 support** — new `compat/19` layer and version-specific API |
Member
There was a problem hiding this comment.
Let's move this to a new "Unreleased" section instead of 5.0.11
mason-sharp
requested changes
Jul 31, 2026
Under spock.exception_behaviour = transdiscard or sub_disable, every action of a replay pass runs in a subtransaction that is rolled back unconditionally, so such a pass never applies a row. handle_commit() detected the "replayed but saw no exception" case only after it had already set replorigin_session_origin_lsn and called CommitTransactionCommand(). The replication origin therefore advanced past a remote transaction that was never applied, the following ERROR restarted the worker beyond it, and the transaction was lost -- while the log reported it as deliberately discarded. Move the check ahead of every finalisation step so that transaction is aborted instead of committed. The origin stays put, the provider retransmits, and clearing commit_lsn makes handle_begin() apply the retransmitted transaction in normal mode. Transactions skipped via spock.sub_alter_skiplsn are exempt, since there the absence of exceptions is intended and the origin must advance past them.
The two loops searching for this subscription's exception-log slot ran to i <= SpockCtx->total_workers, while the array holds exactly total_workers entries (see worker_shmem_size() and the memset in spock_worker_shmem_init). Every other loop over the array in the tree stops at < total_workers. Reading entry [total_workers] is outside the region that startup zeroes, so the slot_name compared there is whatever the adjacent shared memory happens to hold. Worse, an empty-looking name would be picked up as free_slot_index and namestrcpy() would then write past the end of the array. Nothing has been observed to break, because the region is allocated with worker_shmem_size() which over-allocates enough that the write lands in slack, but that is luck rather than design.
handle_begin() looked up this subscription's slot in the shared exception log with strncmp() bounded by strlen(MySubscription->name), which matches on any slot whose name merely starts with ours. Two subscriptions in a prefix relationship therefore share one slot: "test_subscription" matches the slot belonging to "test_subscription_parallel", and the shorter name wins because the comparison stops before the longer name's extra characters. Sharing the slot means sharing commit_lsn, and that is the marker handle_begin() uses to decide whether a transaction has already failed once. Subscriptions from the same provider are delivered the same remote transactions with the same commit LSNs, so a failure recorded by one subscription can make the other enter exception handling for a transaction that never failed -- which under transdiscard means the transaction is discarded. The initial_error_message and failed_action of the two subscriptions are mixed up as well. Compare with namestrcmp(), which is what the rest of the file uses for a NameData against a C string, and which is bounded by NAMEDATALEN. The comparison is now exact, so sub_name_len is no longer needed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/compat/19/spock_compat.h`:
- Around line 159-170: Update the PG19 compatibility definition of
isQueryUsingTempRelation so it detects only temporary relations, matching the
relation-only behavior of PG<=18 rather than all temporary objects returned by
query_uses_temp_object(). Preserve the existing DDL replication policy for
CREATE TABLE AS and add or update coverage for temporary types and typed
temporary tables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: edffe8d4-c16f-4b40-8206-246f2d6ca10d
📒 Files selected for processing (28)
.github/workflows/pg-stable-test.yml.github/workflows/spockbench.yml.github/workflows/zodan_sync.ymlREADME.mddocs/getting_started.mddocs/index.mddocs/spock_release_notes.mdinclude/spock.hinclude/spock_executor.hinclude/spock_jsonb_utils.hinclude/spock_output_plugin.hsql/spock--5.0.0.sqlsrc/compat/19/spock_compat.csrc/compat/19/spock_compat.hsrc/spock.csrc/spock_apply.csrc/spock_apply_heap.csrc/spock_conflict.csrc/spock_executor.csrc/spock_failover_slots.csrc/spock_functions.csrc/spock_manager.csrc/spock_output.csrc/spock_output_plugin.csrc/spock_proto_json.csrc/spock_proto_native.csrc/spock_readonly.csrc/spock_relcache.c
The TAP harness and tests 009/010 wrote "1MB", but the GUC is declared through DefineCustomIntVariable() with no GUC_UNIT_BYTE flag, so a unit suffix is not accepted: WARNING: invalid value for parameter "spock.exception_replay_queue_size": "1MB" The setting was therefore rejected on every major version, and every TAP test ran with the 4MB default instead of the intended 1MB -- so the tests meant to exercise replay queue overflow were not exercising it. Write a plain byte count instead. Adding GUC_UNIT_BYTE to the declaration would be a reasonable follow-up, since a bare number keeps its meaning under that flag, but that is a change to the GUC's contract rather than to its callers and is left alone here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
018_failover_slots.pl builds its standby under /tmp and removes the data directory at the end of each scenario, and CI uploads only tests/tap/logs. The spock_failover_slots worker runs on that standby and its messages appear nowhere else, so when slot synchronisation failed the artifacts said only that the slot had not appeared -- with no way to find out why after the fact. Copy the standby's log into the test log directory, both at the end of each scenario and at the moment the slot-presence check fails. The standby inherits logging_collector = on from the base backup, so the useful log is the one under pg_log/ while startup.log holds just the lines written before the collector took over; keep both, and tag them by scenario so the native run does not overwrite the worker run. This is what identified the cause of this test's PostgreSQL 19 failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ninfo
Without spock.failover_slots_dsn the primary's connection string is taken from
WalRcv->conninfo, and that field is empty for as long as a connection attempt is
in flight: WalReceiverMain() clears it under the walreceiver mutex before calling
walrcv_connect() and fills it in only once the connection is fully established.
The window therefore opens on every walreceiver start and restart, not merely
while the standby comes up.
Reading it then yielded a DSN with neither host nor port, libpq fell back to the
default local socket, and remote_connect() raised
ERROR: could not connect to the postgresql server: connection to server on
socket "/tmp/.s.PGSQL.5432" failed: No such file or directory
DETAIL: dsn was: dbname=postgres
That error propagates out of the worker's main loop and terminates it, and the
worker is registered with bgw_restart_time = 60 -- so no failover slot was
synchronized for the following minute. In 018_failover_slots.pl on PostgreSQL
19 the worker lost this race to the walreceiver by two milliseconds, the slot
appeared 65s after the standby started, and the test's 60s budget had expired
five seconds earlier. On 15 through 18 the same code passed only by winning the
race.
Have make_sync_failover_slots_dsn() report whether a connection string is
available rather than build an unusable one, and let both callers wait for the
next cycle instead. Since the walreceiver rewrites the field with its spinlock
held, copy it out under WalRcv->mutex too: an unlocked reader could otherwise
catch the memset() or strlcpy() in progress and build a DSN from a truncated
string.
This does not help when the primary requires a password.
walrcv_get_conninfo() obfuscates security-sensitive options, so what the
walreceiver publishes cannot be used to reconnect at all, and
spock.failover_slots_dsn has to be set. That is a separate defect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports #518 (PostgreSQL 19 support for main) onto the v5_STABLE line.
Cherry-picked from #518
Main-only files touched by Introduce support of Postgres 19 beta 1 to Spock #518 (spock_conflict_stat.c, spock_progress_recovery.c, 015_skip_lsn.pl and the installcheck/nightly workflows) do not exist on v5_STABLE and were dropped from the pick.
v5-specific adaptations (not in #518)
Code that main refactored away or compiles out on PG18+ still exists on v5 and needed its own PG19 fixes:
spock_apply_heap.c: ExecInsertIndexTuples() now takes a flags bitmask with reordered arguments.spock_failover_slots.c: ReplicationSlotCreate() grew a "repack" argument; ReplicationSlot.active_pid became active_proc (new SlotIsActive() wrapper); the local get_database_oid() copy is renamed to spock_get_database_oid() to avoid the core export PG19 added to catalog/pg_database.h.spock.c: log_min_messages became a per-backend-type array; the emit-log hook now indexes it with MyBackendType. (Note: main has the same comparison and currently compiles it as a pointer/int comparison on PG19 — separate fix needed there.)spock_output.c: include spock_compat.h for the AssertVariableIsOfType rename.compat/19: restore the SPKstandard_ProcessUtility / SPKnext_ProcessUtility_hook macros still used by v5's spock_executor.c.sql/spock--5.0.0.sql: accept major version 19 in the CREATE EXTENSION version gate.