From ad6ae52c49ac07b6ae2f8017b63ebd757b815d8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Mon, 20 Jul 2026 12:12:13 +0200 Subject: [PATCH 01/43] Move code to get_tables_to_repack_partitioned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of its code was pointlessly in its caller. This makes it better contained and clearer. Backpatch to 19, to avoid having two different copies in case we have to modify it again later. Author: Álvaro Herrera Reviewed-by: Bharath Rupireddy Reviewed-by: ChangAo Chen Discussion: https://postgr.es/m/alD9l-XlCuu3eUEe@alvherre.pgsql --- src/backend/commands/repack.c | 137 +++++++++++++++++++--------------- 1 file changed, 75 insertions(+), 62 deletions(-) diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 02883fe34a..dde56fb1e8 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -169,8 +169,8 @@ static void copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldInde MultiXactId *pCutoffMulti); static List *get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt); -static List *get_tables_to_repack_partitioned(RepackCommand cmd, - Oid relid, bool rel_is_index, +static List *get_tables_to_repack_partitioned(RepackStmt *stmt, + Relation rel, MemoryContext permcxt); static bool repack_is_permitted_for_relation(RepackCommand cmd, Oid relid, Oid userid); @@ -387,58 +387,8 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel) } else { - Oid relid; - bool rel_is_index; - - Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); - - /* - * If USING INDEX was specified, resolve the index name now and pass - * it down. - */ - if (stmt->usingindex) - { - /* - * If no index name was specified when repacking a partitioned - * table, punt for now. Maybe we can improve this later. - */ - if (!stmt->indexname) - { - if (stmt->command == REPACK_COMMAND_CLUSTER) - ereport(ERROR, - errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("there is no previously clustered index for table \"%s\"", - RelationGetRelationName(rel))); - else - ereport(ERROR, - errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - /*- translator: first %s is name of a SQL command, eg. REPACK */ - errmsg("cannot execute %s on partitioned table \"%s\" USING INDEX with no index name", - RepackCommandAsString(stmt->command), - RelationGetRelationName(rel))); - } - - relid = determine_clustered_index(rel, stmt->usingindex, - stmt->indexname); - if (!OidIsValid(relid)) - elog(ERROR, "unable to determine index to cluster on"); - check_index_is_clusterable(rel, relid, AccessExclusiveLock); - - rel_is_index = true; - } - else - { - relid = RelationGetRelid(rel); - rel_is_index = false; - } - - rtcs = get_tables_to_repack_partitioned(stmt->command, - relid, rel_is_index, - repack_context); - - /* close parent relation, releasing lock on it */ - table_close(rel, AccessExclusiveLock); - rel = NULL; + rtcs = get_tables_to_repack_partitioned(stmt, rel, repack_context); + rel = NULL; /* clobber no longer valid pointer */ } /* Commit to get out of starting transaction */ @@ -2255,19 +2205,71 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt) } /* - * Given a partitioned table or its index, return a list of RelToCluster for - * all the leaf child tables/indexes. + * Determine relations to process, when REPACK/CLUSTER is called with a + * partitioning table; that is, a list of its leaf partitions. That table has + * already been opened by caller and is passed as 'rel'. It is closed and + * unlocked here before return, so caller should clobber its pointer to avoid + * confusion. + * + * Return it as a list of RelToCluster. * - * 'rel_is_index' tells whether 'relid' is that of an index (true) or of the - * owning relation. + * XXX we don't support CONCURRENTLY for partitioned tables yet. */ static List * -get_tables_to_repack_partitioned(RepackCommand cmd, Oid relid, - bool rel_is_index, MemoryContext permcxt) +get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel, + MemoryContext permcxt) { + Oid relid; + bool rel_is_index; List *inhoids; List *rtcs = NIL; + Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); + Assert(CheckRelationLockedByMe(rel, AccessExclusiveLock, false)); + + /* + * We find the list of tables by looking for inheritors. If USING INDEX + * was given, look for inheritors of that index, whose name we resolve + * now. + * + * Otherwise we look for inheritors of the table itself. + */ + if (stmt->usingindex) + { + /* + * If no index name was specified when repacking a partitioned table, + * punt for now. Maybe we can improve this later. + */ + if (!stmt->indexname) + { + if (stmt->command == REPACK_COMMAND_CLUSTER) + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("there is no previously clustered index for table \"%s\"", + RelationGetRelationName(rel))); + else + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + /*- translator: first %s is name of a SQL command, eg. REPACK */ + errmsg("cannot execute %s on partitioned table \"%s\" USING INDEX with no index name", + RepackCommandAsString(stmt->command), + RelationGetRelationName(rel))); + } + + relid = determine_clustered_index(rel, stmt->usingindex, + stmt->indexname); + if (!OidIsValid(relid)) + elog(ERROR, "unable to determine index to cluster on"); + check_index_is_clusterable(rel, relid, AccessExclusiveLock); + + rel_is_index = true; + } + else + { + relid = RelationGetRelid(rel); + rel_is_index = false; + } + /* * Do not lock the children until they're processed. Note that we do hold * a lock on the parent partitioned table. @@ -2286,7 +2288,14 @@ get_tables_to_repack_partitioned(RepackCommand cmd, Oid relid, if (get_rel_relkind(child_oid) != RELKIND_INDEX) continue; - table_oid = IndexGetRelation(child_oid, false); + /* + * Although we do have a lock on some ancestor partitioned index, + * we may not have one on the immediate parent, so this lookup may + * still return invalid. + */ + table_oid = IndexGetRelation(child_oid, true); + if (!OidIsValid(table_oid)) + continue; index_oid = child_oid; } else @@ -2304,7 +2313,8 @@ get_tables_to_repack_partitioned(RepackCommand cmd, Oid relid, * leaf partition despite having them on the partitioned table. Skip * if so. */ - if (!repack_is_permitted_for_relation(cmd, table_oid, GetUserId())) + if (!repack_is_permitted_for_relation(stmt->command, table_oid, + GetUserId())) continue; /* Use a permanent memory context for the result list */ @@ -2316,6 +2326,9 @@ get_tables_to_repack_partitioned(RepackCommand cmd, Oid relid, MemoryContextSwitchTo(oldcxt); } + /* close parent relation, releasing lock on it */ + table_close(rel, AccessExclusiveLock); + return rtcs; } From c94409ebc3d7e9179d517736cbe9a948571f7c85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Mon, 20 Jul 2026 13:50:45 +0200 Subject: [PATCH 02/43] Fix LSN format in REPACK worker debug message Commit 6f6f284c7ee4 introduced use of LSN_FORMAT_ARGS across the whole tree to remove use of manual bit-shifting, and commit 2633dae2e487 changed the printf format to be %X/%08X; however commit 28d534e2ae0a violated both conventions by reintroducing the old manual-shift style with the deprecated %X/%X format in one debug message. Make that new message conform to our style. Author: kenny Backpatch-through: 19 Discussion: https://postgr.es/m/CAPXstDuWD8jg0=C8PXTXGSTTsZcjqJ+u+xKCrMpN99CXsxQzCg@mail.gmail.com --- src/backend/commands/repack_worker.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index db9ff057cc..af7e2a9476 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -397,8 +397,8 @@ decode_concurrent_changes(LogicalDecodingContext *ctx, { LogicalIncreaseRestartDecodingForSlot(end_lsn, end_lsn); LogicalConfirmReceivedLocation(end_lsn); - elog(DEBUG1, "REPACK: confirmed receive location %X/%X", - (uint32) (end_lsn >> 32), (uint32) end_lsn); + elog(DEBUG1, "REPACK: confirmed receive location %X/%08X", + LSN_FORMAT_ARGS(end_lsn)); repack_current_segment = segno_new; } } From 1009339b3acaf06b457a8182bf50b504847a643c Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Mon, 20 Jul 2026 21:40:31 +0900 Subject: [PATCH 03/43] Allow PostgreSQL::Test::Cluster::start() to pass postmaster options Previously, tests that needed extra postmaster command-line options had to invoke pg_ctl start directly, because PostgreSQL::Test::Cluster::start() provided no way to pass them. That bypassed the test framework's postmaster PID tracking, so a postmaster could be left running if the test failed after startup. Add an options parameter to PostgreSQL::Test::Cluster::start(), which is passed to pg_ctl's --options argument. This allows tests to use start() while preserving the framework's normal cleanup behavior. Author: Fujii Masao Reviewed-by: JoongHyuk Shin Discussion: https://postgr.es/m/CAHGQGwEpfE0CDUUODjBt7GO9U4ZF11hqga_Ci3wP8=O49oFKVw@mail.gmail.com --- src/test/perl/PostgreSQL/Test/Cluster.pm | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 529f49efee..3eae4cf628 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -1139,6 +1139,12 @@ Start the node and wait until it is ready to accept connections. By default, failure terminates the entire F invocation. If given, instead return a true or false value to indicate success or failure. +=item options => B + +Additional postmaster options passed as the value of pg_ctl's C<--options> +argument. This must be a single string, quoted as needed by the caller. +Do not specify C; it is set from the node name. + =back =cut @@ -1150,6 +1156,12 @@ sub start my $pgdata = $self->data_dir; my $name = $self->name; my $ret; + my $options = ""; + + $options .= "$params{options} " + if defined $params{options} && $params{options} ne ""; + + $options .= "--cluster-name=$name"; BAIL_OUT("node \"$name\" is already running") if defined $self->{_pid}; @@ -1168,7 +1180,7 @@ sub start 'pg_ctl', '--wait', '--pgdata' => $self->data_dir, '--log' => $self->logfile, - '--options' => "--cluster-name=$name", + '--options' => $options, 'start'); if ($ret != 0) From d5751c33cc3e842b20dbe67545fd2c850be8fa59 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Mon, 20 Jul 2026 21:41:14 +0900 Subject: [PATCH 04/43] Avoid ERROR in recovery target GUC assign hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recovery target parameters are postmaster-startup GUCs, but their assign hooks previously did more than assign individual parameter values. They also updated the global recoveryTarget state and raised ERROR if more than one recovery target appeared to be set. This was not a good fit for GUC assign hooks. Assign hooks should not throw ERROR, and deriving cross-parameter state while individual GUCs are still being assigned makes the result depend on assignment order rather than the final configuration. For example, setting one recovery target and then setting another recovery_target_* parameter to an empty string could clear recoveryTarget, causing recovery to proceed with no target even though a valid target remained configured. Fix this by having the assign hooks only store their own parameter values. The effective recoveryTarget is now derived once from the final recovery_target* settings in validateRecoveryParameters(), which also rejects configurations that specify more than one recovery target with FATAL. This preserves the expected behavior for repeated assignments of the same GUC, treats empty values as "not set", and removes cross-GUC validation from the assign hooks. Author: JoongHyuk Shin Reviewed-by: Greg Lamberson Reviewed-by: Michael Paquier Reviewed-by: Scott Ray Reviewed-by: Álvaro Herrera Reviewed-by: Kyotaro Horiguchi Reviewed-by: Henson Choi Reviewed-by: Zsolt Parragi Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CACSdjfPUa4UvKjADgOERXoxNYmCg2mqqiqKkiJk6mX6E4qgVFw@mail.gmail.com --- src/backend/access/transam/xlogrecovery.c | 140 +++++++------------- src/backend/utils/misc/guc_parameters.dat | 5 +- src/backend/utils/misc/guc_tables.c | 1 - src/include/access/xlogrecovery.h | 2 +- src/include/utils/guc_hooks.h | 3 - src/test/recovery/t/003_recovery_targets.pl | 123 +++++++++++++---- 6 files changed, 145 insertions(+), 129 deletions(-) diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index a9ebac2d0e..5f3b065b89 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -61,6 +61,7 @@ #include "storage/subsystems.h" #include "utils/datetime.h" #include "utils/fmgrprotos.h" +#include "utils/guc.h" #include "utils/guc_hooks.h" #include "utils/pgstat_internal.h" #include "utils/pg_lsn.h" @@ -92,7 +93,7 @@ int recoveryTargetAction = RECOVERY_TARGET_ACTION_PAUSE; TransactionId recoveryTargetXid; char *recovery_target_time_string; TimestampTz recoveryTargetTime; -const char *recoveryTargetName; +char *recoveryTargetName; XLogRecPtr recoveryTargetLSN; int recovery_min_apply_delay = 0; @@ -392,6 +393,7 @@ static bool HotStandbyActiveInReplay(void); static void SetCurrentChunkStartTime(TimestampTz xtime); static void SetLatestXTime(TimestampTz xtime); +static RecoveryTargetType DetermineRecoveryTargetType(void); /* * Register shared memory for WAL recovery @@ -1067,6 +1069,9 @@ readRecoverySignalFile(void) static void validateRecoveryParameters(void) { + /* Reject conflicting targets even when recovery was not requested */ + recoveryTarget = DetermineRecoveryTargetType(); + if (!ArchiveRecoveryRequested) return; @@ -4769,30 +4774,50 @@ check_primary_slot_name(char **newval, void **extra, GucSource source) } /* - * Recovery target settings: Only one of the several recovery_target* settings - * may be set. Setting a second one results in an error. The global variable - * recoveryTarget tracks which kind of recovery target was chosen. Other - * variables store the actual target value (for example a string or a xid). - * The assign functions of the parameters check whether a competing parameter - * was already set. But we want to allow setting the same parameter multiple - * times. We also want to allow unsetting a parameter and setting a different - * one, so we unset recoveryTarget when the parameter is set to an empty - * string. - * - * XXX this code is broken by design. Throwing an error from a GUC assign - * hook breaks fundamental assumptions of guc.c. So long as all the variables - * for which this can happen are PGC_POSTMASTER, the consequences are limited, - * since we'd just abort postmaster startup anyway. Nonetheless it's likely - * that we have odd behaviors such as unexpected GUC ordering dependencies. + * Return the recovery target derived from the recovery_target* settings, + * raising an error if more than one of them is set. */ - -pg_noreturn static void -error_multiple_recovery_targets(void) +static RecoveryTargetType +DetermineRecoveryTargetType(void) { - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("multiple recovery targets specified"), - errdetail("At most one of \"recovery_target\", \"recovery_target_lsn\", \"recovery_target_name\", \"recovery_target_time\", \"recovery_target_xid\" may be set."))); + int ntargets = 0; + RecoveryTargetType target = RECOVERY_TARGET_UNSET; + const char *val; + StringInfoData buf; + + initStringInfo(&buf); + +#define ADD_TARGET_IF_SET(gucname, kind) \ + do { \ + val = GetConfigOption(gucname, false, false); \ + if (val[0] != '\0') \ + { \ + ntargets++; \ + target = (kind); \ + if (buf.len == 0) \ + appendStringInfo(&buf, _("\"%s\""), gucname); \ + else \ + appendStringInfo(&buf, _(", \"%s\""), gucname); \ + } \ + } while (0) + + ADD_TARGET_IF_SET("recovery_target", RECOVERY_TARGET_IMMEDIATE); + ADD_TARGET_IF_SET("recovery_target_lsn", RECOVERY_TARGET_LSN); + ADD_TARGET_IF_SET("recovery_target_name", RECOVERY_TARGET_NAME); + ADD_TARGET_IF_SET("recovery_target_time", RECOVERY_TARGET_TIME); + ADD_TARGET_IF_SET("recovery_target_xid", RECOVERY_TARGET_XID); +#undef ADD_TARGET_IF_SET + + if (ntargets > 1) + ereport(FATAL, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("cannot specify more than one recovery target"), + errdetail("Parameters set are: %s.", + buf.data)); + + pfree(buf.data); + + return target; } /* @@ -4809,22 +4834,6 @@ check_recovery_target(char **newval, void **extra, GucSource source) return true; } -/* - * GUC assign_hook for recovery_target - */ -void -assign_recovery_target(const char *newval, void *extra) -{ - if (recoveryTarget != RECOVERY_TARGET_UNSET && - recoveryTarget != RECOVERY_TARGET_IMMEDIATE) - error_multiple_recovery_targets(); - - if (newval && strcmp(newval, "") != 0) - recoveryTarget = RECOVERY_TARGET_IMMEDIATE; - else - recoveryTarget = RECOVERY_TARGET_UNSET; -} - /* * GUC check_hook for recovery_target_lsn */ @@ -4856,17 +4865,8 @@ check_recovery_target_lsn(char **newval, void **extra, GucSource source) void assign_recovery_target_lsn(const char *newval, void *extra) { - if (recoveryTarget != RECOVERY_TARGET_UNSET && - recoveryTarget != RECOVERY_TARGET_LSN) - error_multiple_recovery_targets(); - if (newval && strcmp(newval, "") != 0) - { - recoveryTarget = RECOVERY_TARGET_LSN; recoveryTargetLSN = *((XLogRecPtr *) extra); - } - else - recoveryTarget = RECOVERY_TARGET_UNSET; } /* @@ -4885,25 +4885,6 @@ check_recovery_target_name(char **newval, void **extra, GucSource source) return true; } -/* - * GUC assign_hook for recovery_target_name - */ -void -assign_recovery_target_name(const char *newval, void *extra) -{ - if (recoveryTarget != RECOVERY_TARGET_UNSET && - recoveryTarget != RECOVERY_TARGET_NAME) - error_multiple_recovery_targets(); - - if (newval && strcmp(newval, "") != 0) - { - recoveryTarget = RECOVERY_TARGET_NAME; - recoveryTargetName = newval; - } - else - recoveryTarget = RECOVERY_TARGET_UNSET; -} - /* * GUC check_hook for recovery_target_time * @@ -4965,22 +4946,6 @@ check_recovery_target_time(char **newval, void **extra, GucSource source) return true; } -/* - * GUC assign_hook for recovery_target_time - */ -void -assign_recovery_target_time(const char *newval, void *extra) -{ - if (recoveryTarget != RECOVERY_TARGET_UNSET && - recoveryTarget != RECOVERY_TARGET_TIME) - error_multiple_recovery_targets(); - - if (newval && strcmp(newval, "") != 0) - recoveryTarget = RECOVERY_TARGET_TIME; - else - recoveryTarget = RECOVERY_TARGET_UNSET; -} - /* * GUC check_hook for recovery_target_timeline */ @@ -5099,15 +5064,6 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source) void assign_recovery_target_xid(const char *newval, void *extra) { - if (recoveryTarget != RECOVERY_TARGET_UNSET && - recoveryTarget != RECOVERY_TARGET_XID) - error_multiple_recovery_targets(); - if (newval && strcmp(newval, "") != 0) - { - recoveryTarget = RECOVERY_TARGET_XID; recoveryTargetXid = *((TransactionId *) extra); - } - else - recoveryTarget = RECOVERY_TARGET_UNSET; } diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index d421cdbde7..adb72361ce 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -2472,7 +2472,6 @@ variable => 'recovery_target_string', boot_val => '""', check_hook => 'check_recovery_target', - assign_hook => 'assign_recovery_target', }, { name => 'recovery_target_action', type => 'enum', context => 'PGC_POSTMASTER', group => 'WAL_RECOVERY_TARGET', @@ -2498,10 +2497,9 @@ { name => 'recovery_target_name', type => 'string', context => 'PGC_POSTMASTER', group => 'WAL_RECOVERY_TARGET', short_desc => 'Sets the named restore point up to which recovery will proceed.', - variable => 'recovery_target_name_string', + variable => 'recoveryTargetName', boot_val => '""', check_hook => 'check_recovery_target_name', - assign_hook => 'assign_recovery_target_name', }, { name => 'recovery_target_time', type => 'string', context => 'PGC_POSTMASTER', group => 'WAL_RECOVERY_TARGET', @@ -2509,7 +2507,6 @@ variable => 'recovery_target_time_string', boot_val => '""', check_hook => 'check_recovery_target_time', - assign_hook => 'assign_recovery_target_time', }, { name => 'recovery_target_timeline', type => 'string', context => 'PGC_POSTMASTER', group => 'WAL_RECOVERY_TARGET', diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 90aa374b3e..1ec460b6a8 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -667,7 +667,6 @@ static bool exec_backend_enabled = EXEC_BACKEND_ENABLED; static char *recovery_target_timeline_string; static char *recovery_target_string; static char *recovery_target_xid_string; -static char *recovery_target_name_string; static char *recovery_target_lsn_string; /* should be static, but commands/variable.c needs to get at this */ diff --git a/src/include/access/xlogrecovery.h b/src/include/access/xlogrecovery.h index ba7750dca0..9ffd44fcba 100644 --- a/src/include/access/xlogrecovery.h +++ b/src/include/access/xlogrecovery.h @@ -139,7 +139,7 @@ extern PGDLLIMPORT char *archiveCleanupCommand; extern PGDLLIMPORT TransactionId recoveryTargetXid; extern PGDLLIMPORT char *recovery_target_time_string; extern PGDLLIMPORT TimestampTz recoveryTargetTime; -extern PGDLLIMPORT const char *recoveryTargetName; +extern PGDLLIMPORT char *recoveryTargetName; extern PGDLLIMPORT XLogRecPtr recoveryTargetLSN; extern PGDLLIMPORT RecoveryTargetType recoveryTarget; extern PGDLLIMPORT bool wal_receiver_create_temp_slot; diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index 307f4fbaef..6a76f8d5ed 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -103,16 +103,13 @@ extern bool check_recovery_prefetch(int *new_value, void **extra, extern void assign_recovery_prefetch(int new_value, void *extra); extern bool check_recovery_target(char **newval, void **extra, GucSource source); -extern void assign_recovery_target(const char *newval, void *extra); extern bool check_recovery_target_lsn(char **newval, void **extra, GucSource source); extern void assign_recovery_target_lsn(const char *newval, void *extra); extern bool check_recovery_target_name(char **newval, void **extra, GucSource source); -extern void assign_recovery_target_name(const char *newval, void *extra); extern bool check_recovery_target_time(char **newval, void **extra, GucSource source); -extern void assign_recovery_target_time(const char *newval, void *extra); extern bool check_recovery_target_timeline(char **newval, void **extra, GucSource source); extern void assign_recovery_target_timeline(const char *newval, void *extra); diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl index 047eb13293..0469afb5bd 100644 --- a/src/test/recovery/t/003_recovery_targets.pl +++ b/src/test/recovery/t/003_recovery_targets.pl @@ -12,6 +12,10 @@ # Create and test a standby from given backup, with a certain recovery target. # Choose $until_lsn later than the transaction commit that causes the row # count to reach $num_rows, yet not later than the recovery target. +# If options is given, pass it through Cluster->start(options => ...). This is +# used to exercise scenarios that require the postmaster command line to receive +# multiple "-c name=value" instances of the same GUC, which postgresql.conf +# cannot express because ProcessConfigFile collapses duplicate keys. sub test_recovery_standby { local $Test::Builder::Level = $Test::Builder::Level + 1; @@ -22,6 +26,7 @@ sub test_recovery_standby my $recovery_params = shift; my $num_rows = shift; my $until_lsn = shift; + my %params = @_; my $node_standby = PostgreSQL::Test::Cluster->new($node_name); $node_standby->init_from_backup($node_primary, 'my_backup', @@ -32,7 +37,8 @@ sub test_recovery_standby $node_standby->append_conf('postgresql.conf', qq($param_item)); } - $node_standby->start; + $node_standby->start( + defined $params{options} ? (options => $params{options}) : ()); # Wait until standby has replayed enough data my $caughtup_query = @@ -108,6 +114,9 @@ sub test_recovery_standby # Force archiving of WAL file $node_primary->safe_psql('postgres', "SELECT pg_switch_wal()"); +my $lsn6 = + $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn()"); + # Test recovery targets my @recovery_params = ("recovery_target = 'immediate'"); test_recovery_standby('immediate target', @@ -125,11 +134,19 @@ sub test_recovery_standby test_recovery_standby('LSN', 'standby_5', $node_primary, \@recovery_params, "5000", $lsn5); +# Regression: empty-string for one recovery_target_* GUC must not clobber +# another non-empty target. Setting recovery_target_xid + recovery_target_time +# = '' must recover to the xid, not run as no-target recovery. +@recovery_params = + ("recovery_target_xid = '$recovery_txid'", "recovery_target_time = ''"); +test_recovery_standby('xid with empty time GUC', + 'standby_xid_empty_time', $node_primary, \@recovery_params, + "2000", $lsn2); + # Multiple targets # -# Multiple conflicting settings are not allowed, but setting the same -# parameter multiple times or unsetting a parameter and setting a -# different one is allowed. +# Multiple conflicting non-empty settings are rejected, but setting the same +# parameter twice or clearing one with an empty string is allowed. @recovery_params = ( "recovery_target_name = '$recovery_name'", @@ -138,31 +155,9 @@ sub test_recovery_standby test_recovery_standby('multiple overriding settings', 'standby_6', $node_primary, \@recovery_params, "3000", $lsn3); -my $node_standby = PostgreSQL::Test::Cluster->new('standby_7'); -$node_standby->init_from_backup($node_primary, 'my_backup', - has_restoring => 1); -$node_standby->append_conf( - 'postgresql.conf', "recovery_target_name = '$recovery_name' -recovery_target_time = '$recovery_time'"); - -my $res = run_log( - [ - 'pg_ctl', - '--pgdata' => $node_standby->data_dir, - '--log' => $node_standby->logfile, - 'start', - ]); -ok(!$res, 'invalid recovery startup fails'); - -my $logfile = slurp_file($node_standby->logfile()); -like( - $logfile, - qr/multiple recovery targets specified/, - 'multiple conflicting settings'); - # Check behavior when recovery ends before target is reached -$node_standby = PostgreSQL::Test::Cluster->new('standby_8'); +my $node_standby = PostgreSQL::Test::Cluster->new('standby_8'); $node_standby->init_from_backup( $node_primary, 'my_backup', has_restoring => 1, @@ -184,12 +179,84 @@ sub test_recovery_standby last if !-f $node_standby->data_dir . '/postmaster.pid'; usleep(100_000); } -$logfile = slurp_file($node_standby->logfile()); +my $logfile = slurp_file($node_standby->logfile()); like( $logfile, qr/FATAL: .* recovery ended before configured recovery target was reached/, 'recovery end before target reached is a fatal error'); +# Conflicts are rejected at every startup, even without recovery.signal. +# init_from_backup without has_restoring creates no recovery.signal, so this +# cluster would otherwise start as a plain primary; the conflict must still be +# caught. +my $node_no_signal = PostgreSQL::Test::Cluster->new('multi_target_no_signal'); +$node_no_signal->init_from_backup($node_primary, 'my_backup'); +$node_no_signal->append_conf( + 'postgresql.conf', "recovery_target_name = '$recovery_name' +recovery_target_time = '$recovery_time'"); + +ok( !$node_no_signal->start(fail_ok => 1), + 'server fails to start with conflicting recovery targets and no recovery.signal' +); + +my $logfile_no_signal = slurp_file($node_no_signal->logfile()); +like( + $logfile_no_signal, + qr/cannot specify more than one recovery target/, + 'expected error message logged without recovery.signal'); +like( + $logfile_no_signal, + qr/Parameters set are: "recovery_target_name", "recovery_target_time"/, + 'errdetail lists the set parameters in order without recovery.signal'); +unlike( + $logfile_no_signal, + qr/Parameters set are:[^\n]*=/, + 'errdetail does not echo parameter values without recovery.signal'); + +my $node_immediate_conflict = + PostgreSQL::Test::Cluster->new('immediate_target_conflict'); +$node_immediate_conflict->init_from_backup($node_primary, 'my_backup'); +$node_immediate_conflict->append_conf( + 'postgresql.conf', + "recovery_target = 'immediate' +recovery_target_xid = '$recovery_txid'"); + +ok( !$node_immediate_conflict->start(fail_ok => 1), + 'server fails to start with recovery_target=immediate and a second target' +); +like( + slurp_file($node_immediate_conflict->logfile()), + qr/cannot specify more than one recovery target/, + 'recovery_target=immediate conflicting with another target is rejected'); + +# Same-GUC set-then-clear: setting a recovery_target_* GUC and then setting the +# same GUC to an empty string leaves no target, so recovery runs to the end of +# WAL. Duplicate keys collapse in postgresql.conf, so "pg_ctl --options" passes +# both assignments on the postmaster command line. +test_recovery_standby( + 'recovery_target_xid set then cleared', + 'standby_xid_set_clear', + $node_primary, + [], + "6000", + $lsn6, + options => "-c recovery_target_xid=$recovery_txid -c recovery_target_xid=" +); + +# Set recovery_target_xid, then set and clear recovery_target_name. Only the +# xid remains, so recovery must stop at it rather than running to the end of WAL +# (a competing target that is set then cleared must not strand the first one). +test_recovery_standby( + 'recovery target preserved when a competing one is set then cleared', + 'standby_clobber_clear', + $node_primary, + [], + "2000", + $lsn2, + options => + "-c recovery_target_xid=$recovery_txid -c recovery_target_name=$recovery_name -c recovery_target_name=" +); + # Invalid recovery_target_timeline tests my ($result, $stdout, $stderr) = $node_primary->psql('postgres', "ALTER SYSTEM SET recovery_target_timeline TO 'bogus'"); From d049a31a4cd2d43739ae0039002c24c63a24d48c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Mon, 20 Jul 2026 17:21:20 +0200 Subject: [PATCH 05/43] Fix restore of partitions with exclusion constraints Commit 8c852ba9a4 allowed exclusion constraints to be added to partitioned tables, but wasn't careful to verify that pg_restore worked correctly for them. Fix that by making CompareIndexInfo() more selective about what needs to be rejected. Author: Japin Li Reported-by: Keith Paskett Discussion: https://postgr.es/m/2A40921D-83AB-411E-ADA6-7E509A46F1E4@logansw.com --- src/backend/catalog/index.c | 16 ++++++++++++++-- src/test/regress/expected/indexing.out | 15 +++++++++++++++ src/test/regress/sql/indexing.sql | 14 ++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 81bba4beac..31ef84d0a1 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2663,9 +2663,21 @@ CompareIndexInfo(const IndexInfo *info1, const IndexInfo *info2, return false; } - /* No support currently for comparing exclusion indexes. */ - if (info1->ii_ExclusionOps != NULL || info2->ii_ExclusionOps != NULL) + /* If they're exclusion indexes, their properties must be identical */ + if ((info1->ii_ExclusionOps == NULL) != (info2->ii_ExclusionOps == NULL)) return false; + if (info1->ii_ExclusionOps != NULL) + { + for (i = 0; i < info1->ii_NumIndexKeyAttrs; i++) + { + if (info1->ii_ExclusionOps[i] != info2->ii_ExclusionOps[i]) + return false; + if (info1->ii_ExclusionProcs[i] != info2->ii_ExclusionProcs[i]) + return false; + if (info1->ii_ExclusionStrats[i] != info2->ii_ExclusionStrats[i]) + return false; + } + } return true; } diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index 4d350fbc65..4a0a652e9f 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -1785,3 +1785,18 @@ insert into test_pg_wholerow_index values (2, 'addition', 0); drop index row_image_index; drop function row_image(test_pg_wholerow_index); drop table test_pg_wholerow_index; +-- Test of a partitioned index attach, when there are exclusion constraints. +create table idx_excl_part (a int4range, b int4range) partition by list (a); +create table idx_excl_part_1 (a int4range, b int4range); +alter table only idx_excl_part attach partition idx_excl_part_1 for values in ('[0,1)'::int4range); +alter table only idx_excl_part add constraint idxpart_id_data_excl exclude using gist (a with =, b with &&); +alter table idx_excl_part_1 add constraint idxpart_1_id_data_excl exclude using gist (a with &&, b with &&); +-- This should be disallowed, because the constraints don't match. +alter index idxpart_id_data_excl attach partition idxpart_1_id_data_excl; +ERROR: cannot attach index "idxpart_1_id_data_excl" as a partition of index "idxpart_id_data_excl" +DETAIL: The index definitions do not match. +-- but if we recreate the constraint differently, it's allowed: +alter table idx_excl_part_1 drop constraint idxpart_1_id_data_excl; +alter table idx_excl_part_1 add constraint idxpart_1_id_data_excl exclude using gist (a with =, b with &&); +alter index idxpart_id_data_excl attach partition idxpart_1_id_data_excl; +-- leave these tables around, for pg_upgrade testing diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index 561403cc7f..bbcfb36528 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -1005,3 +1005,17 @@ insert into test_pg_wholerow_index values (2, 'addition', 0); drop index row_image_index; drop function row_image(test_pg_wholerow_index); drop table test_pg_wholerow_index; + +-- Test of a partitioned index attach, when there are exclusion constraints. +create table idx_excl_part (a int4range, b int4range) partition by list (a); +create table idx_excl_part_1 (a int4range, b int4range); +alter table only idx_excl_part attach partition idx_excl_part_1 for values in ('[0,1)'::int4range); +alter table only idx_excl_part add constraint idxpart_id_data_excl exclude using gist (a with =, b with &&); +alter table idx_excl_part_1 add constraint idxpart_1_id_data_excl exclude using gist (a with &&, b with &&); +-- This should be disallowed, because the constraints don't match. +alter index idxpart_id_data_excl attach partition idxpart_1_id_data_excl; +-- but if we recreate the constraint differently, it's allowed: +alter table idx_excl_part_1 drop constraint idxpart_1_id_data_excl; +alter table idx_excl_part_1 add constraint idxpart_1_id_data_excl exclude using gist (a with =, b with &&); +alter index idxpart_id_data_excl attach partition idxpart_1_id_data_excl; +-- leave these tables around, for pg_upgrade testing From 17e805e8d8152db474bb368ab95cac127dfe6a4f Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 20 Jul 2026 13:36:39 -0400 Subject: [PATCH 06/43] doc: Granting TRIGGER or REFERENCES on table is dangerous. It's always been the case that granting these privileges to users that you don't fully trust was a bad idea, but it hasn't always been obvious to people reading the documentation that this is the case. To prevent confusion, and also repeated reports to pgsql-security, mention it explicitly. Discussion: http://postgr.es/m/CA+TgmobrjCHBuWHrvX3=2vndUCO2thUOdevrCcMDFW86cqCYvw@mail.gmail.com Reviewed-by: Nathan Bossart Backpatch-through: 14 --- doc/src/sgml/ddl.sgml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 0fcdabd787..5e8c270ab3 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2396,7 +2396,11 @@ REVOKE ALL ON accounts FROM PUBLIC; Allows creation of a foreign key constraint referencing a - table, or specific column(s) of a table. + table, or specific column(s) of a table. Great care should be taken when + granting this privilege, since a user who creates a foreign key can arrange + for enforcement of that foreign key to call an arbitrary function, such as + a cast function, and such functions will be called with the privileges of + the table owner. @@ -2405,7 +2409,9 @@ REVOKE ALL ON accounts FROM PUBLIC; TRIGGER - Allows creation of a trigger on a table, view, etc. + Allows creation of a trigger on a table, view, etc. Great care should be + taken when granting this privilege, since any triggers added to a table + or view will be executed with the privileges of users who modify it. From 83486b6a65aa48ed18e4e872d5e0fb5b0bfaca41 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Tue, 21 Jul 2026 03:24:58 +0900 Subject: [PATCH 07/43] Fix recovery target test waiting on unavailable WAL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Buildfarm member skink reported a failure in recovery/003_recovery_targets after commit d5751c33cc3. The newly added recovery_target_xid set-then-cleared test could time out while waiting for pg_last_wal_replay_lsn() to reach the expected LSN. The test recorded lsn6 after calling pg_switch_wal(). As a result, lsn6 pointed into the next WAL segment, but pg_switch_wal() only archived the previous one. Since the standby in this test restores WAL from the archive only, it could not obtain the segment containing lsn6 and waited indefinitely. Fix this by recording lsn6 before calling pg_switch_wal(), so the archived WAL contains the LSN that the standby is waiting for. Per buildfarm member skink. Reported-by: Álvaro Herrera Author: Fujii Masao Discussion: https://postgr.es/m/al5Y2mWffRs1NP34@alvherre.pgsql --- src/test/recovery/t/003_recovery_targets.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl index 0469afb5bd..db4a0ea74b 100644 --- a/src/test/recovery/t/003_recovery_targets.pl +++ b/src/test/recovery/t/003_recovery_targets.pl @@ -111,12 +111,12 @@ sub test_recovery_standby $node_primary->safe_psql('postgres', "INSERT INTO tab_int VALUES (generate_series(5001,6000))"); -# Force archiving of WAL file -$node_primary->safe_psql('postgres', "SELECT pg_switch_wal()"); - my $lsn6 = $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn()"); +# Force archiving of WAL file containing $lsn6 +$node_primary->safe_psql('postgres', "SELECT pg_switch_wal()"); + # Test recovery targets my @recovery_params = ("recovery_target = 'immediate'"); test_recovery_standby('immediate target', From 99e949f846e9d4c02296795ad041e55d88b1d320 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Mon, 20 Jul 2026 17:11:17 -0700 Subject: [PATCH 08/43] Add logical decoding status to pg_control_checkpoint(). Commit 8108765f04b added the logical decoding status to the pg_controldata output, but overlooked the pg_control_checkpoint() SQL function, which reports the same checkpoint information. This commit adds a logical_decoding column to pg_control_checkpoint(), placed after full_page_writes to match the pg_controldata output order. Oversight in 8108765f04b. Bump catalog version. Reported-by: Fujii Masao Discussion: https://postgr.es/m/CAHGQGwEkp1-1n5iC38+yHSNh955+KshwtCL6DzA0vk_vuUF_Eg@mail.gmail.com Backpatch-through: 19 --- doc/src/sgml/func/func-info.sgml | 5 ++++ src/backend/utils/misc/pg_controldata.c | 35 ++++++++++++++----------- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 6 ++--- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml index 69ef3857cf..122fc740f1 100644 --- a/doc/src/sgml/func/func-info.sgml +++ b/doc/src/sgml/func/func-info.sgml @@ -3436,6 +3436,11 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres} boolean + + logical_decoding + boolean + + next_xid text diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c index c6d9cbb157..d229ae3520 100644 --- a/src/backend/utils/misc/pg_controldata.c +++ b/src/backend/utils/misc/pg_controldata.c @@ -69,8 +69,8 @@ pg_control_system(PG_FUNCTION_ARGS) Datum pg_control_checkpoint(PG_FUNCTION_ARGS) { - Datum values[18]; - bool nulls[18]; + Datum values[19]; + bool nulls[19]; TupleDesc tupdesc; HeapTuple htup; ControlFileData *ControlFile; @@ -116,44 +116,47 @@ pg_control_checkpoint(PG_FUNCTION_ARGS) values[5] = BoolGetDatum(ControlFile->checkPointCopy.fullPageWrites); nulls[5] = false; - values[6] = CStringGetTextDatum(psprintf("%u:%u", - EpochFromFullTransactionId(ControlFile->checkPointCopy.nextXid), - XidFromFullTransactionId(ControlFile->checkPointCopy.nextXid))); + values[6] = BoolGetDatum(ControlFile->checkPointCopy.logicalDecodingEnabled); nulls[6] = false; - values[7] = ObjectIdGetDatum(ControlFile->checkPointCopy.nextOid); + values[7] = CStringGetTextDatum(psprintf("%u:%u", + EpochFromFullTransactionId(ControlFile->checkPointCopy.nextXid), + XidFromFullTransactionId(ControlFile->checkPointCopy.nextXid))); nulls[7] = false; - values[8] = TransactionIdGetDatum(ControlFile->checkPointCopy.nextMulti); + values[8] = ObjectIdGetDatum(ControlFile->checkPointCopy.nextOid); nulls[8] = false; - values[9] = TransactionIdGetDatum(ControlFile->checkPointCopy.nextMultiOffset); + values[9] = TransactionIdGetDatum(ControlFile->checkPointCopy.nextMulti); nulls[9] = false; - values[10] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestXid); + values[10] = TransactionIdGetDatum(ControlFile->checkPointCopy.nextMultiOffset); nulls[10] = false; - values[11] = ObjectIdGetDatum(ControlFile->checkPointCopy.oldestXidDB); + values[11] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestXid); nulls[11] = false; - values[12] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestActiveXid); + values[12] = ObjectIdGetDatum(ControlFile->checkPointCopy.oldestXidDB); nulls[12] = false; - values[13] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestMulti); + values[13] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestActiveXid); nulls[13] = false; - values[14] = ObjectIdGetDatum(ControlFile->checkPointCopy.oldestMultiDB); + values[14] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestMulti); nulls[14] = false; - values[15] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestCommitTsXid); + values[15] = ObjectIdGetDatum(ControlFile->checkPointCopy.oldestMultiDB); nulls[15] = false; - values[16] = TransactionIdGetDatum(ControlFile->checkPointCopy.newestCommitTsXid); + values[16] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestCommitTsXid); nulls[16] = false; - values[17] = TimestampTzGetDatum(time_t_to_timestamptz(ControlFile->checkPointCopy.time)); + values[17] = TransactionIdGetDatum(ControlFile->checkPointCopy.newestCommitTsXid); nulls[17] = false; + values[18] = TimestampTzGetDatum(time_t_to_timestamptz(ControlFile->checkPointCopy.time)); + nulls[18] = false; + htup = heap_form_tuple(tupdesc, values, nulls); PG_RETURN_DATUM(HeapTupleGetDatum(htup)); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index f046605ccf..d0399cc1cb 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202607173 +#define CATALOG_VERSION_NO 202607201 #endif diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 1c55a4dea3..f8a021987b 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -12371,9 +12371,9 @@ descr => 'pg_controldata checkpoint state information as a function', proname => 'pg_control_checkpoint', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{pg_lsn,pg_lsn,text,int4,int4,bool,text,oid,xid,xid,xid,oid,xid,xid,oid,xid,xid,timestamptz}', - proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{checkpoint_lsn,redo_lsn,redo_wal_file,timeline_id,prev_timeline_id,full_page_writes,next_xid,next_oid,next_multixact_id,next_multi_offset,oldest_xid,oldest_xid_dbid,oldest_active_xid,oldest_multi_xid,oldest_multi_dbid,oldest_commit_ts_xid,newest_commit_ts_xid,checkpoint_time}', + proallargtypes => '{pg_lsn,pg_lsn,text,int4,int4,bool,bool,text,oid,xid,xid,xid,oid,xid,xid,oid,xid,xid,timestamptz}', + proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{checkpoint_lsn,redo_lsn,redo_wal_file,timeline_id,prev_timeline_id,full_page_writes,logical_decoding,next_xid,next_oid,next_multixact_id,next_multi_offset,oldest_xid,oldest_xid_dbid,oldest_active_xid,oldest_multi_xid,oldest_multi_dbid,oldest_commit_ts_xid,newest_commit_ts_xid,checkpoint_time}', prosrc => 'pg_control_checkpoint' }, { oid => '3443', From d774576f6f05f65e3a944eb509ef0620ea6b107e Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Tue, 21 Jul 2026 09:23:11 +0530 Subject: [PATCH 09/43] Allow logical replication workers to ignore default_transaction_read_only. Sequence synchronization updates sequence state via setval(), which explicitly calls PreventCommandIfReadOnly(). If default_transaction_read_only is enabled on the subscriber, this causes sequencesync workers to fail with "cannot execute setval() in a read-only transaction". Apply and tablesync workers are not affected, since they write via direct heap access rather than through these read-only-checked functions. Rather than special-casing sequencesync, override default_transaction_read_only to "off" for all logical replication workers in InitializeLogRepWorker(), the same way session_replication_role and search_path are already forced there. This keeps the initialization uniform. For PG-19, we kept the fix narrow by overriding default_transaction_read_only to "off" only for sequencesync workers. Reported-by: Noah Misch Author: vignesh C Reviewed-by: Amit Kapila Backpatch-through: 19 Discussion: https://postgr.es/m/20260710045217.f0.noahmisch@microsoft.com --- src/backend/replication/logical/worker.c | 8 ++++ src/test/subscription/t/036_sequences.pl | 51 ++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 7799266c61..0ff5cef63c 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -5809,6 +5809,14 @@ InitializeLogRepWorker(void) */ SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE); + /* + * Ignore default_transaction_read_only for logical replication workers, + * as they need to be able to modify subscriber-side state regardless of + * that setting. + */ + SetConfigOption("default_transaction_read_only", "off", PGC_SUSET, + PGC_S_OVERRIDE); + ApplyContext = AllocSetContextCreate(TopMemoryContext, "ApplyContext", ALLOCSET_DEFAULT_SIZES); diff --git a/src/test/subscription/t/036_sequences.pl b/src/test/subscription/t/036_sequences.pl index 77ac9386cd..dd6fa515df 100644 --- a/src/test/subscription/t/036_sequences.pl +++ b/src/test/subscription/t/036_sequences.pl @@ -188,6 +188,57 @@ 'REFRESH PUBLICATION will not sync newly published sequence with copy_data as false' ); +########## +# Ensure that ALTER SUBSCRIPTION ... REFRESH SEQUENCES can still update +# sequence values and mark the sequence as ready even when +# default_transaction_read_only is enabled on the subscriber. +########## + +$node_subscriber->safe_psql( + 'postgres', qq( + ALTER SYSTEM SET default_transaction_read_only = on; + SELECT pg_reload_conf(); +)); + +# Update the existing sequence 'regress_s3' on the publisher +$node_publisher->safe_psql( + 'postgres', qq( + INSERT INTO regress_seq_test SELECT nextval('regress_s3') FROM generate_series(1,100); +)); + +$node_subscriber->safe_psql( + 'postgres', qq( + set default_transaction_read_only = off; + ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES; +)); +$node_subscriber->poll_query_until('postgres', $synced_query) + or die "Timed out while waiting for subscriber to synchronize data"; + +# Check - sequence value is updated despite default_transaction_read_only +# being enabled on the subscriber +$result = $node_subscriber->safe_psql( + 'postgres', qq( + SELECT last_value, is_called FROM regress_s3; +)); +is($result, '200|t', + 'REFRESH SEQUENCES updates sequence value with default_transaction_read_only enabled' +); + +# Check - sequence is marked as ready ('r') +$result = $node_subscriber->safe_psql( + 'postgres', qq( + SELECT srsubstate FROM pg_subscription_rel WHERE srrelid = 'regress_s3'::regclass; +)); +is($result, 'r', + 'sequence is marked as ready after REFRESH SEQUENCES with default_transaction_read_only enabled' +); + +$node_subscriber->safe_psql( + 'postgres', qq( + ALTER SYSTEM SET default_transaction_read_only = off; + SELECT pg_reload_conf(); +)); + ########## # A sequence dropped concurrently on the publisher, while the sequencesync # worker's batch query is executing, must be treated the same as any other From 9170c8b71693cf76a4b9a9fbcd16b1fb20b3357c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 21 Jul 2026 08:33:25 +0200 Subject: [PATCH 10/43] Test what BEFORE UPDATE triggers do to FOR PORTION OF If a BEFORE trigger changes NEW.valid_at, what is the interaction with FOR PORTION OF? This commit gives a test to capture our current behavior: The trigger's change replaces the value we computed automatically, but it does not change the bounds of the temporal leftovers. This matches the behavior of MariaDB. On the other hand, DB2 rejects changing the start/end columns of a PERIOD. Since we don't have PERIODs, we can't reject the change at trigger definition time as DB2 does, but we could reject it at run time by comparing the values before and after running triggers. Author: Paul A. Jungwirth Discussion: https://www.postgresql.org/message-id/CA%2BrenyV3Cr9BvWsPeb1t8b%3DPk24apuzyGbubAEs_YsgLUTfXpg%40mail.gmail.com --- src/test/regress/expected/for_portion_of.out | 47 ++++++++++++++++++++ src/test/regress/sql/for_portion_of.sql | 47 ++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index 271282c2d3..0e217f104e 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -1843,6 +1843,53 @@ SELECT * FROM for_portion_of_test ORDER BY valid_at; DROP FUNCTION fpo_append_name_suffix CASCADE; NOTICE: drop cascades to trigger fpo_before_insert_row on table for_portion_of_test DROP TABLE for_portion_of_test; +-- A BEFORE UPDATE trigger that changes the application-time column is allowed, +-- even if the results are senseless. +-- Note this is likely to cause a primary key violation. +CREATE TABLE for_portion_of_test ( + id int4range, + valid_at daterange, + name text +); +CREATE FUNCTION trg_fpo_change_valid_at() +RETURNS TRIGGER LANGUAGE plpgsql AS +$$ +BEGIN + NEW.valid_at = daterange('2018-01-01', '2019-01-01'); + RETURN NEW; +END; +$$; +CREATE TRIGGER fpo_before_update_row + BEFORE UPDATE ON for_portion_of_test + FOR EACH ROW EXECUTE PROCEDURE trg_fpo_change_valid_at(); +INSERT INTO for_portion_of_test VALUES ('[1,2)', '[2010-01-01,2020-01-01)', 'foo'); +UPDATE for_portion_of_test + FOR PORTION OF valid_at FROM '2018-05-01' TO '2018-06-01' + SET name = CONCAT(name, '!') + WHERE id = '[1,2)'; +SELECT * FROM for_portion_of_test ORDER BY id, valid_at; + id | valid_at | name +-------+-------------------------+------ + [1,2) | [2010-01-01,2018-05-01) | foo + [1,2) | [2018-01-01,2019-01-01) | foo! + [1,2) | [2018-06-01,2020-01-01) | foo +(3 rows) + +-- A primary key should reject anything invalid: +TRUNCATE for_portion_of_test; +ALTER TABLE for_portion_of_test + ADD CONSTRAINT for_portion_of_test_key + PRIMARY KEY (id, valid_at WITHOUT OVERLAPS); +INSERT INTO for_portion_of_test VALUES ('[1,2)', '[2010-01-01,2020-01-01)', 'foo'); +UPDATE for_portion_of_test + FOR PORTION OF valid_at FROM '2018-05-01' TO '2018-06-01' + SET name = CONCAT(name, '!') + WHERE id = '[1,2)'; +ERROR: conflicting key value violates exclusion constraint "for_portion_of_test_key" +DETAIL: Key (id, valid_at)=([1,2), [2010-01-01,2018-05-01)) conflicts with existing key (id, valid_at)=([1,2), [2018-01-01,2019-01-01)). +DROP TRIGGER fpo_before_update_row ON for_portion_of_test; +DROP FUNCTION trg_fpo_change_valid_at(); +DROP TABLE for_portion_of_test; -- Test with multiranges CREATE TABLE for_portion_of_test2 ( id int4range NOT NULL, diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index f48644347d..a8d29a76b2 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1215,6 +1215,53 @@ SELECT * FROM for_portion_of_test ORDER BY valid_at; DROP FUNCTION fpo_append_name_suffix CASCADE; DROP TABLE for_portion_of_test; +-- A BEFORE UPDATE trigger that changes the application-time column is allowed, +-- even if the results are senseless. +-- Note this is likely to cause a primary key violation. + +CREATE TABLE for_portion_of_test ( + id int4range, + valid_at daterange, + name text +); + +CREATE FUNCTION trg_fpo_change_valid_at() +RETURNS TRIGGER LANGUAGE plpgsql AS +$$ +BEGIN + NEW.valid_at = daterange('2018-01-01', '2019-01-01'); + RETURN NEW; +END; +$$; + +CREATE TRIGGER fpo_before_update_row + BEFORE UPDATE ON for_portion_of_test + FOR EACH ROW EXECUTE PROCEDURE trg_fpo_change_valid_at(); + +INSERT INTO for_portion_of_test VALUES ('[1,2)', '[2010-01-01,2020-01-01)', 'foo'); + +UPDATE for_portion_of_test + FOR PORTION OF valid_at FROM '2018-05-01' TO '2018-06-01' + SET name = CONCAT(name, '!') + WHERE id = '[1,2)'; + +SELECT * FROM for_portion_of_test ORDER BY id, valid_at; + +-- A primary key should reject anything invalid: +TRUNCATE for_portion_of_test; +ALTER TABLE for_portion_of_test + ADD CONSTRAINT for_portion_of_test_key + PRIMARY KEY (id, valid_at WITHOUT OVERLAPS); +INSERT INTO for_portion_of_test VALUES ('[1,2)', '[2010-01-01,2020-01-01)', 'foo'); +UPDATE for_portion_of_test + FOR PORTION OF valid_at FROM '2018-05-01' TO '2018-06-01' + SET name = CONCAT(name, '!') + WHERE id = '[1,2)'; + +DROP TRIGGER fpo_before_update_row ON for_portion_of_test; +DROP FUNCTION trg_fpo_change_valid_at(); +DROP TABLE for_portion_of_test; + -- Test with multiranges CREATE TABLE for_portion_of_test2 ( From fc6425fbe7a1fc7458fc27060ec78c5d98757690 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 21 Jul 2026 16:50:18 +0900 Subject: [PATCH 11/43] Improve generate_partition_qual()'s cache handling on out-of-memory errors An in-flight failure when trying to set rd_partcheckcxt or rd_partcheck, while for example doing an allocation in copyObject(), would leave a backend cache in a corrupted state. The operations are now ordered so as we avoid a leak in the cache memory context and a semi-filled cache state when an allocation failure happens. This is unlikely going to be hit in practice. Like the other improvements of this kind, no backpatch is done. Reported-by: Alexander Lakhin Author: Matthias van de Meent Discussion: https://postgr.es/m/95c64dc2-3abe-4f4e-b285-4c681f565d9f@gmail.com --- src/backend/utils/cache/partcache.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c index 3107075c9a..a098267488 100644 --- a/src/backend/utils/cache/partcache.c +++ b/src/backend/utils/cache/partcache.c @@ -411,14 +411,31 @@ generate_partition_qual(Relation rel) */ if (result != NIL) { - rel->rd_partcheckcxt = AllocSetContextCreate(CacheMemoryContext, - "partition constraint", - ALLOCSET_SMALL_SIZES); - MemoryContextCopyAndSetIdentifier(rel->rd_partcheckcxt, + /* + * Take care to order operations so that allocation errors don't leave + * the catcache in an invalid state; first allocate everything into a + * transactional context, then associate it with CacheContext and + * update the relation data. This also avoids leaking memory if we + * ever hit OOM here. + */ + List *partcheck; + MemoryContext partctx; + + partctx = AllocSetContextCreate(CurrentMemoryContext, + "partition constraint", + ALLOCSET_SMALL_SIZES); + MemoryContextCopyAndSetIdentifier(partctx, RelationGetRelationName(rel)); - oldcxt = MemoryContextSwitchTo(rel->rd_partcheckcxt); - rel->rd_partcheck = copyObject(result); + + oldcxt = MemoryContextSwitchTo(partctx); + partcheck = copyObject(result); MemoryContextSwitchTo(oldcxt); + + /* finally, link the allocations and memctx into the right places */ + MemoryContextSetParent(partctx, CacheMemoryContext); + + rel->rd_partcheckcxt = partctx; + rel->rd_partcheck = partcheck; } else rel->rd_partcheck = NIL; From c90c9678e533e622a393d3296f8fe4b9e5e317af Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 21 Jul 2026 15:27:02 +0200 Subject: [PATCH 12/43] Message style fixes Change DETAIL messages to conform to the style guide by capitalizing the first word of sentences and ending sentences with a period. Author: Peter Smith Reviewed-by: Chao Li Reviewed-by: vignesh C Reviewed-by: Xiaopeng Wang Reviewed-by: Peter Eisentraut Discussion: https://www.postgresql.org/message-id/flat/CAHut%2BPszSntkUgN%2BQa9matGY6MLEoFGSuVbuKDgnnTdZ7YPRwg%40mail.gmail.com --- contrib/dblink/dblink.c | 2 +- contrib/passwordcheck/expected/passwordcheck.out | 2 +- contrib/passwordcheck/expected/passwordcheck_1.out | 2 +- contrib/passwordcheck/passwordcheck.c | 2 +- contrib/pg_stash_advice/expected/pg_stash_advice.out | 2 +- .../pg_stash_advice/expected/pg_stash_advice_utf8.out | 2 +- contrib/pg_stash_advice/pg_stash_advice.c | 8 ++++---- contrib/postgres_fdw/expected/postgres_fdw.out | 2 +- src/backend/commands/copyto.c | 2 +- src/backend/commands/extension.c | 2 +- src/backend/commands/tablecmds.c | 10 +++++----- src/backend/libpq/be-secure-openssl.c | 2 +- .../test_extensions/expected/test_extensions.out | 2 +- src/test/regress/expected/create_view.out | 2 +- src/test/regress/expected/rangefuncs.out | 2 +- 15 files changed, 22 insertions(+), 22 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index 9e42a64241..9613f88198 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -2714,7 +2714,7 @@ dblink_security_check(PGconn *conn, const char *connname, const char *connstr) ereport(ERROR, (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED), errmsg("password or GSSAPI delegated credentials required"), - errdetail("Non-superusers may only connect using credentials they provide, eg: password in connection string or delegated GSSAPI credentials"), + errdetail("Non-superusers may only connect using credentials they provide, eg: password in connection string or delegated GSSAPI credentials."), errhint("Ensure provided credentials match target server's authentication method."))); } diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out index 83472c76d2..9d02129e93 100644 --- a/contrib/passwordcheck/expected/passwordcheck.out +++ b/contrib/passwordcheck/expected/passwordcheck.out @@ -6,7 +6,7 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password'; -- error: too short ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt'; ERROR: password is too short -DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long +DETAIL: Password must be at least "passwordcheck.min_password_length" (8) bytes long. -- ok SET passwordcheck.min_password_length = 6; ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt'; diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out index fb12ec45cc..a334720431 100644 --- a/contrib/passwordcheck/expected/passwordcheck_1.out +++ b/contrib/passwordcheck/expected/passwordcheck_1.out @@ -6,7 +6,7 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password'; -- error: too short ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt'; ERROR: password is too short -DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long +DETAIL: Password must be at least "passwordcheck.min_password_length" (8) bytes long. -- ok SET passwordcheck.min_password_length = 6; ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt'; diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c index 13fd5c976a..b45187cce9 100644 --- a/contrib/passwordcheck/passwordcheck.c +++ b/contrib/passwordcheck/passwordcheck.c @@ -101,7 +101,7 @@ check_password(const char *username, ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("password is too short"), - errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long", + errdetail("Password must be at least \"passwordcheck.min_password_length\" (%d) bytes long.", min_password_length))); /* check if the password contains the username */ diff --git a/contrib/pg_stash_advice/expected/pg_stash_advice.out b/contrib/pg_stash_advice/expected/pg_stash_advice.out index 788da854aa..8c24a21295 100644 --- a/contrib/pg_stash_advice/expected/pg_stash_advice.out +++ b/contrib/pg_stash_advice/expected/pg_stash_advice.out @@ -315,7 +315,7 @@ SELECT pg_create_advice_stash(' '); ERROR: advice stash name must begin with a letter or underscore and contain only letters, digits, and underscores SET pg_stash_advice.stash_name = '99bottles'; ERROR: invalid value for parameter "pg_stash_advice.stash_name": "99bottles" -DETAIL: advice stash name must begin with a letter or underscore and contain only letters, digits, and underscores +DETAIL: Advice stash name must begin with a letter or underscore and contain only letters, digits, and underscores. -- Clean up state in dynamic shared memory. SELECT pg_drop_advice_stash('regress_stash'); pg_drop_advice_stash diff --git a/contrib/pg_stash_advice/expected/pg_stash_advice_utf8.out b/contrib/pg_stash_advice/expected/pg_stash_advice_utf8.out index 7c532571ed..c4bc93c8ef 100644 --- a/contrib/pg_stash_advice/expected/pg_stash_advice_utf8.out +++ b/contrib/pg_stash_advice/expected/pg_stash_advice_utf8.out @@ -13,4 +13,4 @@ SELECT pg_create_advice_stash('café'); ERROR: advice stash name must not contain non-ASCII characters SET pg_stash_advice.stash_name = 'café'; ERROR: invalid value for parameter "pg_stash_advice.stash_name": "café" -DETAIL: advice stash name must not contain non-ASCII characters +DETAIL: Advice stash name must not contain non-ASCII characters. diff --git a/contrib/pg_stash_advice/pg_stash_advice.c b/contrib/pg_stash_advice/pg_stash_advice.c index 777ff37459..79048329f6 100644 --- a/contrib/pg_stash_advice/pg_stash_advice.c +++ b/contrib/pg_stash_advice/pg_stash_advice.c @@ -388,7 +388,7 @@ pgsa_check_stash_name_guc(char **newval, void **extra, GucSource source) if (strlen(stash_name) + 1 > NAMEDATALEN) { GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE); - GUC_check_errdetail("advice stash names may not be longer than %d bytes", + GUC_check_errdetail("Advice stash names may not be longer than %d bytes.", NAMEDATALEN - 1); return false; } @@ -400,7 +400,7 @@ pgsa_check_stash_name_guc(char **newval, void **extra, GucSource source) if (!pg_is_ascii(stash_name)) { GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE); - GUC_check_errdetail("advice stash name must not contain non-ASCII characters"); + GUC_check_errdetail("Advice stash name must not contain non-ASCII characters."); return false; } @@ -412,7 +412,7 @@ pgsa_check_stash_name_guc(char **newval, void **extra, GucSource source) if (!pgsa_is_identifier(stash_name)) { GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE); - GUC_check_errdetail("advice stash name must begin with a letter or underscore and contain only letters, digits, and underscores"); + GUC_check_errdetail("Advice stash name must begin with a letter or underscore and contain only letters, digits, and underscores."); return false; } @@ -701,7 +701,7 @@ pgsa_set_advice_string(char *stash_name, int64 queryId, char *advice_string) ereport(ERROR, errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), - errdetail("could not insert advice string into shared hash table")); + errdetail("Could not insert advice string into shared hash table.")); } /* Update the entry and release the lock. */ diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 9303de98b6..d19121b05d 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -11839,7 +11839,7 @@ DELETE FROM result_tbl; -- Test COPY TO when foreign table is partition COPY async_pt TO stdout; --error ERROR: cannot copy from foreign table "async_p1" -DETAIL: Partition "async_p1" is a foreign table in partitioned table "async_pt" +DETAIL: Partition "async_p1" is a foreign table in partitioned table "async_pt". HINT: Try the COPY (SELECT ...) TO variant. DROP FOREIGN TABLE async_p3; DROP TABLE base_tbl3; diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index f9bc617ddb..b0bdfb5810 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -856,7 +856,7 @@ BeginCopyTo(ParseState *pstate, ereport(ERROR, errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot copy from foreign table \"%s\"", relation_name), - errdetail("Partition \"%s\" is a foreign table in partitioned table \"%s\"", + errdetail("Partition \"%s\" is a foreign table in partitioned table \"%s\".", relation_name, RelationGetRelationName(rel)), errhint("Try the COPY (SELECT ...) TO variant.")); } diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index d073585c42..ae03f20c34 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -3433,7 +3433,7 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("extension \"%s\" does not support SET SCHEMA", NameStr(extForm->extname)), - errdetail("%s is not in the extension's schema \"%s\"", + errdetail("%s is not in the extension's schema \"%s\".", getObjectDescription(&dep, false), get_namespace_name(oldNspOid)))); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index cb93c3e935..6d4c457b82 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -15664,7 +15664,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used by a function or procedure"), - errdetail("%s depends on column \"%s\"", + errdetail("%s depends on column \"%s\".", getObjectDescription(&foundObject, false), colName))); break; @@ -15679,7 +15679,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used by a view or rule"), - errdetail("%s depends on column \"%s\"", + errdetail("%s depends on column \"%s\".", getObjectDescription(&foundObject, false), colName))); break; @@ -15699,7 +15699,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used in a trigger definition"), - errdetail("%s depends on column \"%s\"", + errdetail("%s depends on column \"%s\".", getObjectDescription(&foundObject, false), colName))); break; @@ -15718,7 +15718,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used in a policy definition"), - errdetail("%s depends on column \"%s\"", + errdetail("%s depends on column \"%s\".", getObjectDescription(&foundObject, false), colName))); break; @@ -15777,7 +15777,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used by a publication WHERE clause"), - errdetail("%s depends on column \"%s\"", + errdetail("%s depends on column \"%s\".", getObjectDescription(&foundObject, false), colName))); break; diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 3674f3cd5d..6a99a3d7f9 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -471,7 +471,7 @@ be_tls_init(bool isServerStart) ereport(isServerStart ? FATAL : LOG, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("could not set SSL protocol version range"), - errdetail("\"%s\" cannot be higher than \"%s\"", + errdetail("\"%s\" cannot be higher than \"%s\".", "ssl_min_protocol_version", "ssl_max_protocol_version"))); goto error; diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out index fdae52d6ab..1b5debdeeb 100644 --- a/src/test/modules/test_extensions/expected/test_extensions.out +++ b/src/test/modules/test_extensions/expected/test_extensions.out @@ -566,7 +566,7 @@ SELECT pg_describe_object(classid, objid, objsubid) as obj, -- fails, as function dep_req1 is not in the same schema as the extension. ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_func_dep3; ERROR: extension "test_ext_req_schema1" does not support SET SCHEMA -DETAIL: function test_func_dep2.dep_req1() is not in the extension's schema "test_func_dep1" +DETAIL: function test_func_dep2.dep_req1() is not in the extension's schema "test_func_dep1". -- Move back the function, and the extension can be moved. ALTER FUNCTION test_func_dep2.dep_req1() SET SCHEMA test_func_dep1; ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_func_dep3; diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index 63cf4b4371..053fa56573 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -1720,7 +1720,7 @@ rollback; -- likewise, altering a referenced column's type is prohibited ... alter table tt14t alter column f4 type integer using f4::integer; -- fail ERROR: cannot alter type of a column used by a view or rule -DETAIL: rule _RETURN on view tt14v depends on column "f4" +DETAIL: rule _RETURN on view tt14v depends on column "f4". -- ... but some bug might let it happen, so check defenses begin; -- destroy the dependency entry that prevents the ALTER: diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out index 5cc94011e9..a7cb1b5611 100644 --- a/src/test/regress/expected/rangefuncs.out +++ b/src/test/regress/expected/rangefuncs.out @@ -2279,7 +2279,7 @@ ERROR: attribute 5 of type record has been dropped rollback; alter table users alter column seq type numeric; -- fail, view has reference ERROR: cannot alter type of a column used by a view or rule -DETAIL: rule _RETURN on view usersview depends on column "seq" +DETAIL: rule _RETURN on view usersview depends on column "seq". -- likewise, check we don't crash if the dependency goes wrong begin; -- destroy the dependency entry that prevents the ALTER: From 078aac2ed649f983e52f039ded9014bf6447963e Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 21 Jul 2026 16:56:37 +0200 Subject: [PATCH 13/43] pg_upgrade: Message wording fix For internally consistent terminology --- src/bin/pg_upgrade/controldata.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c index 02ea02df60..b3bd4ccde8 100644 --- a/src/bin/pg_upgrade/controldata.c +++ b/src/bin/pg_upgrade/controldata.c @@ -659,7 +659,7 @@ check_control_data(ControlData *oldctrl, * data checksums, before retrying. */ if (oldctrl->data_checksum_version > PG_DATA_CHECKSUM_VERSION) - pg_fatal("checksums are being enabled in the old cluster"); + pg_fatal("data checksums are being enabled in the old cluster"); /* * We might eventually allow upgrades from checksum to no-checksum From 02597b42e5ff0df44e6a089801b7f6830ad19d83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Tue, 21 Jul 2026 13:27:15 +0200 Subject: [PATCH 14/43] Unify error messages --- src/backend/catalog/catalog.c | 4 ++-- src/backend/utils/adt/tsvector_op.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index be8791af87..cf9b88b3e2 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -717,8 +717,8 @@ pg_nextoid(PG_FUNCTION_ARGS) if (attform->atttypid != OIDOID) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("column \"%s\" is not of type oid", - NameStr(*attname)))); + errmsg("column \"%s\" is not of type %s", + NameStr(*attname), "oid"))); if (IndexRelationGetNumberOfKeyAttributes(idx) != 1 || idx->rd_index->indkey.values[0] != attno) diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c index 53a9541e89..ea62efebdc 100644 --- a/src/backend/utils/adt/tsvector_op.c +++ b/src/backend/utils/adt/tsvector_op.c @@ -2770,8 +2770,8 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column) TSVECTOROID)) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("column \"%s\" is not of tsvector type", - trigger->tgargs[0]))); + errmsg("column \"%s\" is not of type %s", + trigger->tgargs[0], "tsvector"))); /* Find the configuration to use */ if (config_column) @@ -2788,8 +2788,8 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column) REGCONFIGOID)) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("column \"%s\" is not of regconfig type", - trigger->tgargs[1]))); + errmsg("column \"%s\" is not of type %s", + trigger->tgargs[1], "regconfig"))); datum = SPI_getbinval(rettuple, rel->rd_att, config_attr_num, &isnull); if (isnull) From cb2053dbde7a7e577484f92a60c9bcfe1147472b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Tue, 21 Jul 2026 17:18:02 +0200 Subject: [PATCH 15/43] Remove assertion added by commit 7dcea51c2a4d MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We've got no reports of problems. Get rid of it. Author: Álvaro Herrera Backpatch-through: 19 Discussion: https://postgr.es/m/alewd1f2G0kKeM1i@alvherre.pgsql --- src/backend/replication/logical/logical.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 3541fc793e..c30d40a864 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -2103,7 +2103,6 @@ LogicalSlotAdvanceAndCheckSnapState(XLogRecPtr moveto, bool *found_consistent_snapshot) { LogicalDecodingContext *ctx; - ResourceOwner old_resowner PG_USED_FOR_ASSERTS_ONLY = CurrentResourceOwner; XLogRecPtr retlsn; Assert(XLogRecPtrIsValid(moveto)); @@ -2162,18 +2161,8 @@ LogicalSlotAdvanceAndCheckSnapState(XLogRecPtr moveto, * might still have critical updates to do. */ if (record) - { LogicalDecodingProcessRecord(ctx, ctx->reader); - /* - * We used to have bugs where logical decoding would fail to - * preserve the resource owner. That's important here, so - * verify that that doesn't happen anymore. XXX this could be - * removed once it's been battle-tested. - */ - Assert(CurrentResourceOwner == old_resowner); - } - CHECK_FOR_INTERRUPTS(); } From 96104d6305f22db28b272b3c42c95539e0a091a3 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 21 Jul 2026 17:18:41 +0200 Subject: [PATCH 16/43] Fix typo from commit c1fe2d1a383 --- src/bin/pg_upgrade/check.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 2155b01b11..1fedf63c6d 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -1763,7 +1763,7 @@ check_for_gist_inet_ops(ClusterInfo *cluster) { fclose(report.file); pg_log(PG_REPORT, "fatal"); - pg_fatal("Your installation contains indexes that use btree_gist extension's\n" + pg_fatal("Your installation contains indexes that use the btree_gist extension's\n" "gist_inet_ops or gist_cidr_ops operator classes, which cannot be\n" "binary-upgraded. Replace them with indexes that use the built-in GiST\n" "inet_ops operator class.\n" From 85df8c5bc7fe271bd17ffa1b84b972fabf964d40 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 21 Jul 2026 11:59:37 -0400 Subject: [PATCH 17/43] doc: clarify to_char("OF") HH/MM doesn't represent actual chars Change formatting and chars to be less of a match against actual formatting characters. Reported-by: Phil Discussion: https://postgr.es/m/177801333530.795.16999885814007014333@wrigleys.postgresql.org Backpatch-through: 19 --- doc/src/sgml/func/func-formatting.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/func/func-formatting.sgml b/doc/src/sgml/func/func-formatting.sgml index af9e222399..e4edaf4f42 100644 --- a/doc/src/sgml/func/func-formatting.sgml +++ b/doc/src/sgml/func/func-formatting.sgml @@ -424,8 +424,8 @@ OF - time-zone offset from UTC (HH - or HH:MM) + time-zone offset from UTC (hh + or hh:mi) From 78758d37306cd89ab060f00cb06f249018d5b8da Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 21 Jul 2026 12:06:10 -0400 Subject: [PATCH 18/43] doc: clarify how TIMESTAMP WITH TIME ZONE behaves Mention "time zone conversion" as a way to clarify the time zone is not stored in the database. Reported-by: Richard Neill Discussion: https://postgr.es/m/ddf41f033a8add84e1f28a095defafae@richardneill.org Backpatch-through: 19 --- doc/src/sgml/datatype.sgml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml index cc32f2e816..89985ab7b1 100644 --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -264,13 +264,13 @@ timestamp [ (p) ] [ without time zone ] - date and time (no time zone) + date and time (no time zone conversion) timestamp [ (p) ] with time zone timestamptz - date and time, including time zone + date and time, including time zone conversion @@ -1768,7 +1768,7 @@ SELECT 'abc \153\154\155 \052\251\124'::bytea; timestamp [ (p) ] [ without time zone ] 8 bytes - both date and time (no time zone) + both date and time (no time zone conversion) 4713 BC 294276 AD 1 microsecond @@ -1776,7 +1776,7 @@ SELECT 'abc \153\154\155 \052\251\124'::bytea; timestamp [ (p) ] with time zone 8 bytes - both date and time, with time zone + both date and time, with time zone conversion 4713 BC 294276 AD 1 microsecond @@ -2263,8 +2263,9 @@ TIMESTAMP WITH TIME ZONE '2004-10-19 10:23:54+02' then it is assumed to be in the time zone indicated by the system's parameter, and is converted to UTC using the offset for the timezone zone. - In either case, the value is stored internally as UTC, and the - originally stated or assumed time zone is not retained. + In either case, the value is stored internally as UTC. The + originally stated or assumed time zone is not retained and + cannot be retrieved later. From e65c331b8fbf8c9632b62c5a9dcb589cbd3046a8 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 22 Jul 2026 08:15:00 +0900 Subject: [PATCH 19/43] Add new pgstats routine to split pending data setup This commit adds pgstat_prep_pending_from_entry_ref(), a new pgstats routine that is able to prepare an existing PgStat_EntryRef to receive pending stats. This split gives a way for callers to obtain first a reference via pgstat_get_entry_ref(), then set up pending data as two separate, distinctive, steps. Previously, the only way to get an entry reference with pending data ready was pgstat_prep_pending_entry(), which bundles lookup, creation, and pending setup in a single call. Callers that need finer control over the entry creation had no way to attach pending data to an already-obtained entry reference. One case where this has shown to matter for a stats kind is where one wants to check some capacity (for example where a GUC bounds the maximum numer of entries allowed) before deciding if a new entry should be created. So this split can help in reducing calls to pgstat_get_entry_ref(), meaning less shmem hash table lookups. The only logical ordering change is that pgStatPendingContext is initialized after calling pgstat_get_entry_ref() in pgstat_prep_pending_entry(). This does not matter in practice. pgstat_prep_pending_entry() is refactored to use the new function internally. All the existing callers are unchanged. Existing out-of-core custom stats kinds should see no impact. Author: Sami Imseih Reviewed-by: Kyotaro Horiguchi Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAA5RZ0sV+TsLejUMhAM=PJoOm8u-t8ru7B67KvyLCy=19sM87g@mail.gmail.com --- src/backend/utils/activity/pgstat.c | 55 ++++++++++++++++++----------- src/include/utils/pgstat_internal.h | 1 + 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 9234854b8b..50cd07822b 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -1311,29 +1311,10 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *creat { PgStat_EntryRef *entry_ref; - /* need to be able to flush out */ - Assert(pgstat_get_kind_info(kind)->flush_pending_cb != NULL); - - if (unlikely(!pgStatPendingContext)) - { - pgStatPendingContext = - AllocSetContextCreate(TopMemoryContext, - "PgStat Pending", - ALLOCSET_SMALL_SIZES); - } - entry_ref = pgstat_get_entry_ref(kind, dboid, objid, true, created_entry); - if (entry_ref->pending == NULL) - { - size_t entrysize = pgstat_get_kind_info(kind)->pending_size; - - Assert(entrysize != (size_t) -1); - - entry_ref->pending = MemoryContextAllocZero(pgStatPendingContext, entrysize); - dlist_push_tail(&pgStatPending, &entry_ref->pending_node); - } + pgstat_prep_pending_from_entry_ref(entry_ref); return entry_ref; } @@ -1377,6 +1358,40 @@ pgstat_delete_pending_entry(PgStat_EntryRef *entry_ref) dlist_delete(&entry_ref->pending_node); } +/* + * Prepare the given entry to receive pending stats, if not already done. + */ +void +pgstat_prep_pending_from_entry_ref(PgStat_EntryRef *entry_ref) +{ + PgStat_Kind kind; + + Assert(entry_ref != NULL); + + kind = entry_ref->shared_entry->key.kind; + + /* need to be able to flush out */ + Assert(pgstat_get_kind_info(kind)->flush_pending_cb != NULL); + + if (entry_ref->pending == NULL) + { + size_t entrysize = pgstat_get_kind_info(kind)->pending_size; + + Assert(entrysize != (size_t) -1); + + if (unlikely(!pgStatPendingContext)) + { + pgStatPendingContext = + AllocSetContextCreate(TopMemoryContext, + "PgStat Pending", + ALLOCSET_SMALL_SIZES); + } + + entry_ref->pending = MemoryContextAllocZero(pgStatPendingContext, entrysize); + dlist_push_tail(&pgStatPending, &entry_ref->pending_node); + } +} + /* * Flush out pending variable-numbered stats. */ diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index e62122b883..b0a1769196 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -680,6 +680,7 @@ extern void pgstat_assert_is_up(void); #endif extern void pgstat_delete_pending_entry(PgStat_EntryRef *entry_ref); +extern void pgstat_prep_pending_from_entry_ref(PgStat_EntryRef *entry_ref); extern PgStat_EntryRef *pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *created_entry); From ccfd4b683867a5eded09a82b624992c87396d02d Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 22 Jul 2026 09:52:04 +0900 Subject: [PATCH 20/43] Improve lookup_type_cache() handling on out-of-memory errors If an error happens during the initialization of the TYPEOID catcache, as part of lookup_type_cache(), the error handling of that lookup would cause an assertion failure via finalize_in_progress_typentries(), called during error recovery, the presence of an in-progress type OID causing a catcache initialization outside of a transaction context. The in-progress list is now delayed to happen after the initial entry lookup. Alexander Lakhin has found a fancy way to reproduce the problem, with the injection of probabilistic memory allocation failures. This problem is unlikely going to show up in practice. Like the other changes of this kind, no backpatch is done. Reported-by: Alexander Lakhin Author: Matthias van de Meent Discussion: https://postgr.es/m/95c64dc2-3abe-4f4e-b285-4c681f565d9f@gmail.com --- src/backend/utils/cache/typcache.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c index 650b5d9bad..00133ba92e 100644 --- a/src/backend/utils/cache/typcache.c +++ b/src/backend/utils/cache/typcache.c @@ -459,13 +459,27 @@ lookup_type_cache(Oid type_id, int flags) allocsize * sizeof(*in_progress_list)); in_progress_list_maxlen = allocsize; } - in_progress_offset = in_progress_list_len++; - in_progress_list[in_progress_offset] = type_id; /* Try to look up an existing entry */ typentry = (TypeCacheEntry *) hash_search(TypeCacheHash, &type_id, HASH_FIND, NULL); + + /* + * Only mark the new entry as "in progress" after the initial entry + * lookup. + * + * TypeCacheHash uses type_cache_syshash(), potentially triggering the + * initialization of the TYPEOID catcache, where an out-of-memory failure + * is possible. If an out-of-memory happens, error recovery would call + * finalize_in_progress_typentries(), that could attempt a catcache + * initialization again outside a transaction context. + * + * See also ConditionalCatalogCacheInitializeCache(). + */ + in_progress_offset = in_progress_list_len++; + in_progress_list[in_progress_offset] = type_id; + if (typentry == NULL) { /* From 8767a10cb8c5d08b924f40c8fc1f2a1e5fb8c55e Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 22 Jul 2026 08:47:44 -0400 Subject: [PATCH 21/43] walsummarizer: Guard against WAL files whose tail ends are not valid. SummarizeWAL documents that maximum_lsn should be passed as "the switch point when reading a historic timeline, or the most-recently-measured end of WAL when reading the current timeline." But the caller always passed the most recently measured end-of-WAL even when reading from a historic timeline, due to an oversight on my part. Fix that. As far as I can determine, for this to become an issue in practice, it's necessary to have a corrupted WAL file in the archive. SummarizeWAL checks that every record it processes both starts and ends before switch_lsn; so if all the WAL files in the archive are valid, SummarizeWAL will still discover where it should stop summarizing and do the right thing. However, if there's a corrupted file in the WAL archive, and if it is also the case that the end of the current timeline has advanced past the switch point, then the incorrect maximum_lsn value can result in trying to read an invalid record and erroring out, which leads repeatedly retrying and failing with an error every time. One way this could occur is if a new primary is promoted and creates a .partial file, and the user manually renames that file to remove the suffix, and it is then archived. In that situation, the tail end of the file need not be valid WAL, and that could lead to a stuck WAL summarizer. Reported-by: Fabrice Chapuis Analyzed-by: Thom Brown (using claude) Discussion: http://postgr.es/m/CAA5-nLDdvGMkN6Z-GaHGHG5T7QWEgv4YoHO7XvOJbeD00cghNg@mail.gmail.com Backpatch-through: 17 --- src/backend/postmaster/walsummarizer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/backend/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c index 4f12eaf2c8..8b429cb51d 100644 --- a/src/backend/postmaster/walsummarizer.c +++ b/src/backend/postmaster/walsummarizer.c @@ -349,6 +349,7 @@ WalSummarizerMain(const void *startup_data, size_t startup_data_len) { XLogRecPtr latest_lsn; TimeLineID latest_tli; + XLogRecPtr maximum_lsn; XLogRecPtr end_of_summary_lsn; /* Flush any leaked data in the top-level context */ @@ -413,9 +414,10 @@ WalSummarizerMain(const void *startup_data, size_t startup_data_len) } /* Summarize WAL. */ + maximum_lsn = XLogRecPtrIsValid(switch_lsn) ? switch_lsn : latest_lsn; end_of_summary_lsn = SummarizeWAL(current_tli, current_lsn, exact, - switch_lsn, latest_lsn); + switch_lsn, maximum_lsn); Assert(XLogRecPtrIsValid(end_of_summary_lsn)); Assert(end_of_summary_lsn >= current_lsn); From 1a8c172228db9f8ffd7add086e3b0a3cc4986b1c Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Thu, 23 Jul 2026 10:46:42 +0530 Subject: [PATCH 22/43] Reject sequence synchronization against pre-PostgreSQL 19 publishers. Sequence synchronization requires the page_lsn field returned by pg_get_sequence_data(), which was added in PostgreSQL 19. Previously, requesting sequence synchronization against an older publisher (via ALTER SUBSCRIPTION ... REFRESH SEQUENCES or by running ALTER SUBSCRIPTION ... CONNECTION on a disabled subscription with sequences in the INIT state and subsequently enabling the subscription) would cause the sequence synchronization worker to repeatedly fail with a confusing "invalid query response" error. Check the publisher's server version up front in both AlterSubscription_refresh_seq() and copy_sequences(), and error out immediately when it predates PostgreSQL 19. Also document the PostgreSQL 19 publisher requirement for sequence replication in the logical replication documentation and in ALTER SUBSCRIPTION ... REFRESH SEQUENCES. Reported-by: Noah Misch Author: vignesh C Reviewed-by: Shveta Malik Reviewed-by: Hayato Kuroda Reviewed-by: Amit Kapila Backpatch-through: 19 Discussion: https://postgr.es/m/20260710045217.f0.noahmisch@microsoft.com --- doc/src/sgml/logical-replication.sgml | 18 +++++++++++++++++- doc/src/sgml/ref/alter_subscription.sgml | 6 ++++++ src/backend/commands/subscriptioncmds.c | 10 ++++++++++ src/backend/replication/logical/sequencesync.c | 10 ++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index 690598bff9..36298cacb7 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -1818,6 +1818,13 @@ Included in publications: configuration. + + + Sequence synchronization requires the publisher to be running + PostgreSQL 19 or later. + + + Sequence Definition Mismatches @@ -2368,7 +2375,16 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER ALTER SUBSCRIPTION ... REFRESH SEQUENCES or by copying the current data from the publisher (perhaps using pg_dump) or by determining a sufficiently high value - from the tables themselves. + from the tables themselves. Note that + + ALTER SUBSCRIPTION ... REFRESH SEQUENCES only + re-synchronizes sequences that are already known to the subscription + (see ); in particular, it + requires the publisher to be running PostgreSQL + 19 or later. Before relying on it to prepare for a switchover or + failover, confirm that the publisher's version supports sequence + replication and that the sequences of interest are already known to the + subscription. diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index 8d64744375..6fc3e07a2d 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -245,6 +245,12 @@ ALTER SUBSCRIPTION name RENAME TO < sequences are subscribed. Run REFRESH PUBLICATION first if the publication's set of sequences has changed. + + + Sequence replication requires the publisher to be running + PostgreSQL 19 or later. + + See for recommendations on how to handle any warnings about sequence definition diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 63d288a463..7f946c5b45 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -1381,6 +1381,16 @@ AlterSubscription_refresh_seq(Subscription *sub) /* The publisher connection is only needed for the origin check. */ PG_TRY(); { + /* + * Sequence synchronization depends on publisher-side functionality + * introduced in PostgreSQL 19, so it cannot work against an older + * publisher. + */ + if (walrcv_server_version(wrconn) < 190000) + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot synchronize sequences if the publisher is running a version earlier than PostgreSQL 19")); + check_publications_origin_sequences(wrconn, sub->publications, true, sub->origin, NULL, 0, sub->name); } diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c index 63ad46d7fd..28d4d011a8 100644 --- a/src/backend/replication/logical/sequencesync.c +++ b/src/backend/replication/logical/sequencesync.c @@ -444,6 +444,16 @@ copy_sequences(WalReceiverConn *conn) StringInfoData cmd; MemoryContext oldctx; + /* + * Sequence synchronization depends on publisher-side functionality + * introduced in PostgreSQL 19, so it cannot work against an older + * publisher. + */ + if (walrcv_server_version(conn) < 190000) + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot synchronize sequences if the publisher is running a version earlier than PostgreSQL 19")); + initStringInfo(&seqstr); initStringInfo(&cmd); From a49b6a61094677f75807e452f333f87d4926083f Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 23 Jul 2026 14:37:39 +0900 Subject: [PATCH 23/43] injection_points: Clear waiter slot on error and exit injection_wait() only clears its slot in the waiter array after the wait loop finishes. When the waiting query is canceled or the backend is terminated (wait look has a CHECK_FOR_INTERRUPS), the slot leaks. Later wakeups of the same point then bump the counter of the leaked slot instead of the real waiter, that sleeps forever. Repeated leaks can exhaust all the slots. The code is changed so as the waiting loop is wrapped with PG_ENSURE_ERROR_CLEANUP, so as the injection point slots, that are shared resources, can be cleaned up on ERROR as much as a FATAL. An isolation test is added: cancel one waiter, terminate another waiter, then check that a later waiter still receives a wakeup. Without the fixed code, the test would fail on timeout. Author: Zsolt Parragi Discussion: https://postgr.es/m/CAN4CZFO+KF=cc0-iEg28RhqRBp_fTs6D4b8b7D7DB-pGYP3Ccg@mail.gmail.com Backpatch-through: 17 --- src/test/modules/injection_points/Makefile | 1 + .../expected/wait_cleanup.out | 87 +++++++++++++++++++ .../injection_points/injection_points.c | 31 +++++-- src/test/modules/injection_points/meson.build | 1 + .../injection_points/specs/wait_cleanup.spec | 50 +++++++++++ 5 files changed, 162 insertions(+), 8 deletions(-) create mode 100644 src/test/modules/injection_points/expected/wait_cleanup.out create mode 100644 src/test/modules/injection_points/specs/wait_cleanup.spec diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index c01d2fb095..fac80f3a4a 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -19,6 +19,7 @@ ISOLATION = basic \ repack_temporal_multirange \ repack_toast \ syscache-update-pruned \ + wait_cleanup \ heap_lock_update # some isolation tests require wal_level=replica diff --git a/src/test/modules/injection_points/expected/wait_cleanup.out b/src/test/modules/injection_points/expected/wait_cleanup.out new file mode 100644 index 0000000000..c5be17428f --- /dev/null +++ b/src/test/modules/injection_points/expected/wait_cleanup.out @@ -0,0 +1,87 @@ +Parsed test spec with 3 sessions + +starting permutation: wait1 cancel3 noop3 wait2 wakeup3 noop2 detach3 +injection_points_attach +----------------------- + +(1 row) + +step wait1: SELECT injection_points_run('injection-points-wait'); +step cancel3: + SELECT pg_cancel_backend(pid) FROM pg_stat_activity + WHERE wait_event = 'injection-points-wait'; + +step wait1: <... completed> +ERROR: canceling statement due to user request +step cancel3: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step noop3: +step wait2: SELECT injection_points_run('injection-points-wait'); +step wakeup3: SELECT injection_points_wakeup('injection-points-wait'); +injection_points_wakeup +----------------------- + +(1 row) + +step wait2: <... completed> +injection_points_run +-------------------- + +(1 row) + +step noop2: +step detach3: SELECT injection_points_detach('injection-points-wait'); +injection_points_detach +----------------------- + +(1 row) + + +starting permutation: wait1 terminate3 noop3 wait2 wakeup3 noop2 detach3 +injection_points_attach +----------------------- + +(1 row) + +step wait1: SELECT injection_points_run('injection-points-wait'); +step terminate3: + SELECT pg_terminate_backend(pid) FROM pg_stat_activity + WHERE wait_event = 'injection-points-wait'; + +step wait1: <... completed> +FATAL: terminating connection due to administrator command +server closed the connection unexpectedly + This probably means the server terminated abnormally + before or while processing the request. + +step terminate3: <... completed> +pg_terminate_backend +-------------------- +t +(1 row) + +step noop3: +step wait2: SELECT injection_points_run('injection-points-wait'); +step wakeup3: SELECT injection_points_wakeup('injection-points-wait'); +injection_points_wakeup +----------------------- + +(1 row) + +step wait2: <... completed> +injection_points_run +-------------------- + +(1 row) + +step noop2: +step detach3: SELECT injection_points_detach('injection-points-wait'); +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c index 2d26ecedd5..0ed1dc7c8a 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -223,6 +223,19 @@ injection_notice(const char *name, const void *private_data, void *arg) elog(NOTICE, "notice triggered for injection point %s", name); } +/* + * Error cleanup callback for injection point waits. + */ +static void +injection_wait_cleanup(int code, Datum arg) +{ + int index = DatumGetInt32(arg); + + SpinLockAcquire(&inj_state->lock); + inj_state->name[index][0] = '\0'; + SpinLockRelease(&inj_state->lock); +} + /* Wait until injection_points_wakeup() is called */ void injection_wait(const char *name, const void *private_data, void *arg) @@ -275,19 +288,21 @@ injection_wait(const char *name, const void *private_data, void *arg) delay_us = INJ_WAIT_INITIAL_US; pgstat_report_wait_start(injection_wait_event); - while (pg_atomic_read_u32(&inj_state->wait_counts[index]) == old_wait_counts) + PG_ENSURE_ERROR_CLEANUP(injection_wait_cleanup, Int32GetDatum(index)); { - CHECK_FOR_INTERRUPTS(); - pg_usleep(delay_us); - if (delay_us < INJ_WAIT_MAX_US) - delay_us = Min(delay_us * 2, INJ_WAIT_MAX_US); + while (pg_atomic_read_u32(&inj_state->wait_counts[index]) == old_wait_counts) + { + CHECK_FOR_INTERRUPTS(); + pg_usleep(delay_us); + if (delay_us < INJ_WAIT_MAX_US) + delay_us = Min(delay_us * 2, INJ_WAIT_MAX_US); + } } + PG_END_ENSURE_ERROR_CLEANUP(injection_wait_cleanup, Int32GetDatum(index)); pgstat_report_wait_end(); /* Remove this injection point from the waiters. */ - SpinLockAcquire(&inj_state->lock); - inj_state->name[index][0] = '\0'; - SpinLockRelease(&inj_state->lock); + injection_wait_cleanup(0, Int32GetDatum(index)); } /* diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 59dba1cb02..163b6374eb 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -50,6 +50,7 @@ tests += { 'repack_temporal_multirange', 'repack_toast', 'syscache-update-pruned', + 'wait_cleanup', 'heap_lock_update', ], 'runningcheck': false, # see syscache-update-pruned diff --git a/src/test/modules/injection_points/specs/wait_cleanup.spec b/src/test/modules/injection_points/specs/wait_cleanup.spec new file mode 100644 index 0000000000..ed7d21c4de --- /dev/null +++ b/src/test/modules/injection_points/specs/wait_cleanup.spec @@ -0,0 +1,50 @@ +# Check that a canceled or terminated waiter does not leave a stale slot +# behind in the waiter array. A leaked slot would make later wakeups of +# the same injection point bump the leaked slot's counter instead of the +# real waiter's, leaving the real waiter stuck. + +setup +{ + CREATE EXTENSION injection_points; +} +teardown +{ + DROP EXTENSION injection_points; +} + +# The first waiter, that gets canceled or terminated. This does not +# use injection_points_set_local() on purpose: the injection point +# must survive s1's termination so that s3 can still detach it. +session s1 +setup { + SELECT injection_points_attach('injection-points-wait', 'wait'); +} +step wait1 { SELECT injection_points_run('injection-points-wait'); } + +# The second waiter, that receives a wakeup. +session s2 +step wait2 { SELECT injection_points_run('injection-points-wait'); } +step noop2 { } + +# Control session. The blocker annotations on cancel3/terminate3, +# together with noop3, make the tester wait until wait1 has fully +# completed before starting wait2. Otherwise, wait2 could register a +# new waiter slot while s1 still owns the previous one. +session s3 +step cancel3 { + SELECT pg_cancel_backend(pid) FROM pg_stat_activity + WHERE wait_event = 'injection-points-wait'; +} +step terminate3 { + SELECT pg_terminate_backend(pid) FROM pg_stat_activity + WHERE wait_event = 'injection-points-wait'; +} +step wakeup3 { SELECT injection_points_wakeup('injection-points-wait'); } +step detach3 { SELECT injection_points_detach('injection-points-wait'); } +step noop3 { } + +permutation wait1 cancel3(wait1) noop3 wait2 wakeup3 noop2 detach3 + +# The terminate permutation has to stay last: s1's connection is dead +# afterwards, and the tester never reconnects a session. +permutation wait1 terminate3(wait1) noop3 wait2 wakeup3 noop2 detach3 From abbd74ce8738d536e8d99151122b7a650e3b63d5 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 23 Jul 2026 16:09:13 +0900 Subject: [PATCH 24/43] doc: Improve description of pg_stat_activity.backend_type The documentation of pg_stat_activity used an incomplete list of values for backend_type. While on it, it is improved to use an itemized list, now ordered alphabetically, with a short description about each item. Author: Laurenz Albe Reviewed-By: Michael Paquier Reviewed-By: Fujii Masao Discussion: https://postgr.es/m/5e94c0196084f648ae6a00107125494f5804318a.camel@cybertec.at --- doc/src/sgml/monitoring.sgml | 165 ++++++++++++++++++++++++++++++++--- 1 file changed, 155 insertions(+), 10 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index d1a20d001e..b087d49904 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -1057,16 +1057,161 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser backend_type text - Type of current backend. Possible types are - autovacuum launcher, autovacuum worker, - logical replication launcher, - logical replication worker, - parallel worker, background writer, - client backend, checkpointer, - archiver, standalone backend, - startup, walreceiver, - walsender, walwriter and - walsummarizer. + Type of current backend. Possible types are: + + + + archiver: The WAL archiver, active when + is enabled. + + + + + autovacuum launcher: The background process that + launches autovacuum workers, active when + is on. + + + + + autovacuum worker: A background process running + VACUUM or ANALYZE on a single + table. + + + + + background writer: The background process that + makes sure that there are enough clean buffers in shared buffers. + + + + + checkpointer: The background process that + performs checkpoints + regularly. + + + + + client backend: The server process performing + work for a regular database connection. + + + + + datachecksums launcher: The background process + that launches data checksum workers. + + + + + datachecksums worker: A background process that + calculates data checksums for all pages in one database. + + + + + io worker: A background process performing + asynchronous I/O, active when is set + to worker. + + + + + logical replication apply worker: A background + process that applies data modifications on a logical subscriber. + + + + + logical replication launcher: The background + process that launches logical replication worker processes for + subscriptions. + + + + + logical replication parallel worker: A background + process that applies data modifications on a logical subscriber + for a subscription with streaming = parallel. + + + + + logical replication sequencesync worker: A + background process that replicates sequence data on a logical + subscriber. + + + + + logical replication tablesync worker: A + background process that copies table data on a logical subscriber + for a subscription with copy_data = true. + + + + + parallel worker: A background process that helps + a backend process to perform operations in parallel. + + + + + REPACK decoding worker: A background process that + decodes WAL for REPACK (CONCURRENTLY). + + + + + slotsync worker: The background process that + synchronizes logical replication slots on a streaming replication + standby server, active when + is set to on. + + + + + standalone backend: The backend process when + PostgreSQL was started in + . + + + + + startup: The background process that replays WAL + during crash recovery, archive recovery or streaming replication. + + + + + walreceiver: The background process that receives + WAL records from a WAL sender, active in streaming replication + standby mode. + + + + + walsender: A background process that sends WAL + records to receivers (during streaming replication) or decodes WAL + and sends the decoded information (during logical replication). + + + + + walsummarizer: The background process that + creates summaries from WAL for use with incremental backup, active + when is on. + + + + + walwriter: The background process that persists + WAL records from WAL buffers to disk. + + + In addition, background workers registered by extensions may have additional types. From 544d25b7af958ca6c03e98bfbc1538c295b30601 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 23 Jul 2026 16:48:40 +0900 Subject: [PATCH 25/43] Fix socket_putmessage_noblock() to call socket_putmessage() socket_putmessage_noblock() used pq_putmessage(), which redirects to PqCommMethods->putmessage. In the common cases, this points to socket_putmessage(), but it would become incorrect if PqCommMethods points to a different implementation. This change may look like a bug, but as far as I can see this is mostly cosmetic. The code is able to work currently, as the repalloc() done in the noblock() call ensures that the blocking path of internal_putbytes() is never reached. The issue has gone unnoticed since 2bd9e412f92b. Author: Anthonin Bonnefoy Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAO6_Xqpf5+Rzw_-XOOz-d-R5x6_2JHtpnzXP0nrYWiHyZokA_Q@mail.gmail.com --- src/backend/libpq/pqcomm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index ee9a39107e..aaae7214f1 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -1537,7 +1537,7 @@ socket_putmessage_noblock(char msgtype, const char *s, size_t len) PqSendBuffer = repalloc(PqSendBuffer, required); PqSendBufferSize = required; } - res = pq_putmessage(msgtype, s, len); + res = socket_putmessage(msgtype, s, len); Assert(res == 0); /* should not fail when the message fits in * buffer */ } From 937db82a8d6ffb1b3bb292ed7070dae33aa659ba Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Thu, 23 Jul 2026 19:22:36 +0900 Subject: [PATCH 26/43] doc: Improve pg_stat_recovery documentation Improve the documentation for pg_stat_recovery in several ways: - Mention the view in high-availability.sgml as a way to monitor recovery state and replay progress, alongside the existing recovery information functions. - Clarify that the view returns at most one row, not exactly one row, and no rows to users who lack the pg_read_all_stats privilege. - Correct the description of last_replayed_end_lsn to clarify that it is the end LSN of the last replayed record plus one. - Document that replay_end_tli equals last_replayed_tli when no WAL record is currently being replayed. - Clarify that current_chunk_start_time is NULL until streaming WAL has been received. Backpatch to v19, where pg_stat_recovery was introduced. Author: Fujii Masao Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAHGQGwGRavm18HqnQn_f68QB96qk6arhjET1V93OJH09Mgojkg@mail.gmail.com Backpatch-through: 19 --- doc/src/sgml/high-availability.sgml | 13 ++++++++---- doc/src/sgml/monitoring.sgml | 33 +++++++++++++++++------------ src/include/access/xlogrecovery.h | 4 ++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index 6d9636bd12..fd338ab154 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -920,7 +920,10 @@ primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass' pg_stat_wal_receiver view. A large difference between pg_last_wal_replay_lsn and the view's flushed_lsn indicates that WAL is being - received faster than it can be replayed. + received faster than it can be replayed. Recovery state and replay + progress can also be monitored via the + + pg_stat_recovery view. @@ -1801,9 +1804,11 @@ postgres=# WAIT FOR LSN '0/306EE20'; (In server versions before 14, the in_hot_standby parameter did not exist; a workable substitute method for older servers is SHOW transaction_read_only.) In addition, a set of - functions () allow users to - access information about the standby server. These allow you to write - programs that are aware of the current state of the database. These + functions () and the + + pg_stat_recovery view allow users to + access information about the standby server. These facilities allow you to + write programs that are aware of the current state of the database. They can be used to monitor the progress of recovery, or to allow you to write complex programs that restore the database to particular states. diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index b087d49904..1ce0ef0079 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -340,7 +340,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser pg_stat_recoverypg_stat_recovery - Only one row, showing statistics about the state of recovery. + At most one row, showing statistics about the recovery state. See pg_stat_recovery for details. @@ -2120,9 +2120,11 @@ description | Waiting for a newly initialized WAL file to reach durable storage - The pg_stat_recovery view will contain only + The pg_stat_recovery view will contain at most one row, showing statistics about the recovery state of the startup - process. This view returns no row when the server is not in recovery. + process. This view returns no rows when the server is not in recovery + or the user does not have privileges of the + pg_read_all_stats role. @@ -2164,8 +2166,8 @@ description | Waiting for a newly initialized WAL file to reach durable storage last_replayed_end_lsnpg_lsn - End write-ahead log location of the last successfully replayed - WAL record. + End write-ahead log location, plus one, of the last successfully + replayed WAL record. @@ -2194,18 +2196,20 @@ description | Waiting for a newly initialized WAL file to reach durable storage replay_end_tliinteger - Timeline of the WAL record currently being replayed. + Timeline of the WAL record currently being replayed. When no record + is being actively replayed, equals + last_replayed_tli. - recovery_last_xact_time timestamp with time zone - - - Timestamp of the last transaction commit or abort replayed during - recovery. This is the time at which the commit or abort WAL record - for that transaction was generated on the primary. + recovery_last_xact_time timestamp with time zone + + + Timestamp of the last transaction commit or abort record replayed + during recovery. This is the time at which the commit or abort WAL + record for that transaction was generated on the primary. @@ -2215,8 +2219,9 @@ description | Waiting for a newly initialized WAL file to reach durable storage Time when the startup process observed that replay had caught up - with the latest received WAL chunk. Used in recovery-conflict - timing and replay/apply-lag diagnostics. NULL if not yet + with the latest WAL chunk received from streaming replication. + Used in recovery-conflict timing and replay/apply-lag diagnostics. + NULL if streaming WAL has not yet been received or the time is not available. diff --git a/src/include/access/xlogrecovery.h b/src/include/access/xlogrecovery.h index 9ffd44fcba..a1d8a81dbc 100644 --- a/src/include/access/xlogrecovery.h +++ b/src/include/access/xlogrecovery.h @@ -112,8 +112,8 @@ typedef struct XLogRecoveryCtlData TimestampTz recoveryLastXTime; /* - * timestamp of when we started replaying the current chunk of WAL data, - * only relevant for replication or archive recovery + * timestamp of when we caught up with the latest WAL chunk received from + * streaming replication */ TimestampTz currentChunkStartTime; /* Recovery pause state */ From 1c9c35890421e96a91129b51f2c6446a6d95af95 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Thu, 23 Jul 2026 19:24:55 +0900 Subject: [PATCH 27/43] Validate subscription conninfo on owner change For subscriptions using SERVER, changing the owner can change the effective connection string. However, ALTER SUBSCRIPTION ... OWNER TO did not validate the generated conninfo for the new owner. As a result, ownership could be transferred to a non-superuser whose generated connection string did not satisfy password_required=true. The ownership change succeeded, but the subscription would fail later when the worker or another command tried to connect. Fix this by making ALTER SUBSCRIPTION ... OWNER TO validate the new owner's generated conninfo with walrcv_check_conninfo(). Backpatch to v19, where SERVER subscriptions were introduced. Author: Fujii Masao Reviewed-by: Yuanchao Zhang <145zhangyc@gmail.com> Reviewed-by: Hayato Kuroda Discussion: https://postgr.es/m/CAHGQGwFGa6+wWVgUmZPFwN=fBY59mYPkMK3=TxT=Pv5C1mNNRQ@mail.gmail.com Backpatch-through: 19 --- doc/src/sgml/ref/alter_subscription.sgml | 7 +++++++ src/backend/commands/subscriptioncmds.c | 14 ++++++++++++-- src/test/regress/expected/subscription.out | 17 +++++++++++++++++ src/test/regress/regress.c | 9 +++++++++ src/test/regress/sql/subscription.sql | 16 ++++++++++++++++ 5 files changed, 61 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index 6fc3e07a2d..0f81af5608 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -53,6 +53,13 @@ ALTER SUBSCRIPTION name RENAME TO < to alter the owner, you must be able to SET ROLE to the new owning role. If the subscription has password_required=false, only superusers can modify it. + If the subscription uses a foreign server, the new owner must have + USAGE privilege on the foreign server, a user mapping + for the new owner or for PUBLIC must exist, and the + connection string generated for the new owner must be valid. If the new + owner is not a superuser and the subscription has + password_required=true, the generated connection string + must include a password. diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 7f946c5b45..d4504b4a0c 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -2949,11 +2949,12 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) /* * If the subscription uses a server, check that the new owner has USAGE - * privileges on the server and that a user mapping exists. Note: does not - * re-check the resulting connection string. + * privileges on the server, that a user mapping exists, and that the + * resulting connection string is valid for the new owner. */ if (OidIsValid(form->subserver)) { + char *conninfo; ForeignServer *server = GetForeignServer(form->subserver); aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, newOwnerId, ACL_USAGE); @@ -2966,6 +2967,15 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) /* make sure a user mapping exists */ GetUserMapping(newOwnerId, server->serverid); + + conninfo = ForeignServerConnectionString(newOwnerId, server); + + /* Load the library providing us libpq calls. */ + load_file("libpqwalreceiver", false); + /* Check the connection info string. */ + walrcv_check_conninfo(conninfo, + form->subpasswordrequired && + !superuser_arg(newOwnerId)); } form->subowner = newOwnerId; diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index d201ad764f..1bb785f4f9 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -9,6 +9,10 @@ CREATE FUNCTION test_fdw_connection(oid, oid, internal) RETURNS text AS :'regresslib', 'test_fdw_connection' LANGUAGE C; +CREATE FUNCTION test_fdw_connection_no_password(oid, oid, internal) + RETURNS text + AS :'regresslib', 'test_fdw_connection_no_password' + LANGUAGE C; CREATE ROLE regress_subscription_user LOGIN SUPERUSER; CREATE ROLE regress_subscription_user2; CREATE ROLE regress_subscription_user3 IN ROLE pg_create_subscription; @@ -189,6 +193,18 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server WARNING: subscription was created, but is not connected HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and alter the subscription to refresh publications. RESET SESSION AUTHORIZATION; +GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user2; +CREATE USER MAPPING FOR regress_subscription_user2 SERVER test_server OPTIONS(user 'foo'); +ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection_no_password; +WARNING: changing the foreign-data wrapper connection function can cause the options for dependent objects to become invalid +-- fail, new owner's generated conninfo must satisfy password_required +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2; +ERROR: password is required +DETAIL: Non-superusers must provide a password in the connection string. +ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection; +WARNING: changing the foreign-data wrapper connection function can cause the options for dependent objects to become invalid +DROP USER MAPPING FOR regress_subscription_user2 SERVER test_server; +REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user2; REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; -- ok, lacks USAGE on test_server, but replacing connection anyway @@ -231,6 +247,7 @@ HINT: Use DROP ... CASCADE to drop the dependent objects too. ALTER FOREIGN DATA WRAPPER test_fdw NO CONNECTION; WARNING: removing the foreign-data wrapper connection function will cause dependent subscriptions to fail DROP FUNCTION test_fdw_connection(oid, oid, internal); +DROP FUNCTION test_fdw_connection_no_password(oid, oid, internal); DROP FOREIGN DATA WRAPPER test_fdw; -- fail - invalid connection string during ALTER ALTER SUBSCRIPTION regress_testsub CONNECTION 'foobar'; diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c index 9801cdd1d8..14d301b349 100644 --- a/src/test/regress/regress.c +++ b/src/test/regress/regress.c @@ -742,6 +742,15 @@ test_fdw_connection(PG_FUNCTION_ARGS) PG_RETURN_TEXT_P(cstring_to_text("dbname=regress_doesnotexist user=doesnotexist password=secret")); } +PG_FUNCTION_INFO_V1(test_fdw_connection_no_password); +Datum +test_fdw_connection_no_password(PG_FUNCTION_ARGS) +{ + /* Ensure the test fails if no valid user mapping exists. */ + GetUserMapping(PG_GETARG_OID(0), PG_GETARG_OID(1)); + PG_RETURN_TEXT_P(cstring_to_text("dbname=regress_doesnotexist user=doesnotexist")); +} + PG_FUNCTION_INFO_V1(is_catalog_text_unique_index_oid); Datum is_catalog_text_unique_index_oid(PG_FUNCTION_ARGS) diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 86c402c59a..f19740fdfb 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -12,6 +12,10 @@ CREATE FUNCTION test_fdw_connection(oid, oid, internal) RETURNS text AS :'regresslib', 'test_fdw_connection' LANGUAGE C; +CREATE FUNCTION test_fdw_connection_no_password(oid, oid, internal) + RETURNS text + AS :'regresslib', 'test_fdw_connection_no_password' + LANGUAGE C; CREATE ROLE regress_subscription_user LOGIN SUPERUSER; CREATE ROLE regress_subscription_user2; @@ -136,6 +140,17 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = 'dummy', connect = false); RESET SESSION AUTHORIZATION; +GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user2; +CREATE USER MAPPING FOR regress_subscription_user2 SERVER test_server OPTIONS(user 'foo'); +ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection_no_password; + +-- fail, new owner's generated conninfo must satisfy password_required +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2; + +ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection; +DROP USER MAPPING FOR regress_subscription_user2 SERVER test_server; +REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user2; + REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; @@ -182,6 +197,7 @@ DROP FUNCTION test_fdw_connection(oid, oid, internal); ALTER FOREIGN DATA WRAPPER test_fdw NO CONNECTION; DROP FUNCTION test_fdw_connection(oid, oid, internal); +DROP FUNCTION test_fdw_connection_no_password(oid, oid, internal); DROP FOREIGN DATA WRAPPER test_fdw; From c5f1f41b52b60d4d27a9d77e074da553ee98e26e Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Fri, 24 Jul 2026 15:44:56 +0900 Subject: [PATCH 28/43] Fix EXCEPT publication test to check subscriber Commit fd366065e06 added tests intended to verify that rows inserted on the publisher are replicated to the subscriber when using multiple publications, with one excluding the target table via EXCEPT and another including it. However, the tests queried the publisher instead of the subscriber. Since the rows were inserted directly into the publisher, the checks would always succeed, providing no coverage of replication. Fix this by querying the subscriber so the tests verify the replicated state. Author: Fujii Masao Reviewed-by: Ayush Tiwari Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAHGQGwGfXUO7f4t6KNGurYwg6QsnLtpP0K3EACbAwYWtxGfKfQ@mail.gmail.com Backpatch-through: 19 --- src/test/subscription/t/037_except.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/subscription/t/037_except.pl b/src/test/subscription/t/037_except.pl index 8c58d282ee..43b51c8ff7 100644 --- a/src/test/subscription/t/037_except.pl +++ b/src/test/subscription/t/037_except.pl @@ -244,7 +244,7 @@ sub test_except_root_partition $node_publisher->wait_for_catchup('tap_sub'); $result = - $node_publisher->safe_psql('postgres', "SELECT * FROM tab1 ORDER BY a"); + $node_subscriber->safe_psql('postgres', "SELECT * FROM tab1 ORDER BY a"); is( $result, qq(1 2), "check replication of a table in the EXCEPT clause of one publication but included by another" @@ -272,7 +272,7 @@ sub test_except_root_partition $node_publisher->wait_for_catchup('tap_sub'); $result = - $node_publisher->safe_psql('postgres', "SELECT * FROM tab1 ORDER BY a"); + $node_subscriber->safe_psql('postgres', "SELECT * FROM tab1 ORDER BY a"); is( $result, qq(1 2), "check replication of a table in the EXCEPT clause of one publication but included by another" From b77868f169adcdf31edbc80d8a875204ed7ba191 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Fri, 24 Jul 2026 15:46:46 +0900 Subject: [PATCH 29/43] doc: Add missing CREATE/ALTER PUBLICATION parameter descriptions Document table_name, column_name, and schema_name in the CREATE PUBLICATION and ALTER PUBLICATION reference pages. Also add anchors for the ALTER PUBLICATION parameter list, matching the style already used by CREATE PUBLICATION. Author: Peter Smith Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAHut+Ptekz+TO4ui8-fiBm4Y+O2v=HQnkK_cW4G=w9ep8654EA@mail.gmail.com --- doc/src/sgml/ref/alter_publication.sgml | 32 +++++++++++------ doc/src/sgml/ref/create_publication.sgml | 45 ++++++++++++++++++++---- 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/doc/src/sgml/ref/alter_publication.sgml b/doc/src/sgml/ref/alter_publication.sgml index 52114a16a3..d2898d2633 100644 --- a/doc/src/sgml/ref/alter_publication.sgml +++ b/doc/src/sgml/ref/alter_publication.sgml @@ -149,7 +149,7 @@ ALTER PUBLICATION name RENAME TO Parameters - + name @@ -158,15 +158,17 @@ ALTER PUBLICATION name RENAME TO - + table_name Name of an existing table. If ONLY is specified before the - table name, only that table is affected. If ONLY is not - specified, the table and all its descendant tables (if any) are - affected. Optionally, * can be specified after the table - name to explicitly indicate that descendant tables are included. + table_name, only that table + is affected. If ONLY is not specified, the table and + all its descendant tables (if any) are affected. Optionally, + * can be specified after the + table_name to explicitly + indicate that descendant tables are included. @@ -189,7 +191,17 @@ ALTER PUBLICATION name RENAME TO - + + column_name + + + Name of an existing column of + table_name. + + + + + schema_name @@ -198,7 +210,7 @@ ALTER PUBLICATION name RENAME TO - + SET ( publication_parameter [= value] [, ... ] ) @@ -224,7 +236,7 @@ ALTER PUBLICATION name RENAME TO - + new_owner @@ -233,7 +245,7 @@ ALTER PUBLICATION name RENAME TO - + new_name diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml index 85cfcaddaf..35c28006f6 100644 --- a/doc/src/sgml/ref/create_publication.sgml +++ b/doc/src/sgml/ref/create_publication.sgml @@ -79,15 +79,45 @@ CREATE PUBLICATION name + + table_name + + + Name of an existing table. + + + + + + column_name + + + Name of an existing column of + table_name. + + + + + + schema_name + + + Name of an existing schema. + + + + FOR TABLE Specifies a list of tables to add to the publication. If - ONLY is specified before the table name, only + ONLY is specified before the + table_name, only that table is added to the publication. If ONLY is not specified, the table and all its descendant tables (if any) are added. - Optionally, * can be specified after the table name to + Optionally, * can be specified after the + table_name to explicitly indicate that descendant tables are included. This does not apply to a partitioned table, however. The partitions of a partitioned table are always implicitly considered part of the @@ -208,11 +238,12 @@ CREATE PUBLICATION name For inherited tables, if ONLY is specified before the - table name, only that table is excluded from the publication. If - ONLY is not specified, the table and all its descendant - tables (if any) are excluded. Optionally, * can be - specified after the table name to explicitly indicate that descendant - tables are excluded. + table_name, only that table + is excluded from the publication. If ONLY is not + specified, the table and all its descendant tables (if any) are excluded. + Optionally, * can be specified after the + table_name to explicitly + indicate that descendant tables are excluded. For partitioned tables, only the root partitioned table may be specified From 13b7a8a0ef56d9decae284b4983894175c17d217 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Sat, 25 Jul 2026 10:30:30 +0900 Subject: [PATCH 30/43] Avoid reporting permission-denied publisher sequences as missing Previously, if a sequence synchronization batch contained both a sequence that had been dropped on the publisher and another for which the replication role lacked SELECT privilege, the latter was reported twice: once as a permission failure and again as missing on the publisher. This happened because the permission-denied sequence was not marked as found on the publisher. As a result, when another sequence in the batch was genuinely missing, the later missing-sequence check incorrectly classified the permission-denied sequence as missing as well. Fix this by marking the permission-denied sequence as found before reporting the permission failure, so it is not later reported as missing. Reported-by: Noah Misch Author: Vignesh C Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CALDaNm3LsUjW7PahuCsbYAxajSF+S328tw5E9rF0erdh7dKOXw@mail.gmail.com Backpatch-through: 19 --- .../replication/logical/sequencesync.c | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c index 28d4d011a8..d0370056de 100644 --- a/src/backend/replication/logical/sequencesync.c +++ b/src/backend/replication/logical/sequencesync.c @@ -308,8 +308,24 @@ get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel, */ datum = slot_getattr(slot, ++col, &isnull); if (isnull) - return remote_has_select_priv ? COPYSEQ_SKIPPED : - COPYSEQ_PUBLISHER_INSUFFICIENT_PERM; + { + /* + * The sequence was dropped concurrently after it was identified in + * the catalog snapshot. Treat it as skipped (and, since it no longer + * exists on the publisher, ultimately missing). + */ + if (remote_has_select_priv) + return COPYSEQ_SKIPPED; + + /* + * The publisher lacks the SELECT privilege required by + * pg_get_sequence_data(). Since has_sequence_privilege() returned + * false, not NULL, do not classify this sequence as missing on the + * publisher. + */ + seqinfo_local->found_on_pub = true; + return COPYSEQ_PUBLISHER_INSUFFICIENT_PERM; + } seqinfo_local->last_value = DatumGetInt64(datum); From 38afc3dcb25c45b744d4025029ce0a6c90b7059f Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Sat, 25 Jul 2026 19:08:27 +0900 Subject: [PATCH 31/43] psql: Allow pg_read_all_stats to see database size in \l+ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pg_database_size() allows access to users who have either CONNECT privilege on the target database or privileges of the pg_read_all_stats role. However, previously, psql's \l+ checked only for CONNECT, so users with privileges of pg_read_all_stats still saw "No Access" for databases they could not connect to. Fix this by making \l+ also check pg_has_role('pg_read_all_stats', 'USAGE'), matching pg_database_size()'s permission rules. For back branches, emit the pg_read_all_stats check only when connected to PostgreSQL 10 or later, since earlier releases do not have that predefined role. Backpatch to all supported versions. Author: Christoph Berg Reviewed-by: Álvaro Herrera Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/amCo6qRmnfPVk4-V@msg.df7cb.de Backpatch-through: 14 --- doc/src/sgml/ref/psql-ref.sgml | 5 +++-- src/bin/psql/describe.c | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 56c2692e61..3ec0a3c3b3 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -2817,8 +2817,9 @@ SELECT are displayed in expanded mode. If + is appended to the command name, database sizes, default tablespaces, and descriptions are also displayed. - (Size information is only available for databases that the current - user can connect to.) + Size information is available for databases on which the current user has + CONNECT privilege, or if the current user is a superuser + or has privileges of the pg_read_all_stats role. diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index a2f09c2636..ad9c8affb4 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -986,7 +986,8 @@ listAllDbs(const char *pattern, bool verbose) printACLColumn(&buf, "d.datacl"); if (verbose) appendPQExpBuffer(&buf, - ",\n CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n" + ",\n CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT') OR\n" + " pg_catalog.pg_has_role('pg_read_all_stats', 'USAGE')\n" " THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n" " ELSE 'No Access'\n" " END as \"%s\"" From ce3f19e26218283eaff6436e28113b532bfc4a6f Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sat, 25 Jul 2026 12:01:35 -0400 Subject: [PATCH 32/43] Fix another empty nbtree index SSI race. Commit f9b7fc65 fixed a race when predicate-locking completely empty btrees: without a buffer lock held, a matching key could be inserted between _bt_search and the PredicateLockRelation call, so the scan would miss concurrently inserted tuples while the writer wouldn't see the reader's predicate lock. That commit only fixed _bt_first's _bt_search path, though. Scans without useful insertion scan keys return early from _bt_first via _bt_endpoint, which still didn't recheck if the relation was empty. To fix, add handling to _bt_endpoint that is analogous to the handling added to _bt_search by commit f9b7fc65. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-WzkNoTn3yXY0iGkSuavJ+sL8EROf+kitW+_2v2tJVWuKmA@mail.gmail.com Backpatch-through: 14 --- src/backend/access/nbtree/nbtsearch.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c index aae6acb7f5..dfcdd2d4ce 100644 --- a/src/backend/access/nbtree/nbtsearch.c +++ b/src/backend/access/nbtree/nbtsearch.c @@ -2195,12 +2195,21 @@ _bt_endpoint(IndexScanDesc scan, ScanDirection dir) if (!BufferIsValid(so->currPos.buf)) { /* - * Empty index. Lock the whole relation, as nothing finer to lock - * exists. + * Empty index. Lock the whole relation using the approach explained + * at the same point in the _bt_first path. */ - PredicateLockRelation(rel, scan->xs_snapshot); - _bt_parallel_done(scan); - return false; + if (IsolationIsSerializable()) + { + PredicateLockRelation(rel, scan->xs_snapshot); + so->currPos.buf = _bt_get_endpoint(rel, 0, + ScanDirectionIsBackward(dir)); + } + + if (!BufferIsValid(so->currPos.buf)) + { + _bt_parallel_done(scan); + return false; + } } page = BufferGetPage(so->currPos.buf); From 5168655bc5ed68c0b7d7d3723adbd49c55bafda7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Sat, 25 Jul 2026 19:16:42 +0200 Subject: [PATCH 33/43] Add missing PGDLLIMPORT marker Oversight in commit fb23cc7e81db. Reported-by: Anton Voloshin Discussion: https://postgr.es/m/ad5d772e-09d9-4248-97a4-0011afab9e71@postgrespro.ru --- src/include/postmaster/syslogger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/postmaster/syslogger.h b/src/include/postmaster/syslogger.h index 44409fc254..0e01db6343 100644 --- a/src/include/postmaster/syslogger.h +++ b/src/include/postmaster/syslogger.h @@ -85,7 +85,7 @@ extern PGDLLIMPORT int syslogPipe[2]; extern PGDLLIMPORT HANDLE syslogPipe[2]; #endif -extern bool syslogger_setup_done; +extern PGDLLIMPORT bool syslogger_setup_done; extern int SysLogger_Start(int child_slot); From 62c05d6f2fa64cce44e57871b4cfcd7b34589fcf Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sat, 25 Jul 2026 14:15:33 -0400 Subject: [PATCH 34/43] Add tests for nbtree empty index predicate locking. Add coverage for predicate locking of completely empty nbtree indexes, where we must predicate lock the entire relation (instead of some individual leaf page). Both paths that can find the index empty (and must consider whether it's still empty after PredicateLockRelation returns) are covered by a new isolation test that uses injection points. Catalog relation scans skip the injection points. The waiting session runs catalog queries of its own after arming the (session-local) points, and could otherwise suspend itself with nothing lined up to wake it. Follow-up to bugfix commits ce3f19e2 (the _bt_endpoint fix) and f9b7fc65 (the _bt_first/_bt_search fix). Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-WzkNoTn3yXY0iGkSuavJ+sL8EROf+kitW+_2v2tJVWuKmA@mail.gmail.com --- src/backend/access/nbtree/nbtsearch.c | 12 +++ src/test/modules/nbtree/Makefile | 2 + .../nbtree/expected/predicate-empty-index.out | 87 +++++++++++++++++++ src/test/modules/nbtree/meson.build | 5 ++ .../nbtree/specs/predicate-empty-index.spec | 73 ++++++++++++++++ 5 files changed, 179 insertions(+) create mode 100644 src/test/modules/nbtree/expected/predicate-empty-index.out create mode 100644 src/test/modules/nbtree/specs/predicate-empty-index.spec diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c index dfcdd2d4ce..8eb245e2fc 100644 --- a/src/backend/access/nbtree/nbtsearch.c +++ b/src/backend/access/nbtree/nbtsearch.c @@ -18,10 +18,12 @@ #include "access/nbtree.h" #include "access/relscan.h" #include "access/xact.h" +#include "catalog/catalog.h" #include "executor/instrument_node.h" #include "miscadmin.h" #include "pgstat.h" #include "storage/predicate.h" +#include "utils/injection_point.h" #include "utils/lsyscache.h" #include "utils/rel.h" @@ -1516,6 +1518,11 @@ _bt_first(IndexScanDesc scan, ScanDirection dir) { Assert(!so->needPrimScan); +#ifdef USE_INJECTION_POINTS + if (!IsCatalogRelation(rel)) + INJECTION_POINT("nbtree-first-empty", NULL); +#endif + /* * We only get here if the index is completely empty. Lock relation * because nothing finer to lock exists. Without a buffer lock, it's @@ -2194,6 +2201,11 @@ _bt_endpoint(IndexScanDesc scan, ScanDirection dir) if (!BufferIsValid(so->currPos.buf)) { +#ifdef USE_INJECTION_POINTS + if (!IsCatalogRelation(rel)) + INJECTION_POINT("nbtree-endpoint-empty", NULL); +#endif + /* * Empty index. Lock the whole relation using the approach explained * at the same point in the _bt_first path. diff --git a/src/test/modules/nbtree/Makefile b/src/test/modules/nbtree/Makefile index eec264b16a..72b42d32e2 100644 --- a/src/test/modules/nbtree/Makefile +++ b/src/test/modules/nbtree/Makefile @@ -5,6 +5,8 @@ EXTRA_INSTALL = src/test/modules/injection_points contrib/amcheck REGRESS = nbtree_half_dead_pages \ nbtree_incomplete_splits +ISOLATION = predicate-empty-index + ifdef USE_PGXS PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) diff --git a/src/test/modules/nbtree/expected/predicate-empty-index.out b/src/test/modules/nbtree/expected/predicate-empty-index.out new file mode 100644 index 0000000000..455988e1c0 --- /dev/null +++ b/src/test/modules/nbtree/expected/predicate-empty-index.out @@ -0,0 +1,87 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_scan_first s2_scan s2_insert s2_commit s2_wakeup_first s1_insert s1_commit s2_detach +injection_points_attach +----------------------- + +(1 row) + +step s1_scan_first: SELECT id FROM ssi_btree WHERE id = 2 AND pg_backend_pid() <> 0; +step s2_scan: SELECT id FROM ssi_btree; +id +-- +(0 rows) + +step s2_insert: INSERT INTO ssi_btree VALUES (2); +step s2_commit: COMMIT; +step s2_wakeup_first: SELECT injection_points_wakeup('nbtree-first-empty'); +injection_points_wakeup +----------------------- + +(1 row) + +step s1_scan_first: <... completed> +id +-- +(0 rows) + +step s1_insert: INSERT INTO ssi_btree VALUES (1); +ERROR: could not serialize access due to read/write dependencies among transactions +step s1_commit: COMMIT; +step s2_detach: + SELECT injection_points_detach('nbtree-first-empty'); + SELECT injection_points_detach('nbtree-endpoint-empty'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + + +starting permutation: s1_scan_endpoint s2_scan s2_insert s2_commit s2_wakeup_endpoint s1_insert s1_commit s2_detach +injection_points_attach +----------------------- + +(1 row) + +step s1_scan_endpoint: SELECT id FROM ssi_btree WHERE pg_backend_pid() <> 0 ORDER BY id; +step s2_scan: SELECT id FROM ssi_btree; +id +-- +(0 rows) + +step s2_insert: INSERT INTO ssi_btree VALUES (2); +step s2_commit: COMMIT; +step s2_wakeup_endpoint: SELECT injection_points_wakeup('nbtree-endpoint-empty'); +injection_points_wakeup +----------------------- + +(1 row) + +step s1_scan_endpoint: <... completed> +id +-- +(0 rows) + +step s1_insert: INSERT INTO ssi_btree VALUES (1); +ERROR: could not serialize access due to read/write dependencies among transactions +step s1_commit: COMMIT; +step s2_detach: + SELECT injection_points_detach('nbtree-first-empty'); + SELECT injection_points_detach('nbtree-endpoint-empty'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/nbtree/meson.build b/src/test/modules/nbtree/meson.build index 209c3323b7..8cf861cb2f 100644 --- a/src/test/modules/nbtree/meson.build +++ b/src/test/modules/nbtree/meson.build @@ -14,4 +14,9 @@ tests += { 'nbtree_incomplete_splits', ], }, + 'isolation': { + 'specs': [ + 'predicate-empty-index', + ], + }, } diff --git a/src/test/modules/nbtree/specs/predicate-empty-index.spec b/src/test/modules/nbtree/specs/predicate-empty-index.spec new file mode 100644 index 0000000000..bfcb8bd1ab --- /dev/null +++ b/src/test/modules/nbtree/specs/predicate-empty-index.spec @@ -0,0 +1,73 @@ +# Test SSI's handling of concurrent insertions into an initially empty +# btree index. +# +# When predicate-locking a completely empty btree there is no page to +# lock, so we lock the whole relation instead. This was racy: without a +# buffer lock held, a concurrent transaction can insert a matching key +# between the descent that found the index empty and the +# PredicateLockRelation() call. The scan then misses the inserted tuple, +# but the writer doesn't see the reader's predicate lock either, allowing +# a write skew anomaly to go undetected. + +setup +{ + CREATE EXTENSION injection_points; + CREATE TABLE ssi_btree (id int PRIMARY KEY); +} + +teardown +{ + DROP TABLE ssi_btree; + DROP EXTENSION injection_points; +} + +session s1 +setup { + BEGIN ISOLATION LEVEL SERIALIZABLE; + SET LOCAL enable_seqscan = off; + SET LOCAL enable_bitmapscan = off; + SELECT injection_points_set_local(); + SELECT injection_points_attach('nbtree-first-empty', 'wait'); + SELECT injection_points_attach('nbtree-endpoint-empty', 'wait'); +} +# Scan with a useful insertion scan key: descends via _bt_first/_bt_search. +step s1_scan_first { SELECT id FROM ssi_btree WHERE id = 2 AND pg_backend_pid() <> 0; } +# Scan without useful insertion scan keys: starts at _bt_endpoint(). +step s1_scan_endpoint { SELECT id FROM ssi_btree WHERE pg_backend_pid() <> 0 ORDER BY id; } +step s1_insert { INSERT INTO ssi_btree VALUES (1); } +step s1_commit { COMMIT; } + +# Note: Both scan variants call parallel restricted pg_backend_pid() so that +# the scan runs in the leader process under debug_parallel_query + +session s2 +setup { BEGIN ISOLATION LEVEL SERIALIZABLE; } +step s2_scan { SELECT id FROM ssi_btree; } +step s2_insert { INSERT INTO ssi_btree VALUES (2); } +step s2_commit { COMMIT; } +step s2_wakeup_first { SELECT injection_points_wakeup('nbtree-first-empty'); } +step s2_wakeup_endpoint { SELECT injection_points_wakeup('nbtree-endpoint-empty'); } +step s2_detach { + SELECT injection_points_detach('nbtree-first-empty'); + SELECT injection_points_detach('nbtree-endpoint-empty'); +} + +# _bt_first()/_bt_search() path +permutation s1_scan_first + s2_scan + s2_insert + s2_commit + s2_wakeup_first + s1_insert + s1_commit + s2_detach + +# _bt_endpoint() path +permutation s1_scan_endpoint + s2_scan + s2_insert + s2_commit + s2_wakeup_endpoint + s1_insert + s1_commit + s2_detach From e395fbd32a07557de4ac98088928c1749d4845d8 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sat, 25 Jul 2026 17:13:00 -0400 Subject: [PATCH 35/43] Add test coverage for nbtree backwards scans. Backwards scans have unique concurrency rules: rather than unreservedly trusting a saved left link, the scan optimistically rechecks its pointed-to leaf page's right link (i.e. whether it still points back to the page that _bt_readpage just read). Usually, the left sibling of the just-read page won't have changed, in which case the scan can proceed with reading the left sibling as planned. But it's possible that the key space that the scan needs to read next is no longer covered by the original left sibling page due to concurrent page splits and/or page deletions. When that happens, the scan must recover by relocating the new/current left sibling of the just-read page. Test coverage for backwards scans was limited to the happy path. Add an isolation test (and associated injection points) that test the recovery path. This covers several distinct recovery scenarios (concurrent page splits, concurrent page deletions, and minor variants thereof). Author: Peter Geoghegan Reviewed-by: Andrey Borodin Discussion: https://postgr.es/m/CAH2-WzmD+jUBOpFS2jrnqqrdPSAjoxqyL9FPKaE1BtnY=8Nntg@mail.gmail.com --- src/backend/access/nbtree/nbtsearch.c | 21 ++ src/test/modules/nbtree/Makefile | 3 +- .../backwards-scan-concurrent-splits.out | 304 ++++++++++++++++++ src/test/modules/nbtree/meson.build | 2 + .../backwards-scan-concurrent-splits.spec | 123 +++++++ 5 files changed, 452 insertions(+), 1 deletion(-) create mode 100644 src/test/modules/nbtree/expected/backwards-scan-concurrent-splits.out create mode 100644 src/test/modules/nbtree/specs/backwards-scan-concurrent-splits.spec diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c index 8eb245e2fc..5964bc9195 100644 --- a/src/backend/access/nbtree/nbtsearch.c +++ b/src/backend/access/nbtree/nbtsearch.c @@ -1984,6 +1984,11 @@ _bt_lock_and_validate_left(Relation rel, BlockNumber *blkno, { BlockNumber origblkno = *blkno; /* detects circular links */ +#ifdef USE_INJECTION_POINTS + if (!IsCatalogRelation(rel)) + INJECTION_POINT("nbtree-walk-left", NULL); +#endif + for (;;) { Buffer buf; @@ -2018,6 +2023,12 @@ _bt_lock_and_validate_left(Relation rel, BlockNumber *blkno, } if (P_RIGHTMOST(opaque) || ++tries > 4) break; + +#ifdef USE_INJECTION_POINTS + if (!IsCatalogRelation(rel)) + INJECTION_POINT("nbtree-walk-left-step-right", NULL); +#endif + /* step right */ *blkno = opaque->btpo_next; buf = _bt_relandgetbuf(rel, buf, *blkno, BT_READ); @@ -2035,6 +2046,11 @@ _bt_lock_and_validate_left(Relation rel, BlockNumber *blkno, opaque = BTPageGetOpaque(page); if (P_ISDELETED(opaque)) { +#ifdef USE_INJECTION_POINTS + if (!IsCatalogRelation(rel)) + INJECTION_POINT("nbtree-walk-left-deleted", NULL); +#endif + /* * It was deleted. Move right to first nondeleted page (there * must be one); that is the page that has acquired the deleted @@ -2082,6 +2098,11 @@ _bt_lock_and_validate_left(Relation rel, BlockNumber *blkno, /* Start from scratch with new lastcurrblkno's blkno/prev link */ *blkno = origblkno = opaque->btpo_prev; _bt_relbuf(rel, buf); + +#ifdef USE_INJECTION_POINTS + if (!IsCatalogRelation(rel)) + INJECTION_POINT("nbtree-walk-left-restart", NULL); +#endif } return InvalidBuffer; diff --git a/src/test/modules/nbtree/Makefile b/src/test/modules/nbtree/Makefile index 72b42d32e2..20a1ca6a92 100644 --- a/src/test/modules/nbtree/Makefile +++ b/src/test/modules/nbtree/Makefile @@ -5,7 +5,8 @@ EXTRA_INSTALL = src/test/modules/injection_points contrib/amcheck REGRESS = nbtree_half_dead_pages \ nbtree_incomplete_splits -ISOLATION = predicate-empty-index +ISOLATION = backwards-scan-concurrent-splits \ + predicate-empty-index ifdef USE_PGXS PG_CONFIG = pg_config diff --git a/src/test/modules/nbtree/expected/backwards-scan-concurrent-splits.out b/src/test/modules/nbtree/expected/backwards-scan-concurrent-splits.out new file mode 100644 index 0000000000..906c10d10a --- /dev/null +++ b/src/test/modules/nbtree/expected/backwards-scan-concurrent-splits.out @@ -0,0 +1,304 @@ +Parsed test spec with 2 sessions + +starting permutation: b_attach b_scan i_insert_dups i_detach b_detach +step b_attach: + SELECT injection_points_attach('nbtree-walk-left', 'wait'); + SELECT injection_points_attach('nbtree-walk-left-step-right', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-restart', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-deleted', 'notice'); + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step b_scan: SELECT col FROM backwards_scan_tbl + WHERE col % 100 = 1 AND pg_backend_pid() <> 0 + ORDER BY col DESC; +step i_insert_dups: INSERT INTO backwards_scan_tbl SELECT 100 FROM generate_series(1, 60); +step i_detach: + SELECT injection_points_detach('nbtree-walk-left'); + SELECT injection_points_wakeup('nbtree-walk-left'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-step-right +step b_scan: <... completed> +col +--- +601 +501 +401 +301 +201 +101 + 1 +(7 rows) + +step b_detach: + SELECT injection_points_detach('nbtree-walk-left-step-right'); + SELECT injection_points_detach('nbtree-walk-left-restart'); + SELECT injection_points_detach('nbtree-walk-left-deleted'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + + +starting permutation: b_attach b_scan i_insert i_detach b_detach +step b_attach: + SELECT injection_points_attach('nbtree-walk-left', 'wait'); + SELECT injection_points_attach('nbtree-walk-left-step-right', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-restart', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-deleted', 'notice'); + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step b_scan: SELECT col FROM backwards_scan_tbl + WHERE col % 100 = 1 AND pg_backend_pid() <> 0 + ORDER BY col DESC; +step i_insert: INSERT INTO backwards_scan_tbl SELECT i FROM generate_series(-2000, 700) i; +step i_detach: + SELECT injection_points_detach('nbtree-walk-left'); + SELECT injection_points_wakeup('nbtree-walk-left'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-step-right +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-step-right +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-step-right +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-step-right +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-restart +step b_scan: <... completed> +col +--- +601 +501 +401 +301 +201 +101 + 1 +(7 rows) + +step b_detach: + SELECT injection_points_detach('nbtree-walk-left-step-right'); + SELECT injection_points_detach('nbtree-walk-left-restart'); + SELECT injection_points_detach('nbtree-walk-left-deleted'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + + +starting permutation: b_attach d_delete_left b_scan vacuum_tbl i_detach b_detach +step b_attach: + SELECT injection_points_attach('nbtree-walk-left', 'wait'); + SELECT injection_points_attach('nbtree-walk-left-step-right', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-restart', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-deleted', 'notice'); + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step d_delete_left: DELETE FROM backwards_scan_tbl WHERE col < 601; +step b_scan: SELECT col FROM backwards_scan_tbl + WHERE col % 100 = 1 AND pg_backend_pid() <> 0 + ORDER BY col DESC; +step vacuum_tbl: VACUUM backwards_scan_tbl; +step i_detach: + SELECT injection_points_detach('nbtree-walk-left'); + SELECT injection_points_wakeup('nbtree-walk-left'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-step-right +step b_scan: <... completed> +col +--- +601 +(1 row) + +step b_detach: + SELECT injection_points_detach('nbtree-walk-left-step-right'); + SELECT injection_points_detach('nbtree-walk-left-restart'); + SELECT injection_points_detach('nbtree-walk-left-deleted'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + + +starting permutation: b_attach_nosr i_grow d_delete_mid b_scan_999 vacuum_tbl i_detach b_detach_nosr +step b_attach_nosr: + SELECT injection_points_attach('nbtree-walk-left', 'wait'); + SELECT injection_points_attach('nbtree-walk-left-restart', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-deleted', 'notice'); + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step i_grow: INSERT INTO backwards_scan_tbl SELECT i FROM generate_series(701, 2200) i; +step d_delete_mid: DELETE FROM backwards_scan_tbl WHERE col BETWEEN 367 AND 2100; +step b_scan_999: SELECT col FROM backwards_scan_tbl + WHERE col <= 999 AND col % 100 = 1 AND pg_backend_pid() <> 0 + ORDER BY col DESC; +step vacuum_tbl: VACUUM backwards_scan_tbl; +step i_detach: + SELECT injection_points_detach('nbtree-walk-left'); + SELECT injection_points_wakeup('nbtree-walk-left'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-deleted +scan_session: NOTICE: notice triggered for injection point nbtree-walk-left-restart +step b_scan_999: <... completed> +col +--- +301 +201 +101 + 1 +(4 rows) + +step b_detach_nosr: + SELECT injection_points_detach('nbtree-walk-left-restart'); + SELECT injection_points_detach('nbtree-walk-left-deleted'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/nbtree/meson.build b/src/test/modules/nbtree/meson.build index 8cf861cb2f..b5dc026392 100644 --- a/src/test/modules/nbtree/meson.build +++ b/src/test/modules/nbtree/meson.build @@ -16,7 +16,9 @@ tests += { }, 'isolation': { 'specs': [ + 'backwards-scan-concurrent-splits', 'predicate-empty-index', ], + 'runningcheck': false, # see syscache-update-pruned }, } diff --git a/src/test/modules/nbtree/specs/backwards-scan-concurrent-splits.spec b/src/test/modules/nbtree/specs/backwards-scan-concurrent-splits.spec new file mode 100644 index 0000000000..62c0cf25a3 --- /dev/null +++ b/src/test/modules/nbtree/specs/backwards-scan-concurrent-splits.spec @@ -0,0 +1,123 @@ +# Backwards scan isolation test +# +# Backwards scans cannot unreservedly trust their saved left link: by the time +# the scan follows it, concurrent page splits and/or page deletions may have +# left it pointing to a page that is no longer the correct page for the scan +# to read next. The scan checks for this by verifying that the pointed-to +# page's right link still points back to the page that the scan just read, and +# recovers when it doesn't (see nbtree/README for details). +# +# Each permutation makes the scan wait "between pages" at the nbtree-walk-left +# injection point while the concurrent session splits and/or deletes pages, +# then wakes it, forcing the scan to take one of its recovery paths. The +# notice-mode injection points confirm which recovery steps ran. +# +# Note: the permutations' expected notifications (and the leaf pages that each +# concurrent session step splits or deletes) assume the default 8KB BLCKSZ. + +setup +{ + CREATE EXTENSION injection_points; + CREATE TABLE backwards_scan_tbl(col int4) WITH (autovacuum_enabled = off); + CREATE INDEX ON backwards_scan_tbl(col) WITH (deduplicate_items = off); + INSERT INTO backwards_scan_tbl SELECT i FROM generate_series(0, 700) i; +} +setup +{ + VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) backwards_scan_tbl; +} +teardown +{ + DROP EXTENSION injection_points; + DROP TABLE backwards_scan_tbl; +} + +session scan_session +setup { + SELECT injection_points_set_local(); + SET enable_seqscan=off; + SET enable_sort=off; +} +step b_attach { + SELECT injection_points_attach('nbtree-walk-left', 'wait'); + SELECT injection_points_attach('nbtree-walk-left-step-right', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-restart', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-deleted', 'notice'); +} +# Variant that doesn't attach to nbtree-walk-left-step-right, for +# permutations whose number of step right attempts varies with the amount of +# free space that index tuples' varying alignment padding leaves on each page +step b_attach_nosr { + SELECT injection_points_attach('nbtree-walk-left', 'wait'); + SELECT injection_points_attach('nbtree-walk-left-restart', 'notice'); + SELECT injection_points_attach('nbtree-walk-left-deleted', 'notice'); +} +# Note: Both scan variants call parallel restricted pg_backend_pid() so that +# the scan runs in the leader process under debug_parallel_query +step b_scan { SELECT col FROM backwards_scan_tbl + WHERE col % 100 = 1 AND pg_backend_pid() <> 0 + ORDER BY col DESC; } +step b_scan_999 { SELECT col FROM backwards_scan_tbl + WHERE col <= 999 AND col % 100 = 1 AND pg_backend_pid() <> 0 + ORDER BY col DESC; } +step b_detach { + SELECT injection_points_detach('nbtree-walk-left-step-right'); + SELECT injection_points_detach('nbtree-walk-left-restart'); + SELECT injection_points_detach('nbtree-walk-left-deleted'); +} +step b_detach_nosr { + SELECT injection_points_detach('nbtree-walk-left-restart'); + SELECT injection_points_detach('nbtree-walk-left-deleted'); +} + +session concurrent_session +step i_insert { INSERT INTO backwards_scan_tbl SELECT i FROM generate_series(-2000, 700) i; } +step i_insert_dups { INSERT INTO backwards_scan_tbl SELECT 100 FROM generate_series(1, 60); } +step i_grow { INSERT INTO backwards_scan_tbl SELECT i FROM generate_series(701, 2200) i; } +step d_delete_left { DELETE FROM backwards_scan_tbl WHERE col < 601; } +step d_delete_mid { DELETE FROM backwards_scan_tbl WHERE col BETWEEN 367 AND 2100; } +step vacuum_tbl { VACUUM backwards_scan_tbl; } +step i_detach { + SELECT injection_points_detach('nbtree-walk-left'); + SELECT injection_points_wakeup('nbtree-walk-left'); +} + +# A single concurrent page split. When the backwards scan session wakes up, +# its search recovers by stepping right just once. +permutation b_attach + b_scan + i_insert_dups + i_detach + b_detach + +# Many concurrent page splits. When the backwards scan session wakes up, its +# search steps right the maximum number of times before giving up and +# starting over with the right sibling page's current left link. +permutation b_attach + b_scan + i_insert + i_detach + b_detach + +# Concurrent deletion of all pages to the left of the page that the scan just +# read. When the backwards scan session wakes up, its search determines that +# the scan has no page to the left to move to, ending the scan. +permutation b_attach + d_delete_left + b_scan + vacuum_tbl + i_detach + b_detach + +# Concurrent deletion of the page that the scan just read (which the scan can +# only safely rely on when a search locates its left sibling using its saved +# right link, which the deleted page's right sibling has acquired). The scan +# just read a page whose tuples all pointed to dead-to-all heap tuples, which +# VACUUM deletes during the scan's wait, along with all nearby pages. +permutation b_attach_nosr + i_grow + d_delete_mid + b_scan_999 + vacuum_tbl + i_detach + b_detach_nosr From e01accdb50ce8f879182edb1b50f3bc9bd78bfa7 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sun, 26 Jul 2026 12:49:56 -0400 Subject: [PATCH 36/43] Add _bt_set_startikey row compare test coverage. Add pg_regress tests that exercise the row compare logic that commit 7d9cd2df added to _bt_set_startikey. Also add tests that exercise the _bt_set_startikey SAOP array path. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wz=KjQsD2W2a=b51uH905=0mF6Le4evhWkN2FL1+uRPhUg@mail.gmail.com Backpatch-through: 19 --- src/test/regress/expected/btree_index.out | 157 ++++++++++++++++++++++ src/test/regress/sql/btree_index.sql | 85 ++++++++++++ 2 files changed, 242 insertions(+) diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out index 21dc9b5783..3a83e9a053 100644 --- a/src/test/regress/expected/btree_index.out +++ b/src/test/regress/expected/btree_index.out @@ -308,6 +308,163 @@ ORDER BY proname, proargtypes, pronamespace; ---------+-------------+-------------- (0 rows) +-- +-- Test RowCompare handling within _bt_set_startikey, which decides whether +-- every tuple on a page (a page beyond the scan's first) must satisfy the +-- scan's RowCompare qual. +-- +-- The index mixes an ASC column with a DESC column (so RowCompare members +-- don't all use the same inequality strategy and are not marked required), +-- uses a low fillfactor (so scans read several pages), and disables +-- deduplication (so the "b" NULLs span more than one page). +create temp table btree_rowcompare_tab (a int, b int, c int); +insert into btree_rowcompare_tab + select a, b, b from generate_series(1, 3) a, generate_series(1, 150) b; +insert into btree_rowcompare_tab + select 2, null, null from generate_series(1, 50); +create index btree_rowcompare_idx on btree_rowcompare_tab (a, b desc, c) + with (fillfactor = 10, deduplicate_items = off); +vacuum analyze btree_rowcompare_tab; +set enable_seqscan to false; +set enable_bitmapscan to false; +-- RowCompare satisfied by every tuple on many pages (decided by its first +-- member on "a = 3" pages, and by its final member on "a = 2" pages) +explain (costs off) +select count(*) from btree_rowcompare_tab where (a, b) >= (2, 75); + QUERY PLAN +-------------------------------------------------------------------------- + Aggregate + -> Index Only Scan using btree_rowcompare_idx on btree_rowcompare_tab + Index Cond: (ROW(a, b) >= ROW(2, 75)) +(3 rows) + +select count(*) from btree_rowcompare_tab where (a, b) >= (2, 75); + count +------- + 226 +(1 row) + +-- Reaches the RowCompare's unsatisfiable NULL member argument on "a = 2" +-- pages (the "a = 2" key positions the scan within the "a = 2" group, which +-- the RowCompare qual alone would not). The combined quals are +-- contradictory, but preprocessing cannot detect that. +explain (costs off) +select count(*) from btree_rowcompare_tab where a = 2 and (a, b) >= (2, null); + QUERY PLAN +-------------------------------------------------------------------------- + Aggregate + -> Index Only Scan using btree_rowcompare_idx on btree_rowcompare_tab + Index Cond: ((ROW(a, b) >= ROW(2, NULL::integer)) AND (a = 2)) +(3 rows) + +select count(*) from btree_rowcompare_tab where a = 2 and (a, b) >= (2, null); + count +------- + 0 +(1 row) + +-- RowCompare's row omits the index's second column, so on pages whose "b" +-- values change _bt_set_startikey can't prove that every tuple satisfies the +-- RowCompare. +explain (costs off) +select count(*) from btree_rowcompare_tab where (a, c) >= (2, 100); + QUERY PLAN +-------------------------------------------------------------------------- + Aggregate + -> Index Only Scan using btree_rowcompare_idx on btree_rowcompare_tab + Index Cond: (ROW(a, c) >= ROW(2, 100)) +(3 rows) + +select count(*) from btree_rowcompare_tab where (a, c) >= (2, 100); + count +------- + 201 +(1 row) + +-- Variant that uses the remaining inequality strategies +explain (costs off) +select count(*) from btree_rowcompare_tab where (a, b) < (2, 10); + QUERY PLAN +-------------------------------------------------------------------------- + Aggregate + -> Index Only Scan using btree_rowcompare_idx on btree_rowcompare_tab + Index Cond: (ROW(a, b) < ROW(2, 10)) +(3 rows) + +select count(*) from btree_rowcompare_tab where (a, b) < (2, 10); + count +------- + 159 +(1 row) + +drop table btree_rowcompare_tab; +-- +-- Test SAOP array handling within _bt_set_startikey +-- +create temp table btree_saop_tab (a int, b int, c int); +insert into btree_saop_tab + select a, b, b from generate_series(1, 3) a, generate_series(1, 150) b; +insert into btree_saop_tab + select 2, 0, 7 from generate_series(1, 60); +create index btree_saop_idx on btree_saop_tab (a, b desc, c) + with (fillfactor = 10, deduplicate_items = off); +vacuum analyze btree_saop_tab; +-- SAOP on the leading column: pages beyond each primitive scan's first page +-- have a single "a" value that a binary search finds in the array, so the +-- scan starts past the SAOP key (forcing the nonrequired key protocol) +explain (costs off) +select count(*) from btree_saop_tab where a in (1, 3) and b >= 100; + QUERY PLAN +--------------------------------------------------------------------- + Aggregate + -> Index Only Scan using btree_saop_idx on btree_saop_tab + Index Cond: ((a = ANY ('{1,3}'::integer[])) AND (b >= 100)) +(3 rows) + +select count(*) from btree_saop_tab where a in (1, 3) and b >= 100; + count +------- + 102 +(1 row) + +-- Skip array on "b" precedes the "c" SAOP; pages whose "b" values change +-- prevent starting past the "c" SAOP key +explain (costs off) +select count(*) from btree_saop_tab where a = 2 and c in (101, 105); + QUERY PLAN +---------------------------------------------------------------------- + Aggregate + -> Index Only Scan using btree_saop_idx on btree_saop_tab + Index Cond: ((a = 2) AND (c = ANY ('{101,105}'::integer[]))) +(3 rows) + +select count(*) from btree_saop_tab where a = 2 and c in (101, 105); + count +------- + 2 +(1 row) + +-- "c" SAOP follows the "b" inequality; on pages that lie wholly within the +-- duplicate "(2, 0, 7)" run, the scan starts past all of its scan keys, +-- including the SAOP key +explain (costs off) +select count(*) from btree_saop_tab where a = 2 and b < 1 and c in (6, 7); + QUERY PLAN +------------------------------------------------------------------------------ + Aggregate + -> Index Only Scan using btree_saop_idx on btree_saop_tab + Index Cond: ((a = 2) AND (b < 1) AND (c = ANY ('{6,7}'::integer[]))) +(3 rows) + +select count(*) from btree_saop_tab where a = 2 and b < 1 and c in (6, 7); + count +------- + 60 +(1 row) + +reset enable_seqscan; +reset enable_bitmapscan; +drop table btree_saop_tab; -- -- Performs a recheck of > key following array advancement on previous (left -- sibling) page that used a high key whose attribute value corresponding to diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql index 6aaaa386ab..a08bb101c2 100644 --- a/src/test/regress/sql/btree_index.sql +++ b/src/test/regress/sql/btree_index.sql @@ -216,6 +216,91 @@ SELECT proname, proargtypes, pronamespace AND pronamespace IN (1, 2, 3) AND proargtypes IN ('26 23', '5077') ORDER BY proname, proargtypes, pronamespace; +-- +-- Test RowCompare handling within _bt_set_startikey, which decides whether +-- every tuple on a page (a page beyond the scan's first) must satisfy the +-- scan's RowCompare qual. +-- +-- The index mixes an ASC column with a DESC column (so RowCompare members +-- don't all use the same inequality strategy and are not marked required), +-- uses a low fillfactor (so scans read several pages), and disables +-- deduplication (so the "b" NULLs span more than one page). +create temp table btree_rowcompare_tab (a int, b int, c int); +insert into btree_rowcompare_tab + select a, b, b from generate_series(1, 3) a, generate_series(1, 150) b; +insert into btree_rowcompare_tab + select 2, null, null from generate_series(1, 50); +create index btree_rowcompare_idx on btree_rowcompare_tab (a, b desc, c) + with (fillfactor = 10, deduplicate_items = off); +vacuum analyze btree_rowcompare_tab; + +set enable_seqscan to false; +set enable_bitmapscan to false; + +-- RowCompare satisfied by every tuple on many pages (decided by its first +-- member on "a = 3" pages, and by its final member on "a = 2" pages) +explain (costs off) +select count(*) from btree_rowcompare_tab where (a, b) >= (2, 75); +select count(*) from btree_rowcompare_tab where (a, b) >= (2, 75); + +-- Reaches the RowCompare's unsatisfiable NULL member argument on "a = 2" +-- pages (the "a = 2" key positions the scan within the "a = 2" group, which +-- the RowCompare qual alone would not). The combined quals are +-- contradictory, but preprocessing cannot detect that. +explain (costs off) +select count(*) from btree_rowcompare_tab where a = 2 and (a, b) >= (2, null); +select count(*) from btree_rowcompare_tab where a = 2 and (a, b) >= (2, null); + +-- RowCompare's row omits the index's second column, so on pages whose "b" +-- values change _bt_set_startikey can't prove that every tuple satisfies the +-- RowCompare. +explain (costs off) +select count(*) from btree_rowcompare_tab where (a, c) >= (2, 100); +select count(*) from btree_rowcompare_tab where (a, c) >= (2, 100); + +-- Variant that uses the remaining inequality strategies +explain (costs off) +select count(*) from btree_rowcompare_tab where (a, b) < (2, 10); +select count(*) from btree_rowcompare_tab where (a, b) < (2, 10); + +drop table btree_rowcompare_tab; + +-- +-- Test SAOP array handling within _bt_set_startikey +-- +create temp table btree_saop_tab (a int, b int, c int); +insert into btree_saop_tab + select a, b, b from generate_series(1, 3) a, generate_series(1, 150) b; +insert into btree_saop_tab + select 2, 0, 7 from generate_series(1, 60); +create index btree_saop_idx on btree_saop_tab (a, b desc, c) + with (fillfactor = 10, deduplicate_items = off); +vacuum analyze btree_saop_tab; + +-- SAOP on the leading column: pages beyond each primitive scan's first page +-- have a single "a" value that a binary search finds in the array, so the +-- scan starts past the SAOP key (forcing the nonrequired key protocol) +explain (costs off) +select count(*) from btree_saop_tab where a in (1, 3) and b >= 100; +select count(*) from btree_saop_tab where a in (1, 3) and b >= 100; + +-- Skip array on "b" precedes the "c" SAOP; pages whose "b" values change +-- prevent starting past the "c" SAOP key +explain (costs off) +select count(*) from btree_saop_tab where a = 2 and c in (101, 105); +select count(*) from btree_saop_tab where a = 2 and c in (101, 105); + +-- "c" SAOP follows the "b" inequality; on pages that lie wholly within the +-- duplicate "(2, 0, 7)" run, the scan starts past all of its scan keys, +-- including the SAOP key +explain (costs off) +select count(*) from btree_saop_tab where a = 2 and b < 1 and c in (6, 7); +select count(*) from btree_saop_tab where a = 2 and b < 1 and c in (6, 7); + +reset enable_seqscan; +reset enable_bitmapscan; +drop table btree_saop_tab; + -- -- Performs a recheck of > key following array advancement on previous (left -- sibling) page that used a high key whose attribute value corresponding to From 0962f9e344390c69e44bc55675510b2fa2b3f778 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 27 Jul 2026 09:42:50 +0900 Subject: [PATCH 37/43] Use direct hash lookup in logicalrep_partmap_invalidate_cb() This replaces an O(N) hash_seq_search() loop by an O(1) lookup, removing a TODO item, making the invalidation callback faster when dealing with many relations. This can work because LogicalRepPartMap is keyed by a partition OID, and a relmapentry's localreloid matches with it. An assertion is added in logicalrep_partition_open() to enforce the fact that localreloid matches with the hash key. Author: DaeMyung Kang Discussion: https://postgr.es/m/20260417174450.4158878-1-charsyam@gmail.com --- src/backend/replication/logical/relation.c | 23 +++++++++------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 296cbaede3..8749826425 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -543,20 +543,14 @@ logicalrep_partmap_invalidate_cb(Datum arg, Oid reloid) if (reloid != InvalidOid) { - HASH_SEQ_STATUS status; - - hash_seq_init(&status, LogicalRepPartMap); - - /* TODO, use inverse lookup hashtable? */ - while ((entry = (LogicalRepPartMapEntry *) hash_seq_search(&status)) != NULL) - { - if (entry->relmapentry.localreloid == reloid) - { - entry->relmapentry.localrelvalid = false; - hash_seq_term(&status); - break; - } - } + /* + * LogicalRepPartMap is keyed by partition OID, matching with + * entry->relmapentry.localreloid (see logicalrep_partition_open), so + * we can invalidate via a direct hash lookup. + */ + entry = hash_search(LogicalRepPartMap, &reloid, HASH_FIND, NULL); + if (entry != NULL) + entry->relmapentry.localrelvalid = false; } else { @@ -675,6 +669,7 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root, */ if (found && entry->localrelvalid) { + Assert(entry->localreloid == partOid); entry->localrel = partrel; return entry; } From 87f08dbf3499929f4941f224f52ccd6a20081f00 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 27 Jul 2026 09:58:04 +0900 Subject: [PATCH 38/43] Update .gitignore in test/modules/nbtree Noticed while doing some routine work. Oversight in e395fbd32a07. --- src/test/modules/nbtree/.gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/modules/nbtree/.gitignore b/src/test/modules/nbtree/.gitignore index 5dcb3ff972..0de307e70a 100644 --- a/src/test/modules/nbtree/.gitignore +++ b/src/test/modules/nbtree/.gitignore @@ -1,4 +1,6 @@ # Generated subdirectories /log/ +/output_iso/ /results/ /tmp_check/ +/tmp_check_iso/ From f4c850d11afc60a4fc4bc782fc40d7b752bf8d7f Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Mon, 27 Jul 2026 10:21:18 +0900 Subject: [PATCH 39/43] Fix deparsing of JSON_ARRAY(subquery) with a FORMAT clause Commit 8d829f5a0 introduced the JSCTOR_JSON_ARRAY_QUERY constructor type so that ruleutils.c could deparse JSON_ARRAY(subquery) using its original syntax, storing the transformed subquery in a new orig_query field. However, the input FORMAT clause of JSON_ARRAY(subquery FORMAT ...) was not preserved for deparsing. The format was recorded only in the executable expression kept in the func field, which ruleutils.c does not inspect, so it is silently dropped. This is more than cosmetic, because FORMAT JSON changes the result: without it a text value is treated as a string to be quoted, while with it the value is treated as already-formatted JSON. To fix, record the input FORMAT in a new deparse-only field of JsonConstructorExpr, alongside orig_query, and emit it in ruleutils.c. Bump catalog version. Author: Chao Li Reviewed-by: Ewan Young Reviewed-by: Richard Guo Discussion: https://postgr.es/m/4C89B193-7D54-4705-9CF9-F0D484B9E099@gmail.com Backpatch-through: 19 --- src/backend/parser/parse_expr.c | 3 +++ src/backend/utils/adt/ruleutils.c | 1 + src/include/catalog/catversion.h | 2 +- src/include/nodes/primnodes.h | 5 +++++ src/test/regress/expected/sqljson.out | 7 +++++++ src/test/regress/sql/sqljson.sql | 8 ++++++++ 6 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index e6ea34a780..30c889f505 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -3808,6 +3808,8 @@ transformJsonObjectConstructor(ParseState *pstate, JsonObjectConstructor *ctor) * - orig_query: the transformed Query of the user's original subquery, so * that ruleutils.c can deparse the original JSON_ARRAY(SELECT ...) syntax * for view definitions. + * + * - format: the input FORMAT clause, so that ruleutils.c can deparse it. */ static Node * transformJsonArrayQueryConstructor(ParseState *pstate, @@ -3944,6 +3946,7 @@ transformJsonArrayQueryConstructor(ParseState *pstate, false, ctor->absent_on_null, ctor->location); ((JsonConstructorExpr *) result)->orig_query = (Node *) query; + ((JsonConstructorExpr *) result)->format = ctor->format; return result; } diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 1b44b7a78d..043e43b630 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -12291,6 +12291,7 @@ get_json_constructor(JsonConstructorExpr *ctor, deparse_context *context, context->prettyFlags, context->wrapColumn, context->indentLevel); + get_json_format(ctor->format, buf); get_json_constructor_options(ctor, buf); appendStringInfoChar(buf, ')'); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index d0399cc1cb..83d462f4d4 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202607201 +#define CATALOG_VERSION_NO 202607271 #endif diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index cacef7d415..1f71266651 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1718,6 +1718,10 @@ typedef enum JsonConstructorType * orig_query holds the user's original subquery for JSON_ARRAY(query), used * only by ruleutils.c for deparsing; it is not walked because func is * authoritative for all other purposes. + * + * format likewise holds the input FORMAT clause of JSON_ARRAY(query), which + * is otherwise only represented inside func; it is used only by ruleutils.c + * for deparsing. */ typedef struct JsonConstructorExpr { @@ -1728,6 +1732,7 @@ typedef struct JsonConstructorExpr Expr *coercion; /* coercion to RETURNING type */ JsonReturning *returning; /* RETURNING clause */ Node *orig_query; /* original subquery for deparsing */ + JsonFormat *format; /* input FORMAT for JSON_ARRAY(query) */ bool absent_on_null; /* ABSENT ON NULL? */ bool unique; /* WITH UNIQUE KEYS? (JSON_OBJECT[AGG] only) */ ParseLoc location; diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index 091a0b9857..d72278d67c 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -1233,6 +1233,13 @@ CREATE OR REPLACE VIEW public.json_array_subquery_view AS SELECT JSON_ARRAY( SELECT foo.i FROM ( VALUES (1), (2), (NULL::integer), (4)) foo(i) RETURNING text) AS "json_array" DROP VIEW json_array_subquery_view; +-- JSON_ARRAY(subquery) with an input FORMAT clause +CREATE VIEW json_array_subquery_view AS +SELECT JSON_ARRAY(SELECT '{"a": 1}'::text FORMAT JSON); +\sv json_array_subquery_view +CREATE OR REPLACE VIEW public.json_array_subquery_view AS + SELECT JSON_ARRAY( SELECT '{"a": 1}'::text AS text FORMAT JSON RETURNING json) AS "json_array" +DROP VIEW json_array_subquery_view; -- Test mutability of JSON_OBJECTAGG, JSON_ARRAYAGG, JSON_ARRAY, JSON_OBJECT create type comp1 as (a int, b date); create domain d_comp1 as comp1; diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql index 2550da15c4..96217a5593 100644 --- a/src/test/regress/sql/sqljson.sql +++ b/src/test/regress/sql/sqljson.sql @@ -443,6 +443,14 @@ SELECT JSON_ARRAY(SELECT i FROM (VALUES (1), (2), (NULL), (4)) foo(i) RETURNING DROP VIEW json_array_subquery_view; +-- JSON_ARRAY(subquery) with an input FORMAT clause +CREATE VIEW json_array_subquery_view AS +SELECT JSON_ARRAY(SELECT '{"a": 1}'::text FORMAT JSON); + +\sv json_array_subquery_view + +DROP VIEW json_array_subquery_view; + -- Test mutability of JSON_OBJECTAGG, JSON_ARRAYAGG, JSON_ARRAY, JSON_OBJECT create type comp1 as (a int, b date); create domain d_comp1 as comp1; From b8d9cf512c1259f97f9896593cc1c8352c1118ac Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Mon, 27 Jul 2026 09:06:05 +0530 Subject: [PATCH 40/43] Fix issues in logical replication sequence synchronization. 1. Stop a running sequence synchronization worker when ALTER SUBSCRIPTION ... DISABLE is executed. The worker did not reread its subscription after starting a transaction, so it kept running with a stale copy and missed the disable. It now calls maybe_reread_subscription() after StartTransactionCommand(), matching the apply worker. 2. Restore the invariant that publisher-side synchronization slots are dropped last during ALTER SUBSCRIPTION ... REFRESH PUBLICATION. The slot-drop loop now runs after the sequence-removal loop, so the non-transactional slot drops happen only after all catalog changes that could still be rolled back on error. 3. Restore psql tab completion for ALTER SUBSCRIPTION ... REFRESH PUBLICATION WITH (. 4. Make pg_stat_subscription report NULL for the fields that do not apply to a sequence synchronization worker, which does not stream from a walsender, and update the documentation accordingly. 5. Update the pg_subscription_rel.srsublsn catalog documentation to describe its semantics for sequence rows. Reported-by: Noah Misch Author: vignesh C Reviewed-by: Hayato Kuroda Reviewed-by: Amit Kapila Backpatch-through: 19, where it was introduced Discussion: https://postgr.es/m/20260710045217.f0.noahmisch@microsoft.com --- doc/src/sgml/catalogs.sgml | 6 +- doc/src/sgml/monitoring.sgml | 19 ++++--- src/backend/commands/subscriptioncmds.c | 56 +++++++++---------- .../replication/logical/sequencesync.c | 2 + src/backend/replication/logical/worker.c | 14 ++++- src/bin/psql/tab-complete.in.c | 3 + 6 files changed, 61 insertions(+), 39 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 4b474c1391..6066c4784f 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -8893,7 +8893,11 @@ SCRAM-SHA-256$<iteration count>:&l Remote LSN of the state change used for synchronization coordination when in s or r states, - otherwise null + otherwise null. For sequences, this instead holds the publisher + sequence's page LSN as of the last synchronization, which does not + track replication progress the way it does for tables; see + for how it is used to detect + out-of-sync sequences. diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 1ce0ef0079..a209e891b1 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -2473,8 +2473,8 @@ description | Waiting for a newly initialized WAL file to reach durable storage Process ID of the leader apply worker if this process is a parallel - apply worker; NULL if this process is a leader apply worker or a table - synchronization worker + apply worker; NULL if this process is a leader apply worker, a table + synchronization worker or a sequence synchronization worker @@ -2484,7 +2484,8 @@ description | Waiting for a newly initialized WAL file to reach durable storage OID of the relation that the worker is synchronizing; NULL for the - leader apply worker and parallel apply workers + leader apply worker, parallel apply workers and the sequence + synchronization worker @@ -2494,7 +2495,8 @@ description | Waiting for a newly initialized WAL file to reach durable storage Last write-ahead log location received, the initial value of - this field being 0; NULL for parallel apply workers + this field being 0; NULL for parallel apply workers and the sequence + synchronization worker @@ -2504,7 +2506,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage Send time of last message received from origin WAL sender; NULL for - parallel apply workers + parallel apply workers and the sequence synchronization worker @@ -2514,7 +2516,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage Receipt time of last message received from origin WAL sender; NULL for - parallel apply workers + parallel apply workers and the sequence synchronization worker @@ -2524,7 +2526,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage Last write-ahead log location reported to origin WAL sender; NULL for - parallel apply workers + parallel apply workers and the sequence synchronization worker @@ -2534,7 +2536,8 @@ description | Waiting for a newly initialized WAL file to reach durable storage Time of last write-ahead log location reported to origin WAL - sender; NULL for parallel apply workers + sender; NULL for parallel apply workers and the sequence synchronization + worker diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index d4504b4a0c..013ac46db0 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -1288,34 +1288,6 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data, } } - /* - * Drop the tablesync slots associated with removed tables. This has - * to be at the end because otherwise if there is an error while doing - * the database operations we won't be able to rollback dropped slots. - */ - foreach_ptr(SubRemoveRels, sub_remove_rel, sub_remove_rels) - { - if (sub_remove_rel->state != SUBREL_STATE_READY && - sub_remove_rel->state != SUBREL_STATE_SYNCDONE) - { - char syncslotname[NAMEDATALEN] = {0}; - - /* - * For READY/SYNCDONE states we know the tablesync slot has - * already been dropped by the tablesync worker. - * - * For other states, there is no certainty, maybe the slot - * does not exist yet. Also, if we fail after removing some of - * the slots, next time, it will again try to drop already - * dropped slots and fail. For these reasons, we allow - * missing_ok = true for the drop. - */ - ReplicationSlotNameForTablesync(sub->oid, sub_remove_rel->relid, - syncslotname, sizeof(syncslotname)); - ReplicationSlotDropAtPubNode(wrconn, syncslotname, true); - } - } - /* * Next remove state for sequences we should not care about anymore * using the data we collected above @@ -1343,6 +1315,34 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data, sub->name)); } } + + /* + * Drop the tablesync slots associated with removed tables. This has + * to be at the end because otherwise if there is an error while doing + * the database operations we won't be able to rollback dropped slots. + */ + foreach_ptr(SubRemoveRels, sub_remove_rel, sub_remove_rels) + { + if (sub_remove_rel->state != SUBREL_STATE_READY && + sub_remove_rel->state != SUBREL_STATE_SYNCDONE) + { + char syncslotname[NAMEDATALEN] = {0}; + + /* + * For READY/SYNCDONE states we know the tablesync slot has + * already been dropped by the tablesync worker. + * + * For other states, there is no certainty, maybe the slot + * does not exist yet. Also, if we fail after removing some of + * the slots, next time, it will again try to drop already + * dropped slots and fail. For these reasons, we allow + * missing_ok = true for the drop. + */ + ReplicationSlotNameForTablesync(sub->oid, sub_remove_rel->relid, + syncslotname, sizeof(syncslotname)); + ReplicationSlotDropAtPubNode(wrconn, syncslotname, true); + } + } } PG_FINALLY(); { diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c index d0370056de..fe506a98c2 100644 --- a/src/backend/replication/logical/sequencesync.c +++ b/src/backend/replication/logical/sequencesync.c @@ -495,6 +495,7 @@ copy_sequences(WalReceiverConn *conn) TupleTableSlot *slot; StartTransactionCommand(); + maybe_reread_subscription(); for (int idx = cur_batch_base_index; idx < n_seqinfos; idx++) { @@ -724,6 +725,7 @@ LogicalRepSyncSequences(void) StringInfoData app_name; StartTransactionCommand(); + maybe_reread_subscription(); rel = table_open(SubscriptionRelRelationId, AccessShareLock); diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 0ff5cef63c..0bd1907401 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -5985,8 +5985,18 @@ SetupApplyOrSyncWorker(int worker_slot) */ /* Initialise stats to a sanish value */ - MyLogicalRepWorker->last_send_time = MyLogicalRepWorker->last_recv_time = - MyLogicalRepWorker->reply_time = GetCurrentTimestamp(); + if (am_sequencesync_worker()) + { + MyLogicalRepWorker->last_send_time = + MyLogicalRepWorker->last_recv_time = + MyLogicalRepWorker->reply_time = 0; + } + else + { + MyLogicalRepWorker->last_send_time = + MyLogicalRepWorker->last_recv_time = + MyLogicalRepWorker->reply_time = GetCurrentTimestamp(); + } /* Load the libpq-specific functions */ load_file("libpqwalreceiver", false); diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index 1cacc8c3ea..17dcabe755 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -2354,6 +2354,9 @@ match_previous_words(int pattern_id, /* ALTER SUBSCRIPTION REFRESH */ else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, "REFRESH")) COMPLETE_WITH("PUBLICATION", "SEQUENCES"); + /* ALTER SUBSCRIPTION REFRESH PUBLICATION */ + else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, "REFRESH", "PUBLICATION")) + COMPLETE_WITH("WITH ("); /* ALTER SUBSCRIPTION REFRESH PUBLICATION WITH ( */ else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, "REFRESH", "PUBLICATION", "WITH", "(")) COMPLETE_WITH("copy_data"); From 4b1167e6480e6c547b36f83d3a01861488220759 Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Tue, 10 Feb 2026 12:47:32 +0500 Subject: [PATCH 41/43] Add archive_mode=shared for coordinated WAL archiving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a new archive_mode setting "shared" to prevent WAL history loss during standby promotion in HA streaming replication setups. In shared mode, the standby tracks which files have been archived by the primary. The standby refrains from recycling files that the primary has not yet archived, and at failover, the standby archives all those files too from the old timeline. This prevents WAL from being recycled before it's safely archived, addressing a possible gap in PITR continuity during failover. Primary sends last archived WAL segment every `archive_status_report_interval` ms via new walsender protocol message. Standby marks all segments <= reported segment as .done on current timeline. For any ancestor timeline, standby marks all segments which are in its history as .done. Cascading replication on each standby coordinates with immediate upstream. Standby relays its primary last archived WAL donwstream. Implementaion based on Heikki Linnakangas's 2014 design & patсh, also we grabbed some ideas & tests from Greenplum's production implementation[0], and modernized the whole thing for current HEAD (PostgreSQL 20 at time). Includes TAP tests covering basic synchronization, promotion, cascading replication. Also includes additional test for checkpoint wal recycling on standby logic in shared-archive mode. Author: Andrey Borodin Co-authored-by: Heikki Linnakangas (earlier versions) Co-authored-by: Kirill Reshke Reviewed-by: Fujii Masao (earlier versions) Reviewed-by: Grigory Smolkin Reviewed-by: Jaroslav Novikov [0] https://github.com/open-gpdb/gpdb/commit/4f2db1929df1b5eed28f33505955636096bb4e8b --- doc/src/sgml/config.sgml | 36 ++- doc/src/sgml/high-availability.sgml | 72 +++-- doc/src/sgml/protocol.sgml | 26 ++ src/backend/access/transam/xlog.c | 2 + src/backend/access/transam/xlogarchive.c | 14 +- src/backend/commands/subscriptioncmds.c | 10 +- src/backend/postmaster/pgarch.c | 16 +- .../libpqwalreceiver/libpqwalreceiver.c | 9 +- .../replication/logical/sequencesync.c | 2 +- src/backend/replication/logical/slotsync.c | 2 +- src/backend/replication/logical/tablesync.c | 2 +- src/backend/replication/logical/worker.c | 2 +- src/backend/replication/slotfuncs.c | 2 +- src/backend/replication/walreceiver.c | 257 +++++++++++++++++- src/backend/replication/walsender.c | 108 ++++++++ src/backend/tcop/backend_startup.c | 10 + src/backend/utils/misc/guc_parameters.dat | 9 + src/backend/utils/misc/postgresql.conf.sample | 6 +- src/include/access/xlog.h | 2 + src/include/libpq/protocol.h | 1 + src/include/postmaster/pgarch.h | 24 ++ src/include/replication/walreceiver.h | 5 +- src/include/replication/walsender.h | 1 + src/interfaces/libpq/fe-connect.c | 5 + src/interfaces/libpq/fe-protocol3.c | 2 + src/interfaces/libpq/libpq-int.h | 1 + src/test/recovery/meson.build | 2 + src/test/recovery/t/055_archive_shared.pl | 212 +++++++++++++++ .../t/056_archive_shared_checkpoint.pl | 214 +++++++++++++++ 29 files changed, 990 insertions(+), 64 deletions(-) create mode 100644 src/test/recovery/t/055_archive_shared.pl create mode 100644 src/test/recovery/t/056_archive_shared_checkpoint.pl diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index aa7b1bd75d..55d40ecb2c 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4048,14 +4048,36 @@ include_dir 'conf.d' are sent to archive storage by setting or . In addition to off, - to disable, there are two modes: on, and - always. During normal operation, there is no - difference between the two modes, but when set to always - the WAL archiver is enabled also during archive recovery or standby - mode. In always mode, all files restored from the archive - or streamed with streaming physical replication will be archived (again). See - for details. + to disable, there are three modes: on, shared, + and always. During normal operation as a primary, there is no + difference between the three modes, but they differ during archive recovery or + standby mode: + + + + on: Archives WAL only when running as a primary. + + + + + shared: Coordinates archiving between primary and standby. + The standby defers WAL archival and deletion until the primary confirms + archival via streaming replication. This prevents WAL history loss during + standby promotion in high availability setups. Upon promotion, the standby + automatically starts archiving any remaining unarchived WAL. This mode works + with cascading replication, where each standby coordinates with its immediate + upstream server. See for details. + + + + + always: Archives all WAL independently, even during recovery. + All files restored from the archive or streamed with streaming physical + replication will be archived (again), regardless of their source. + + + archive_mode is a separate setting from archive_command and diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index fd338ab154..e5b455cc10 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -1452,35 +1452,61 @@ postgres=# WAIT FOR LSN '0/306EE20'; - When continuous WAL archiving is used in a standby, there are two - different scenarios: the WAL archive can be shared between the primary - and the standby, or the standby can have its own WAL archive. When - the standby has its own WAL archive, set archive_mode + When continuous WAL archiving is used in a standby, there are three + different scenarios: the standby can have its own independent WAL archive, + the WAL archive can be shared between the primary and standby, or archiving + can be coordinated between them. + + + + For an independent archive, set archive_mode to always, and the standby will call the archive command for every WAL segment it receives, whether it's by restoring - from the archive or by streaming replication. The shared archive can - be handled similarly, but the archive_command or archive_library must - test if the file being archived exists already, and if the existing file - has identical contents. This requires more care in the - archive_command or archive_library, as it must - be careful to not overwrite an existing file with different contents, - but return success if the exactly same file is archived twice. And - all that must be done free of race conditions, if two servers attempt - to archive the same file at the same time. + from the archive or by streaming replication. + + + + For a shared archive where both primary and standby can write, use + always mode as well, but the archive_command + or archive_library must test if the file being archived + exists already, and if the existing file has identical contents. This requires + more care in the archive_command or archive_library, + as it must be careful to not overwrite an existing file with different contents, + but return success if the exactly same file is archived twice. And all that must + be done free of race conditions, if two servers attempt to archive the same file + at the same time. + + + + For coordinated archiving in high availability setups, use + archive_mode=shared. In this mode, only + the primary archives WAL segments. The standby creates .ready + files for received segments but defers actual archiving. The primary periodically + sends archival status updates to the standby via streaming replication, informing + it which segments have been archived. The standby then marks these as archived + and allows them to be recycled. Upon promotion, the standby automatically starts + archiving any remaining WAL segments that weren't confirmed as archived by the + former primary. This prevents WAL history loss during failover while avoiding + the complexity of coordinating concurrent archiving. This mode works with cascading + replication, where each standby coordinates with its immediate upstream server. If archive_mode is set to on, the - archiver is not enabled during recovery or standby mode. If the standby - server is promoted, it will start archiving after the promotion, but - will not archive any WAL or timeline history files that - it did not generate itself. To get a complete - series of WAL files in the archive, you must ensure that all WAL is - archived, before it reaches the standby. This is inherently true with - file-based log shipping, as the standby can only restore files that - are found in the archive, but not if streaming replication is enabled. - When a server is not in recovery mode, there is no difference between - on and always modes. + archiver is not enabled during recovery or standby mode, and this setting + cannot be used on a standby. If a standby with archive_mode + set to on is promoted, it will start archiving after the + promotion, but will not archive any WAL or timeline history files that it did + not generate itself. To get a complete series of WAL files in the archive, you + must ensure that all WAL is archived before it reaches the standby. This is + inherently true with file-based log shipping, as the standby can only restore + files that are found in the archive, but not if streaming replication is enabled. + + + + When a server is not in recovery mode, on, + shared, and always modes all behave + identically, archiving completed WAL segments. diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 49f8167671..5a66944304 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -2839,6 +2839,32 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" + + + WAL archival report message (B) + + + + Byte1('a') + + + Identifies the message as a last archived WAL segment update. + + + + + + Byten + + + Filename of the latest archived file. + + + + + + + diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f8b939853e..1496c68550 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -123,6 +123,7 @@ int min_wal_size_mb = 80; /* 80 MB */ int wal_keep_size_mb = 0; int XLOGbuffers = -1; int XLogArchiveTimeout = 0; +int XLogArchiveStatusReportInterval = 0; int XLogArchiveMode = ARCHIVE_MODE_OFF; char *XLogArchiveCommand = NULL; bool EnableHotStandby = false; @@ -199,6 +200,7 @@ const struct config_enum_entry archive_mode_options[] = { {"always", ARCHIVE_MODE_ALWAYS, false}, {"on", ARCHIVE_MODE_ON, false}, {"off", ARCHIVE_MODE_OFF, false}, + {"shared", ARCHIVE_MODE_SHARED, false}, {"true", ARCHIVE_MODE_ON, true}, {"false", ARCHIVE_MODE_OFF, true}, {"yes", ARCHIVE_MODE_ON, true}, diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 9a0c8097cb..62360cc486 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -574,16 +574,22 @@ XLogArchiveCheckDone(const char *xlog) /* * During archive recovery, the file is deletable if archive_mode is not - * "always". + * "always" or "shared". + * + * In "shared" mode the standby does not archive independently; instead it + * waits for the primary to report successful archival, at which point the + * walreceiver converts the .ready file to .done. We must therefore fall + * through to the .done/.ready check below so that checkpoint cannot + * delete a segment whose .ready file has not yet become .done. */ - if (!XLogArchivingAlways() && + if (!XLogArchivingAlways() && !(XLogArchiveMode == ARCHIVE_MODE_SHARED) && GetRecoveryState() == RECOVERY_STATE_ARCHIVE) return true; /* * At this point of the logic, note that we are either a primary with - * archive_mode set to "on" or "always", or a standby with archive_mode - * set to "always". + * archive_mode set to "on" or "always", a standby with archive_mode set + * to "always", or a standby with archive_mode set to "shared". */ /* First check for .done --- this means archiver is done with it */ diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 013ac46db0..a939defd31 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -953,7 +953,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, /* Try to connect to the publisher. */ must_use_password = !superuser_arg(owner) && opts.passwordrequired; - wrconn = walrcv_connect(conninfo, true, true, must_use_password, + wrconn = walrcv_connect(conninfo, true, true, must_use_password, false, stmt->subname, &err); if (!wrconn) ereport(ERROR, @@ -1117,7 +1117,7 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data, /* Try to connect to the publisher. */ must_use_password = sub->passwordrequired && !sub->ownersuperuser; - wrconn = walrcv_connect(sub->conninfo, true, true, must_use_password, + wrconn = walrcv_connect(sub->conninfo, true, true, must_use_password, false, sub->name, &err); if (!wrconn) ereport(ERROR, @@ -1370,7 +1370,7 @@ AlterSubscription_refresh_seq(Subscription *sub) /* Try to connect to the publisher. */ must_use_password = sub->passwordrequired && !sub->ownersuperuser; - wrconn = walrcv_connect(sub->conninfo, true, true, must_use_password, + wrconn = walrcv_connect(sub->conninfo, true, true, must_use_password, false, sub->name, &err); if (!wrconn) ereport(ERROR, @@ -2417,7 +2417,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, */ must_use_password = sub->passwordrequired && !sub->ownersuperuser; wrconn = walrcv_connect(new_conninfo ? new_conninfo : sub->conninfo, - true, true, must_use_password, sub->name, + true, true, must_use_password, false, sub->name, &err); if (!wrconn) ereport(ERROR, @@ -2770,7 +2770,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) conninfo = subconninfo; if (conninfo) - wrconn = walrcv_connect(conninfo, true, true, must_use_password, + wrconn = walrcv_connect(conninfo, true, true, must_use_password, false, subname, &err); if (wrconn == NULL) diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 0f207ac035..ef1423133c 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -83,16 +83,6 @@ */ #define NUM_FILES_PER_DIRECTORY_SCAN 64 -/* Shared memory area for archiver process */ -typedef struct PgArchData -{ - int pgprocno; /* proc number of archiver process */ - - /* - * Forces a directory scan in pgarch_readyXlog(). - */ - pg_atomic_uint32 force_dir_scan; -} PgArchData; char *XLogArchiveLibrary = ""; char *arch_module_check_errdetail_string; @@ -103,7 +93,7 @@ char *arch_module_check_errdetail_string; * ---------- */ static time_t last_sigterm_time = 0; -static PgArchData *PgArch = NULL; +PgArchData *PgArch = NULL; static const ArchiveModuleCallbacks *ArchiveCallbacks; static ArchiveModuleState *archive_module_state; static MemoryContext archive_context; @@ -180,6 +170,10 @@ PgArchShmemInit(void *arg) MemSet(PgArch, 0, sizeof(PgArchData)); PgArch->pgprocno = INVALID_PROC_NUMBER; pg_atomic_init_u32(&PgArch->force_dir_scan, 0); + + PgArch->primary_last_archived[0] = '\0'; + + SpinLockInit(&PgArch->lock); } /* diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c index 86d31c4659..e438cba137 100644 --- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c +++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c @@ -56,6 +56,7 @@ struct WalReceiverConn static WalReceiverConn *libpqrcv_connect(const char *conninfo, bool replication, bool logical, bool must_use_password, + bool expect_archive_reports, const char *appname, char **err); static void libpqrcv_check_conninfo(const char *conninfo, bool must_use_password); @@ -146,7 +147,7 @@ _PG_init(void) */ static WalReceiverConn * libpqrcv_connect(const char *conninfo, bool replication, bool logical, - bool must_use_password, const char *appname, char **err) + bool must_use_password, bool expect_archive_reports, const char *appname, char **err) { WalReceiverConn *conn; const char *keys[6]; @@ -216,6 +217,12 @@ libpqrcv_connect(const char *conninfo, bool replication, bool logical, keys[++i] = "fallback_application_name"; vals[i] = appname; + if (expect_archive_reports) + { + keys[++i] = "archive_status_reports"; + vals[i] = "true"; + } + keys[++i] = NULL; vals[i] = NULL; diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c index fe506a98c2..8e6f88354f 100644 --- a/src/backend/replication/logical/sequencesync.c +++ b/src/backend/replication/logical/sequencesync.c @@ -806,7 +806,7 @@ LogicalRepSyncSequences(void) * Establish the connection to the publisher for sequence synchronization. */ LogRepWorkerWalRcvConn = - walrcv_connect(MySubscription->conninfo, true, true, + walrcv_connect(MySubscription->conninfo, true, true, false, must_use_password, app_name.data, &err); if (LogRepWorkerWalRcvConn == NULL) diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c index d193682350..be0fdddd3d 100644 --- a/src/backend/replication/logical/slotsync.c +++ b/src/backend/replication/logical/slotsync.c @@ -1701,7 +1701,7 @@ ReplSlotSyncWorkerMain(const void *startup_data, size_t startup_data_len) * Establish the connection to the primary server for slot * synchronization. */ - wrconn = walrcv_connect(PrimaryConnInfo, false, false, false, + wrconn = walrcv_connect(PrimaryConnInfo, false, false, false, false, app_name.data, &err); if (!wrconn) diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index a04b84ebc1..693a0c3c81 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -1306,7 +1306,7 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) */ LogRepWorkerWalRcvConn = walrcv_connect(MySubscription->conninfo, true, true, - must_use_password, + must_use_password, false, slotname, &err); if (LogRepWorkerWalRcvConn == NULL) ereport(ERROR, diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 0bd1907401..98c522e739 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -5719,7 +5719,7 @@ run_apply_worker(void) !MySubscription->ownersuperuser; LogRepWorkerWalRcvConn = walrcv_connect(MySubscription->conninfo, true, - true, must_use_password, + true, must_use_password, false, MySubscription->name, &err); if (LogRepWorkerWalRcvConn == NULL) diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c index 16fbd38373..6fa5f2e5e2 100644 --- a/src/backend/replication/slotfuncs.c +++ b/src/backend/replication/slotfuncs.c @@ -944,7 +944,7 @@ pg_sync_replication_slots(PG_FUNCTION_ARGS) appendStringInfoString(&app_name, "slotsync"); /* Connect to the primary server. */ - wrconn = walrcv_connect(PrimaryConnInfo, false, false, false, + wrconn = walrcv_connect(PrimaryConnInfo, false, false, false, false, app_name.data, &err); if (!wrconn) diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 429a1b2d96..8f90d19bf7 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -69,6 +69,7 @@ #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/ipc.h" +#include "storage/fd.h" #include "storage/proc.h" #include "storage/procarray.h" #include "storage/procsignal.h" @@ -133,6 +134,17 @@ static TimestampTz wakeup[NUM_WALRCV_WAKEUPS]; static StringInfoData reply_message; +static TimeLineID primary_last_archived_tli = 0; +static XLogSegNo primary_last_archived_segno = 0; + +/* + * Last segment we successfully marked as .done. Used to optimize + * ProcessArchivalReport() by generating expected filenames instead + * of scanning the archive_status directory. + */ +static TimeLineID last_processed_tli = 0; +static XLogSegNo last_processed_segno = 0; + /* Prototypes for private functions */ static void WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last); static void WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI); @@ -146,6 +158,7 @@ static void XLogWalRcvClose(XLogRecPtr recptr, TimeLineID tli); static void XLogWalRcvSendReply(bool force, bool requestReply, bool checkApply); static void XLogWalRcvSendHSFeedback(bool immed); static void ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime); +static void ProcessArchivalReport(const char *primary_last_archived); static void WalRcvComputeNextWakeup(WalRcvWakeupReason reason, TimestampTz now); @@ -283,7 +296,7 @@ WalReceiverMain(const void *startup_data, size_t startup_data_len) /* Establish the connection to the primary for XLOG streaming */ appname = cluster_name[0] ? cluster_name : "walreceiver"; - wrconn = walrcv_connect(conninfo, true, false, false, appname, &err); + wrconn = walrcv_connect(conninfo, true, false, false, XLogArchiveMode == ARCHIVE_MODE_SHARED, appname, &err); if (!wrconn) ereport(ERROR, (errcode(ERRCODE_CONNECTION_FAILURE), @@ -846,6 +859,7 @@ XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len, TimeLineID tli) XLogRecPtr walEnd; TimestampTz sendTime; bool replyRequested; + char primary_last_archived[MAX_XFN_CHARS + 1]; switch (type) { @@ -898,6 +912,31 @@ XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len, TimeLineID tli) XLogWalRcvSendReply(true, false, false); break; } + case PqReplMsg_ArchiveStatusReport: + { + /* Check that the filename looks valid */ + if (len >= sizeof(primary_last_archived)) + ereport(ERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg_internal("invalid archival report message with length %d, expected at most %zu", + (int) len, sizeof(primary_last_archived)))); + + memcpy(primary_last_archived, buf, len); + primary_last_archived[len] = '\0'; + + /* Verify it contains only valid characters */ + if (!IsXLogFileName(primary_last_archived)) + ereport(ERROR, + errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg_internal("unexpected character in primary's last archived filename")); + + SpinLockAcquire(&PgArch->lock); + memcpy(PgArch->primary_last_archived, primary_last_archived, sizeof(PgArch->primary_last_archived)); + SpinLockRelease(&PgArch->lock); + + ProcessArchivalReport(primary_last_archived); + break; + } default: ereport(ERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), @@ -1100,12 +1139,44 @@ XLogWalRcvClose(XLogRecPtr recptr, TimeLineID tli) /* * Create .done file forcibly to prevent the streamed segment from being - * archived later. + * archived later, unless archive_mode is 'always' or 'shared'. + * + * In 'always' mode, the standby archives independently. + * + * In 'shared' mode, we optimize by checking if this segment is already + * covered by the last archival report from the primary. If so, create + * .done directly. Otherwise, create .ready and wait for the next report. */ - if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) - XLogArchiveForceDone(xlogfname); - else + if (XLogArchiveMode == ARCHIVE_MODE_ALWAYS) + { XLogArchiveNotify(xlogfname); + } + else if (XLogArchiveMode == ARCHIVE_MODE_SHARED) + { + /* + * In shared mode, check if this segment is already archived on primary. + * If we're on the same timeline and this segment is <= last archived, + * mark it .done immediately. Otherwise create .ready. + * + * We don't check ancestor timeline cases here to avoid reading timeline + * history files on every segment close. ProcessArchivalReport() will + * handle marking ancestor timeline segments as .done when it scans + * the archive_status directory. + */ + if (primary_last_archived_tli == recvFileTLI && + recvSegNo <= primary_last_archived_segno) + { + XLogArchiveForceDone(xlogfname); + } + else + { + XLogArchiveNotify(xlogfname); + } + } + else + { + XLogArchiveForceDone(xlogfname); + } recvFile = -1; } @@ -1296,6 +1367,182 @@ XLogWalRcvSendHSFeedback(bool immed) primary_has_standby_xmin = false; } +/* + * Process archival report from primary. + * + * The primary sends us the last WAL segment it has archived. We scan the + * archive_status directory for .ready files and mark segments on the same + * timeline as .done if they're <= the reported segment. + */ +static void +ProcessArchivalReport(const char *primary_last_archived) +{ + TimeLineID reported_tli; + XLogSegNo reported_segno; + char status_path[MAXPGPATH]; + bool use_direct_check = false; + XLogSegNo start_segno; + + elog(DEBUG2, "received archival report from primary: %s", + primary_last_archived); + + XLogFromFileName(primary_last_archived, &reported_tli, &reported_segno, + wal_segment_size); + + /* Remember the last archived segment for XLogWalRcvClose() */ + primary_last_archived_tli = reported_tli; + primary_last_archived_segno = reported_segno; + + /* + * Optimization: If the new report is on the same timeline as the last + * processed segment and moves forward, we can directly check for .ready + * files for segments between last_processed_segno and reported_segno + * instead of scanning the entire archive_status directory. + * + * Fall back to directory scan if: + * - Timeline changed (need to handle ancestor timelines) + * - This is the first report (last_processed_tli == 0) + * - Reported segment is not ahead (nothing new to process) + */ + if (last_processed_tli == reported_tli && + last_processed_tli != 0 && + reported_segno > last_processed_segno) + { + use_direct_check = true; + start_segno = last_processed_segno + 1; + } + + if (use_direct_check) + { + /* + * Direct check: generate filenames for expected segments. + * XLogArchiveForceDone() will handle the case where .ready doesn't + * exist or .done already exists, so no need to stat() first. + */ + XLogSegNo segno; + + for (segno = start_segno; segno <= reported_segno; segno++) + { + char walfile[MAXFNAMELEN]; + + /* Generate WAL filename and mark as archived */ + XLogFileName(walfile, reported_tli, segno, wal_segment_size); + XLogArchiveForceDone(walfile); + elog(DEBUG3, "marked WAL segment %s as archived (primary archived up to %s)", + walfile, primary_last_archived); + + /* Track the last segment we processed */ + last_processed_tli = reported_tli; + last_processed_segno = segno; + } + } + else + { + /* + * Directory scan: needed when timeline changed or first report. + * This handles both same-timeline and ancestor-timeline cases. + */ + DIR *status_dir; + struct dirent *status_de; + List *tli_history = NIL; + + snprintf(status_path, MAXPGPATH, XLOGDIR "/archive_status"); + status_dir = AllocateDir(status_path); + if (status_dir == NULL) + { + elog(DEBUG2, "could not open archive_status directory: %m"); + return; + } + + while ((status_de = ReadDir(status_dir, status_path)) != NULL) + { + char *ready_suffix; + char walfile[MAXPGPATH]; + size_t namelen; + TimeLineID file_tli; + XLogSegNo file_segno; + + /* Look for .ready files only */ + ready_suffix = strstr(status_de->d_name, ".ready"); + if (ready_suffix == NULL || ready_suffix[6] != '\0') + continue; + + /* Extract WAL filename (remove .ready suffix) */ + namelen = ready_suffix - status_de->d_name; + memcpy(walfile, status_de->d_name, namelen); + walfile[namelen] = '\0'; + + /* Parse the WAL filename */ + if (!IsXLogFileName(walfile)) + continue; + + XLogFromFileName(walfile, &file_tli, &file_segno, wal_segment_size); + + /* + * Mark as .done if: + * 1. Same timeline and segment <= reported segment, OR + * 2. Ancestor timeline and segment is before the timeline switch point + * + * For ancestor timelines: if primary archived segment X on timeline T, + * then all segments on ancestor timelines before the switch to T must + * have been archived (they're required to reach timeline T). + */ + if (file_tli == reported_tli && file_segno <= reported_segno) + { + /* Same timeline, segment already archived */ + XLogArchiveForceDone(walfile); + elog(DEBUG3, "marked WAL segment %s as archived (primary archived up to %s)", + walfile, primary_last_archived); + } + else if (file_tli != reported_tli) + { + /* + * Different timeline - check if it's an ancestor and if this + * segment is before the timeline switch point. Only read timeline + * history if we haven't already (lazy loading). + * + * Note: Timelines form a tree structure, not a linear sequence, + * so we can't use < or > to compare them. + */ + if (tli_history == NIL) + tli_history = readTimeLineHistory(reported_tli); + + if (tliInHistory(file_tli, tli_history)) + { + XLogRecPtr switchpoint; + XLogSegNo switchpoint_segno; + + /* Get the point where we switched away from this timeline */ + switchpoint = tliSwitchPoint(file_tli, tli_history, NULL); + + /* + * If the segment is at or before the switch point, it must have + * been archived (it's required to reach the reported timeline). + * The segment containing the switch point belongs to the old + * timeline up to the switch point and should be archived. + */ + XLByteToSeg(switchpoint, switchpoint_segno, wal_segment_size); + if (file_segno <= switchpoint_segno) + { + XLogArchiveForceDone(walfile); + elog(DEBUG3, "marked ancestor timeline segment %s as archived (before switch to timeline %u)", + walfile, reported_tli); + } + } + } + } + + FreeDir(status_dir); + + /* + * After a full directory scan following a timeline change, update + * our tracking to the newly reported position for future optimizations. + */ + last_processed_tli = reported_tli; + last_processed_segno = reported_segno; + } +} + /* * Update shared memory status upon receiving a message from primary. * diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 35ebc7e61c..bfe43ef621 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -135,6 +135,7 @@ WalSnd *MyWalSnd = NULL; bool am_walsender = false; /* Am I a walsender process? */ bool am_cascading_walsender = false; /* Am I cascading WAL to another * standby? */ +bool am_archive_status_walsender = false; /* Am I replying with archive status reports? */ bool am_db_walsender = false; /* Connected to a database? */ /* GUC variables */ @@ -216,6 +217,14 @@ static TimestampTz shutdown_request_timestamp = 0; */ static bool shutdown_stream_done_queued = false; +/* + * Last archived WAL file. This is fetched from pgstat periodically and sent + * to the standby. last_archival_report_timestamp tracks when we last sent + * the report to avoid excessive pgstat access. + */ +static char last_archived_report_wal[MAX_XFN_CHARS + 1]; +static TimestampTz last_archival_report_timestamp = 0; + /* * While streaming WAL in Copy mode, streamingDoneSending is set to true * after we have sent CopyDone. We should not send any more CopyData messages @@ -304,6 +313,7 @@ static void ProcessStandbyMessage(void); static void ProcessStandbyReplyMessage(void); static void ProcessStandbyHSFeedbackMessage(void); static void ProcessStandbyPSRequestMessage(void); +static void WalSndArchivalReport(void); static void ProcessRepliesIfAny(void); static void ProcessPendingWrites(void); static void WalSndKeepalive(bool requestReply, XLogRecPtr writePtr); @@ -2844,6 +2854,101 @@ ProcessStandbyHSFeedbackMessage(void) } } +/* + * Send archival status report to standby. + * + * This is called periodically during physical replication to inform the + * standby about the last WAL segment archived by the primary. The standby + * can then mark segments up to that point as .done, allowing them to be + * recycled. This prevents WAL loss during standby promotion. + */ +static void +WalSndArchivalReport(void) +{ + PgStat_ArchiverStats *archiver_stats; + TimestampTz now; + char last_archived[MAX_XFN_CHARS + 1]; + + /* Only send reports when requested */ + if (!am_archive_status_walsender) + return; + + if (MyWalSnd->state != WALSNDSTATE_CATCHUP && + MyWalSnd->state != WALSNDSTATE_STREAMING) + return; + + /* + * Don't send to temporary replication slots (used by pg_basebackup). + * Connections without slots (regular standbys) are OK. + */ + if (MyReplicationSlot != NULL && + MyReplicationSlot->data.persistency == RS_TEMPORARY) + return; + + now = GetCurrentTimestamp(); + + /* + * Send report at most once per configured interval. + * This avoids excessive pgstat access. + */ + if (now < TimestampTzPlusMilliseconds(last_archival_report_timestamp, + XLogArchiveStatusReportInterval)) + return; + + last_archival_report_timestamp = now; + + /* In recovery, simply relay received message downstream. */ + if (RecoveryInProgress()) + { + SpinLockAcquire(&PgArch->lock); + if (PgArch->primary_last_archived[0] == '\0') + { + SpinLockRelease(&PgArch->lock); + return; + } + memcpy(last_archived, PgArch->primary_last_archived, sizeof(last_archived)); + SpinLockRelease(&PgArch->lock); + } + else + { + /* + * Get archiver statistics. The pgstat snapshot is cached per-session and + * is only invalidated at transaction boundaries. The walsender runs + * without transaction boundaries, so we must clear the snapshot explicitly + * to avoid reading stale data (e.g. last_archived_wal stuck at its initial + * empty value even after the archiver has archived new segments). + */ + pgstat_clear_snapshot(); + archiver_stats = pgstat_fetch_stat_archiver(); + if (archiver_stats == NULL) + return; + + memcpy(last_archived, archiver_stats->last_archived_wal, sizeof(last_archived)); + } + /* + * Only send a report if the last archived WAL has changed. This is both + * an optimization and ensures we don't send empty reports on startup. + */ + if (strcmp(last_archived, last_archived_report_wal) == 0) + return; + + /* Only send reports for WAL segments, not backup history files or other archived files */ + if (!IsXLogFileName(last_archived)) + return; + + elog(DEBUG2, "sending archival report: %s", last_archived); + + /* Remember what we sent */ + strlcpy(last_archived_report_wal, last_archived, sizeof(last_archived_report_wal)); + + /* Construct the message... */ + resetStringInfo(&output_message); + pq_sendbyte(&output_message, PqReplMsg_ArchiveStatusReport); + pq_sendbytes(&output_message, last_archived, strlen(last_archived)); + /* ... and send it wrapped in CopyData */ + pq_putmessage_noblock(PqMsg_CopyData, output_message.data, output_message.len); +} + /* * Process the request for a primary status update message. */ @@ -4467,6 +4572,9 @@ WalSndKeepaliveIfNecessary(void) if (pq_flush_if_writable() != 0) WalSndShutdown(); } + + /* Send archival status report if needed */ + WalSndArchivalReport(); } /* diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c index 25205cee0f..6df50d3c98 100644 --- a/src/backend/tcop/backend_startup.c +++ b/src/backend/tcop/backend_startup.c @@ -803,6 +803,16 @@ ProcessStartupPacket(Port *port) valptr), errhint("Valid values are: \"false\", 0, \"true\", 1, \"database\"."))); } + else if (strcmp(nameptr, "archive_status_reports") == 0) + { + if (!parse_bool(valptr, &am_archive_status_walsender)) + ereport(FATAL, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid value for parameter \"%s\": \"%s\"", + "archive_status_reports", + valptr), + errhint("Valid values are: \"false\", 0, \"true\", 1.")); + } else if (strncmp(nameptr, "_pq_.", 5) == 0) { /* diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index adb72361ce..949f73d7a4 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -95,6 +95,15 @@ options => 'archive_mode_options', }, +{ name => 'archive_status_report_interval', type => 'int', context => 'PGC_SIGHUP', group => 'WAL_ARCHIVING', + short_desc => 'Sets the amount of time between consecutive WAL archive status reports.', + flags => 'GUC_UNIT_MS', + variable => 'XLogArchiveStatusReportInterval', + boot_val => '10000', + min => '10', + max => 'INT_MAX / 2', +}, + { name => 'archive_timeout', type => 'int', context => 'PGC_SIGHUP', group => 'WAL_ARCHIVING', short_desc => 'Sets the amount of time to wait before forcing a switch to the next WAL file.', long_desc => '0 disables the timeout.', diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 7958653077..5715d36b26 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -291,8 +291,12 @@ # - Archiving - -#archive_mode = off # enables archiving; off, on, or always +#archive_mode = off # enables archiving; off, on, always or shared # (change requires restart) + +#archive_status_report_interval = 10s # configures archive status reports frequency in + # shared archive mode + #archive_library = '' # library to use to archive a WAL file # (empty string indicates archive_command should # be used) diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 4dd9862420..2aeb8ce14b 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -42,6 +42,7 @@ extern PGDLLIMPORT int wal_keep_size_mb; extern PGDLLIMPORT int max_slot_wal_keep_size_mb; extern PGDLLIMPORT int XLOGbuffers; extern PGDLLIMPORT int XLogArchiveTimeout; +extern PGDLLIMPORT int XLogArchiveStatusReportInterval; extern PGDLLIMPORT int wal_retrieve_retry_interval; extern PGDLLIMPORT char *XLogArchiveCommand; extern PGDLLIMPORT bool EnableHotStandby; @@ -67,6 +68,7 @@ typedef enum ArchiveMode ARCHIVE_MODE_OFF = 0, /* disabled */ ARCHIVE_MODE_ON, /* enabled while server is running normally */ ARCHIVE_MODE_ALWAYS, /* enabled always (even during recovery) */ + ARCHIVE_MODE_SHARED, /* shared archive between primary and standby */ } ArchiveMode; extern PGDLLIMPORT int XLogArchiveMode; diff --git a/src/include/libpq/protocol.h b/src/include/libpq/protocol.h index eae8f0e723..d22aaf9e22 100644 --- a/src/include/libpq/protocol.h +++ b/src/include/libpq/protocol.h @@ -72,6 +72,7 @@ /* Replication codes sent by the primary (wrapped in CopyData messages). */ +#define PqReplMsg_ArchiveStatusReport 'a' #define PqReplMsg_Keepalive 'k' #define PqReplMsg_PrimaryStatusUpdate 's' #define PqReplMsg_WALData 'w' diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h index 9772bb573a..1d8571fc5a 100644 --- a/src/include/postmaster/pgarch.h +++ b/src/include/postmaster/pgarch.h @@ -13,6 +13,9 @@ #ifndef _PGARCH_H #define _PGARCH_H +#include "port/atomics.h" +#include "storage/spin.h" + /* ---------- * Archiver control info. * @@ -31,4 +34,25 @@ pg_noreturn extern void PgArchiverMain(const void *startup_data, size_t startup_ extern void PgArchWakeup(void); extern void PgArchForceDirScan(void); +/* Shared memory area for archiver process */ +typedef struct PgArchData +{ + int pgprocno; /* proc number of archiver process */ + + /* Lock to protect the `primary_last_archived`. */ + slock_t lock; + + /* Last archived WAL segment file reported by the primary */ + char primary_last_archived[MAX_XFN_CHARS + 1]; + + /* + * Forces a directory scan in pgarch_readyXlog(). + */ + pg_atomic_uint32 force_dir_scan; +} PgArchData; + + +extern PgArchData *PgArch; + + #endif /* _PGARCH_H */ diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h index 760364e358..0e5459d3ac 100644 --- a/src/include/replication/walreceiver.h +++ b/src/include/replication/walreceiver.h @@ -244,6 +244,7 @@ typedef WalReceiverConn *(*walrcv_connect_fn) (const char *conninfo, bool replication, bool logical, bool must_use_password, + bool expect_archive_reports, const char *appname, char **err); @@ -433,8 +434,8 @@ typedef struct WalReceiverFunctionsType extern PGDLLIMPORT WalReceiverFunctionsType *WalReceiverFunctions; -#define walrcv_connect(conninfo, replication, logical, must_use_password, appname, err) \ - WalReceiverFunctions->walrcv_connect(conninfo, replication, logical, must_use_password, appname, err) +#define walrcv_connect(conninfo, replication, logical, must_use_password, expect_archive_reports, appname, err) \ + WalReceiverFunctions->walrcv_connect(conninfo, replication, logical, must_use_password, expect_archive_reports, appname, err) #define walrcv_check_conninfo(conninfo, must_use_password) \ WalReceiverFunctions->walrcv_check_conninfo(conninfo, must_use_password) #define walrcv_get_conninfo(conn) \ diff --git a/src/include/replication/walsender.h b/src/include/replication/walsender.h index 386cedfc7a..f2fbada75d 100644 --- a/src/include/replication/walsender.h +++ b/src/include/replication/walsender.h @@ -27,6 +27,7 @@ typedef enum /* global state */ extern PGDLLIMPORT bool am_walsender; extern PGDLLIMPORT bool am_cascading_walsender; +extern PGDLLIMPORT bool am_archive_status_walsender; extern PGDLLIMPORT bool am_db_walsender; extern PGDLLIMPORT bool wake_wal_senders; diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 17c2288e9b..9b1929fc39 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -380,6 +380,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Replication", "D", 5, offsetof(struct pg_conn, replication)}, + {"archive_status_reports", NULL, NULL, NULL, + "Replication", "D", 5, + offsetof(struct pg_conn, archive_status_reports)}, + {"target_session_attrs", "PGTARGETSESSIONATTRS", DefaultTargetSessionAttrs, NULL, "Target-Session-Attrs", "", 15, /* sizeof("prefer-standby") = 15 */ @@ -5118,6 +5122,7 @@ freePGconn(PGconn *conn) free(conn->fbappname); free(conn->dbName); free(conn->replication); + free(conn->archive_status_reports); free(conn->pgservice); free(conn->pgservicefile); free(conn->pguser); diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index 9d6a285fb2..ed7a3f6b9d 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -2513,6 +2513,8 @@ build_startup_packet(const PGconn *conn, char *packet, ADD_STARTUP_OPTION("database", conn->dbName); if (conn->replication && conn->replication[0]) ADD_STARTUP_OPTION("replication", conn->replication); + if (conn->archive_status_reports && conn->archive_status_reports[0]) + ADD_STARTUP_OPTION("archive_status_reports", conn->archive_status_reports); if (conn->pgoptions && conn->pgoptions[0]) ADD_STARTUP_OPTION("options", conn->pgoptions); if (conn->send_appname) diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 3f921207a1..8115db5e33 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -391,6 +391,7 @@ struct pg_conn char *fbappname; /* fallback application name */ char *dbName; /* database name */ char *replication; /* connect as the replication standby? */ + char *archive_status_reports; /* do we send primary last archived WAL to the standby? */ char *pgservice; /* Postgres service, if any */ char *pgservicefile; /* path to a service file containing * service(s) */ diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index ad0d85f418..7e0c6ba67c 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -63,6 +63,8 @@ tests += { 't/052_checkpoint_segment_missing.pl', 't/053_standby_login_event_trigger.pl', 't/054_unlogged_sequence_promotion.pl', + 't/055_archive_shared.pl', + 't/056_archive_shared_checkpoint.pl', ], }, } diff --git a/src/test/recovery/t/055_archive_shared.pl b/src/test/recovery/t/055_archive_shared.pl new file mode 100644 index 0000000000..5c70f5c511 --- /dev/null +++ b/src/test/recovery/t/055_archive_shared.pl @@ -0,0 +1,212 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test archive_mode=shared for coordinated WAL archiving between primary and standby +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; +use File::Path qw(rmtree); + +# Initialize primary node with archiving +my $archive_dir = PostgreSQL::Test::Utils::tempdir(); +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init(has_archiving => 1, allows_streaming => 1); + +my $archive_command = + $PostgreSQL::Test::Utils::windows_os + ? qq{copy "%p" "$archive_dir\\%f"} + : qq{cp "%p" "$archive_dir/%f"}; + +$primary->append_conf('postgresql.conf', " +archive_mode = shared +archive_status_report_interval = 10ms +archive_command = '$archive_command' +wal_keep_size = 128MB +"); +$primary->start; + +############################################################################### +# Test 1: Basic testing +############################################################################### + +# Ensure WAL activity exists in the current segment before switching. +# pg_switch_wal() is a no-op when called at the very start of a segment, +# so we write a bump transaction counter +# first to guarantee there is WAL to switch away from. +$primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); + +# Wait for archiver to archive segments +$primary->poll_query_until('postgres', + "SELECT archived_count > 0 FROM pg_stat_archiver") + or die "Timed out waiting for archiver to complete archiving"; + +my $archived_count = () = glob("$archive_dir/*"); +ok($archived_count > 0, "primary has archived WAL files to shared archive"); +note("Primary archived $archived_count files"); + +# Take backup for standby +my $backup_name = 'standby_backup'; +$primary->backup($backup_name); + +# Exclude possible race condition when backup WAL is last archived +$primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); + +# Set up standby with archive_mode=shared +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, $backup_name, has_streaming => 1); +$standby->append_conf('postgresql.conf', " +archive_mode = shared +archive_status_report_interval = 10ms +archive_command = '$archive_command' +wal_receiver_status_interval = 1s +"); +$standby->start; + +# Wait for standby to catch up +$primary->wait_for_catchup($standby); + +# Generate more WAL on primary (these are new segments not yet archived) +$primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); + +# Wait for standby to receive the new WAL +$primary->wait_for_catchup($standby); + +# Check that standby has .ready or .done files for the newly received segments. +# Normally they should be .ready (not yet archived by primary), but in rare cases +# the archiver could be very fast and an archive report sent immediately, creating +# .done files instead. Both are correct behavior - the key is that files exist. +my $standby_archive_status = $standby->data_dir . '/pg_wal/archive_status'; +my $status_count = 0; +if (opendir(my $dh, $standby_archive_status)) +{ + my @files = grep { /\.(ready|done)$/ } readdir($dh); + $status_count = scalar(@files); + my $ready_count = scalar(grep { /\.ready$/ } @files); + my $done_count = scalar(grep { /\.done$/ } @files); + note("Standby has $ready_count .ready files and $done_count .done files"); + closedir($dh); +} +cmp_ok($status_count, '>', 0, "standby creates archive status files for received WAL"); + +# Generate more WAL and wait for archiving on primary +my $initial_archived = $primary->safe_psql('postgres', 'SELECT archived_count FROM pg_stat_archiver'); +$primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); + +# Wait for primary to archive the new segments +$primary->poll_query_until('postgres', + "SELECT archived_count > $initial_archived FROM pg_stat_archiver") + or die "Timed out waiting for primary to archive new segments"; + +# Wait for standby to catch up (archive status is sent during replication) +$primary->wait_for_catchup($standby); + +# Wait for primary to send archival status updates and standby to process them +# The standby should mark segments as .done after receiving archive status from primary +my $done_count = 0; +for (my $i = 0; $i < $PostgreSQL::Test::Utils::timeout_default; $i++) +{ + $done_count = 0; + if (opendir(my $dh, $standby_archive_status)) + { + $done_count = scalar(grep { /\.done$/ } readdir($dh)); + closedir($dh); + } + last if $done_count > 0; + sleep(1); +} +ok($done_count > 0, "standby marked segments as .done after primary's archival report"); +note("Standby has $done_count .done files"); + +############################################################################### +# Test 2: Cascading replication +############################################################################### + +# Take a backup from the promoted standby (now the new primary) +my $promoted_backup = 'promoted_backup'; +$standby->backup($promoted_backup); + +# Set up second-level standby (cascading from first standby, now promoted) +my $cascade_standby = PostgreSQL::Test::Cluster->new('cascade_standby'); +$cascade_standby->init_from_backup($standby, $promoted_backup, has_streaming => 1); +$cascade_standby->append_conf('postgresql.conf', " +archive_mode = shared +archive_status_report_interval = 10ms +archive_command = '$archive_command' +wal_receiver_status_interval = 1s +"); +$cascade_standby->start; + +# Generate WAL +my $cascading_archived_before = $primary->safe_psql('postgres', 'SELECT archived_count FROM pg_stat_archiver'); + +my $current_walfile = $primary->safe_psql('postgres', "SELECT pg_walfile_name(pg_current_wal_lsn());"); + +$primary->safe_psql( + 'postgres', q{ + CHECKPOINT; + SELECT pg_switch_wal(); +}); + +my $walfile_ready = "pg_wal/archive_status/$current_walfile.ready"; +my $walfile_done = "pg_wal/archive_status/$current_walfile.done"; + +# Wait for the primary to send archive status +$primary->poll_query_until('postgres', + "SELECT archived_count > $cascading_archived_before FROM pg_stat_archiver") + or die "Timed out waiting for primary to archive segment in cascading test"; + +# Wait for cascading standby to catch up +$standby->wait_for_catchup($cascade_standby); + +my $cascade_data = $cascade_standby->data_dir; +my $cascade_standby_archive_status = $cascade_standby->data_dir . '/pg_wal/archive_status'; + +for (my $i = 0; $i < $PostgreSQL::Test::Utils::timeout_default; $i++) +{ + if (-f "$cascade_data/$walfile_done") + { + last; + } + sleep(1); +} + +# Wait for cascading standby to receive archive status and mark segments as .done +ok( !-f "$cascade_data/$walfile_ready", + ".ready file exists on cascade replica for WAL segment $current_walfile" +); +ok( -f "$cascade_data/$walfile_done", + ".done file exists on cascade replica for WAL segment $current_walfile" +); + + +############################################################################### +# Test 3: Standby promotion - verify archiver activates +############################################################################### + +# Before promotion, verify archiver is not running on standby (shared mode during recovery) +# In shared mode, the standby's archiver should not be archiving during recovery +my $archived_before = $standby->safe_psql('postgres', + "SELECT archived_count FROM pg_stat_archiver"); + +is($archived_before, '0', + "archiver not active on standby before promotion"); + +# Verify standby is still in recovery before promoting +is($standby->safe_psql('postgres', "SELECT pg_is_in_recovery();"), 't', "standby is in recovery before promotion"); + +# Promote the standby +$standby->promote; +$standby->poll_query_until('postgres', "SELECT NOT pg_is_in_recovery();"); + +# Generate WAL on new primary (former standby) +$standby->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); + +# Wait for archiver to activate and archive the new WAL +# Check pg_stat_archiver to verify archiving is happening +$standby->poll_query_until('postgres', + "SELECT archived_count > 0 FROM pg_stat_archiver") + or die "Timed out waiting for promoted standby to start archiving"; +pass("promoted standby started archiving"); + +done_testing(); diff --git a/src/test/recovery/t/056_archive_shared_checkpoint.pl b/src/test/recovery/t/056_archive_shared_checkpoint.pl new file mode 100644 index 0000000000..8083ee9951 --- /dev/null +++ b/src/test/recovery/t/056_archive_shared_checkpoint.pl @@ -0,0 +1,214 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Tests for archive_mode=shared correctness on standbys: +# +# 1. Checkpoint on standby must NOT remove WAL segments that have a .ready +# status file (i.e. not yet archived by the primary). With the bug, +# XLogArchiveCheckDone() returns true unconditionally during recovery for +# any mode that is not "always", so checkpoint deletes these segments. +# +# 2. After archiving is broken on the primary and then restored, .ready files +# on the standby must eventually transition to .done (primary sends archival +# status reports to the standby via the walsender). + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Use 1 MB WAL segments so we can generate many segments cheaply. +my $wal_segsize = 1; + +# An archive command that always fails (but is recognized by the archiver as a +# real failure, not a missing command). Mirrors the approach in +# 020_archive_status.pl to stay portable. +my $broken_command = + $PostgreSQL::Test::Utils::windows_os + ? q{copy "%p_does_not_exist" "%f_does_not_exist"} + : q{cp "%p_does_not_exist" "%f_does_not_exist"}; + +my $archive_dir = PostgreSQL::Test::Utils::tempdir(); +my $good_command = + $PostgreSQL::Test::Utils::windows_os + ? qq{copy "%p" "$archive_dir\\%f"} + : qq{cp %p "$archive_dir/%f"}; + +############################################################################### +# Set up primary with archive_mode=shared and BROKEN archiving so that every +# WAL segment received by the standby gets a .ready file. +############################################################################### + +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init( + has_archiving => 1, + allows_streaming => 1, + extra => [ '--wal-segsize' => $wal_segsize ]); +$primary->append_conf('postgresql.conf', qq{ +archive_mode = shared +archive_status_report_interval = 10ms +archive_command = '$broken_command' +wal_keep_size = 0 # to trigger wal deletion +}); +$primary->start; + +my $backup_name = 'standby_backup'; +$primary->backup($backup_name); + +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, $backup_name, has_streaming => 1); +$standby->append_conf('postgresql.conf', qq{ +archive_mode = shared +archive_status_report_interval = 10ms +archive_command = '$good_command' +wal_receiver_status_interval = 1s +wal_keep_size = 0 # to trigger wal deletion +}); +$standby->start; + +$primary->wait_for_catchup($standby); + +############################################################################### +# Generate WAL while archiving is broken. +# The standby will create .ready files for every received segment. +############################################################################### + +# Switch WAL several times to create clearly-identifiable old segments. +# We capture the name of the first switched-away segment; it is the primary +# candidate that checkpoint would delete. +my $target_seg = $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); + +for my $i (1..3) +{ + $primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); +} + +# Wait for the archiver to register failures so we are sure archiving is +# truly broken (not just slow). +$primary->poll_query_until('postgres', + q{SELECT failed_count > 0 FROM pg_stat_archiver}) + or die "Timed out waiting for archiver to fail"; + +# Issue a CHECKPOINT on the primary so that the standby can form a +# restartpoint whose redo LSN is past $target_seg. +$primary->safe_psql('postgres', 'CHECKPOINT'); + +# Wait for the standby to replay everything up to that checkpoint. +$primary->wait_for_catchup($standby); + +my $standby_wal_dir = $standby->data_dir . '/pg_wal'; +my $standby_status_dir = "$standby_wal_dir/archive_status"; + +# The target segment must already be visible on the standby as .ready. +my $target_ready = "$standby_status_dir/$target_seg.ready"; +ok(-f $target_ready, + "standby has .ready file for segment $target_seg (not archived by primary)"); + +# The WAL file itself must also be present. +ok(-f "$standby_wal_dir/$target_seg", + "WAL segment $target_seg exists in standby pg_wal before CHECKPOINT"); + +############################################################################### +# Test 1: CHECKPOINT (restartpoint) on standby must not remove .ready segments +############################################################################### + +# This triggers CreateRestartPoint, which calls RemoveOldXlogFiles. +# With the bug, XLogArchiveCheckDone returns true for every segment in +# archive_mode=shared during recovery, so $target_seg would be deleted. +$standby->safe_psql('postgres', 'CHECKPOINT'); + +ok(-f "$standby_wal_dir/$target_seg", + "WAL segment $target_seg still exists after CHECKPOINT on standby " + . "(not deleted despite .ready status)"); + +ok(-f $target_ready, + ".ready file for $target_seg still present after CHECKPOINT on standby"); + +############################################################################### +# Test 2: Restoring archiving on primary causes .ready -> .done on standby +# +# This part is independent of Test 1: we generate fresh WAL (with archiving +# still broken) so the standby accumulates new .ready files, then restore +# archiving and verify those files become .done. +############################################################################### + +# Generate a few more segments so the standby definitely has fresh .ready files +# regardless of what checkpoint may have done above. +for my $i (1..3) +{ + $primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); +} +$primary->wait_for_catchup($standby); + +# Collect all current .ready files on the standby. +my @ready_segs; +if (opendir(my $dh, $standby_status_dir)) +{ + @ready_segs = + map { (my $s = $_) =~ s/\.ready$//; $s } + grep { /\.ready$/ } readdir($dh); + closedir($dh); +} +note("Standby has " + . scalar(@ready_segs) + . " .ready segments before archiving is restored"); +cmp_ok(scalar(@ready_segs), '>', 0, + "standby has fresh .ready files for newly received unarchived segments"); + +# Restore archiving on the primary. +$primary->safe_psql('postgres', qq{ + ALTER SYSTEM SET archive_command TO '$good_command'; + SELECT pg_reload_conf(); +}); + +# Wait until primary has archived at least one segment. +$primary->poll_query_until('postgres', + q{SELECT archived_count > 0 FROM pg_stat_archiver}) + or die "Timed out waiting for primary to start archiving after restore"; + +# Generate one more WAL switch so the walsender picks up the updated +# last_archived_wal and sends a fresh archival report to the standby. +# (The walsender only sends when last_archived_wal changes and every +# archive_status_report_interval = 10 ms at most.) +$primary->safe_psql('postgres', 'SELECT pg_switch_wal()'); +$primary->wait_for_catchup($standby); + +# Poll until all previously .ready segments have become .done. +# Allow up to the framework default timeout (usually 120 s); the walsender +# reports every 10 ms so convergence should happen well within that. +my $remaining_ready = scalar(@ready_segs); +for my $i (1 .. $PostgreSQL::Test::Utils::timeout_default) +{ + $remaining_ready = 0; + if (opendir(my $dh, $standby_status_dir)) + { + # Count only the segments that were .ready before archiving was restored + for my $seg (@ready_segs) + { + $remaining_ready++ if -f "$standby_status_dir/$seg.ready"; + } + closedir($dh); + } + last if $remaining_ready == 0; + sleep(1); +} + +for my $seg (@ready_segs) +{ + ok( -f "$standby_status_dir/$seg.done", + "$seg .ready file on standby transitioned to .done" + ); +} + +is($remaining_ready, 0, + "all .ready files on standby transitioned to .done " + . "after archiving restored on primary"); + +# Sanity-check: the WAL files are still present (they weren't deleted by +# checkpoint while .ready, nor disappeared otherwise). +my @still_missing = grep { !-f "$standby_wal_dir/$_" } @ready_segs; +is(scalar(@still_missing), 0, + "WAL segments were not lost while waiting for archival reports"); + +done_testing(); From d18ff47c6b8b1ffbd22e92c0a6cda6b3f419d943 Mon Sep 17 00:00:00 2001 From: reshke Date: Mon, 20 Jul 2026 18:25:39 +0300 Subject: [PATCH 42/43] Add `primary_last_archived` column to pg_stat_wal_receiver view. Comes in-handy for monitoring shared archive feature. TAP tests for shared archive feature were extended to check for this column value. Bumps catversion. --- src/backend/catalog/system_views.sql | 1 + src/backend/replication/walreceiver.c | 25 +++++++++++++++-------- src/include/catalog/pg_proc.dat | 6 +++--- src/test/recovery/t/055_archive_shared.pl | 20 ++++++++++++++++++ src/test/regress/expected/rules.out | 3 ++- 5 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 090281a03d..5bd037aa59 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1025,6 +1025,7 @@ CREATE VIEW pg_stat_wal_receiver AS s.last_msg_receipt_time, s.latest_end_lsn, s.latest_end_time, + s.primary_last_archived, s.slot_name, s.sender_host, s.sender_port, diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 8f90d19bf7..145e60d5fc 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -1709,6 +1709,7 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS) int sender_port = 0; char slotname[NAMEDATALEN]; char conninfo[MAXCONNINFO]; + char primary_last_archived[MAX_XFN_CHARS + 1]; /* Take a lock to ensure value consistency */ SpinLockAcquire(&WalRcv->mutex); @@ -1736,6 +1737,10 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS) if (pid == 0 || !ready_to_display) PG_RETURN_NULL(); + SpinLockAcquire(&PgArch->lock); + strlcpy(primary_last_archived, PgArch->primary_last_archived, sizeof(primary_last_archived)); + SpinLockRelease(&PgArch->lock); + /* * Read "writtenUpto" without holding a spinlock. Note that it may not be * consistent with the other shared variables of the WAL receiver @@ -1797,22 +1802,26 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS) nulls[10] = true; else values[10] = TimestampTzGetDatum(latest_end_time); - if (*slotname == '\0') + if (*primary_last_archived == '\0') nulls[11] = true; else - values[11] = CStringGetTextDatum(slotname); - if (*sender_host == '\0') + values[11] = CStringGetTextDatum(primary_last_archived); + if (*slotname == '\0') nulls[12] = true; else - values[12] = CStringGetTextDatum(sender_host); - if (sender_port == 0) + values[12] = CStringGetTextDatum(slotname); + if (*sender_host == '\0') nulls[13] = true; else - values[13] = Int32GetDatum(sender_port); - if (*conninfo == '\0') + values[13] = CStringGetTextDatum(sender_host); + if (sender_port == 0) nulls[14] = true; else - values[14] = CStringGetTextDatum(conninfo); + values[14] = Int32GetDatum(sender_port); + if (*conninfo == '\0') + nulls[15] = true; + else + values[15] = CStringGetTextDatum(conninfo); } /* Returns the record as Datum */ diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f8a021987b..1974b430f8 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5718,9 +5718,9 @@ { oid => '3317', descr => 'statistics: information about WAL receiver', proname => 'pg_stat_get_wal_receiver', proisstrict => 'f', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => '', - proallargtypes => '{int4,text,pg_lsn,int4,pg_lsn,pg_lsn,int4,timestamptz,timestamptz,pg_lsn,timestamptz,text,text,int4,text}', - proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{pid,status,receive_start_lsn,receive_start_tli,written_lsn,flushed_lsn,received_tli,last_msg_send_time,last_msg_receipt_time,latest_end_lsn,latest_end_time,slot_name,sender_host,sender_port,conninfo}', + proallargtypes => '{int4,text,pg_lsn,int4,pg_lsn,pg_lsn,int4,timestamptz,timestamptz,pg_lsn,timestamptz,text,text,text,int4,text}', + proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{pid,status,receive_start_lsn,receive_start_tli,written_lsn,flushed_lsn,received_tli,last_msg_send_time,last_msg_receipt_time,latest_end_lsn,latest_end_time,primary_last_archived,slot_name,sender_host,sender_port,conninfo}', prosrc => 'pg_stat_get_wal_receiver' }, { oid => '6514', descr => 'statistics: information about WAL recovery', proname => 'pg_stat_get_recovery', proisstrict => 'f', provolatile => 's', diff --git a/src/test/recovery/t/055_archive_shared.pl b/src/test/recovery/t/055_archive_shared.pl index 5c70f5c511..24fc739e23 100644 --- a/src/test/recovery/t/055_archive_shared.pl +++ b/src/test/recovery/t/055_archive_shared.pl @@ -91,6 +91,9 @@ # Generate more WAL and wait for archiving on primary my $initial_archived = $primary->safe_psql('postgres', 'SELECT archived_count FROM pg_stat_archiver'); +my $primary_last_archived = $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); + $primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); # Wait for primary to archive the new segments @@ -115,9 +118,26 @@ last if $done_count > 0; sleep(1); } + ok($done_count > 0, "standby marked segments as .done after primary's archival report"); note("Standby has $done_count .done files"); +for (my $i = 0; $i < $PostgreSQL::Test::Utils::timeout_default; $i++) +{ + if (-f "$standby_archive_status/$primary_last_archived.done") + { + last; + } + sleep(1); +} + +# The primary_last_archived done status for WAL file itself must also be present. +ok(-f "$standby_archive_status/$primary_last_archived.done", + "WAL segment $primary_last_archived done file exists in standby pg_wal/archive_status"); + +is($primary_last_archived, $standby->safe_psql('postgres', + q{SELECT primary_last_archived from pg_stat_wal_receiver; }), "standby wal receiver stat view updated with primary last archived wal"); + ############################################################################### # Test 2: Cascading replication ############################################################################### diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 6a3341356d..eae03d9c10 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -2444,11 +2444,12 @@ pg_stat_wal_receiver| SELECT pid, last_msg_receipt_time, latest_end_lsn, latest_end_time, + primary_last_archived, slot_name, sender_host, sender_port, conninfo - FROM pg_stat_get_wal_receiver() s(pid, status, receive_start_lsn, receive_start_tli, written_lsn, flushed_lsn, received_tli, last_msg_send_time, last_msg_receipt_time, latest_end_lsn, latest_end_time, slot_name, sender_host, sender_port, conninfo) + FROM pg_stat_get_wal_receiver() s(pid, status, receive_start_lsn, receive_start_tli, written_lsn, flushed_lsn, received_tli, last_msg_send_time, last_msg_receipt_time, latest_end_lsn, latest_end_time, primary_last_archived, slot_name, sender_host, sender_port, conninfo) WHERE (pid IS NOT NULL); pg_stat_xact_all_tables| SELECT c.oid AS relid, n.nspname AS schemaname, From 492994be9b7fb226513d37983f3db970f695c884 Mon Sep 17 00:00:00 2001 From: Kirill Reshke Date: Wed, 29 Jul 2026 20:30:58 +0500 Subject: [PATCH 43/43] Guard against configuration hazards in archive_mode=shared setups. A configuration that is otherwise valid may cause problems when used with archive_mode=shared. For example, when standby has archive_mode=shared and its replication source is unable to provide archive status reports, standby will keep all WAL segments forever, leading to disk exhaustion. Fix this by refusing the archive_status_report_interval request at connection startup unless archive_mode is active on the upstream node. Two TAP tests in 057_archive_shared_hazards.pl are added: the first checks that a shared-mode standby is rejected with a FATAL error when its upstream has archive_mode=off; the second demonstrates a still-hazardous but not-rejected topology, where an intermediate archive_mode=on standby never requested reports and thus cannot relay them to a cascading shared-mode standby, which consequently never marks segments as .done. To address this, allow archiver to work on standby in archive mode shared, and archive WAL's for which we did not receive archive status report for too long (PGARCH_SHARED_FALLBACK_INTERVALS times of archive_status_report_interval). --- src/backend/postmaster/pgarch.c | 64 +++- src/backend/postmaster/postmaster.c | 25 +- .../libpqwalreceiver/libpqwalreceiver.c | 9 +- src/backend/replication/walreceiver.c | 4 + src/backend/replication/walsender.c | 8 +- src/backend/tcop/backend_startup.c | 14 +- src/include/access/xlog.h | 3 + src/include/postmaster/pgarch.h | 2 + src/include/replication/walsender.h | 2 +- src/interfaces/libpq/fe-connect.c | 8 +- src/interfaces/libpq/fe-protocol3.c | 4 +- src/interfaces/libpq/libpq-int.h | 5 +- src/test/recovery/meson.build | 1 + src/test/recovery/t/055_archive_shared.pl | 31 +- .../t/056_archive_shared_checkpoint.pl | 19 ++ .../recovery/t/057_archive_shared_hazards.pl | 316 ++++++++++++++++++ 16 files changed, 486 insertions(+), 29 deletions(-) create mode 100644 src/test/recovery/t/057_archive_shared_hazards.pl diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index ef1423133c..c7a051b4ec 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -50,6 +50,7 @@ #include "storage/shmem.h" #include "storage/subsystems.h" #include "utils/guc.h" +#include "utils/injection_point.h" #include "utils/memutils.h" #include "utils/ps_status.h" #include "utils/resowner.h" @@ -66,6 +67,15 @@ #define PGARCH_RESTART_INTERVAL 10 /* How often to attempt to restart a * failed archiver; in seconds. */ +/* + * In archive_mode=shared, a standby normally relies on the primary's archival + * status reports and does not archive segments itself. If reports stop + * arriving, the standby must fall back to archiving on its own to avoid + * accumulating WAL forever. We wait for this many report intervals to pass + * with no report before starting to archive locally. + */ +#define PGARCH_SHARED_FALLBACK_INTERVALS 3 + /* * Maximum number of retries allowed when attempting to archive a WAL * file. @@ -173,6 +183,15 @@ PgArchShmemInit(void *arg) PgArch->primary_last_archived[0] = '\0'; + /* + * Seed the report timestamp when starting up during recovery so that the + * archiver does not rush to archive segments itself right after startup. + * We give the primary a chance to send its first archival report before + * the fallback timeout (see pgarch_MainLoop) elapses. + */ + if (RecoveryInProgress()) + PgArch->last_archival_report_timestamp = GetCurrentTimestamp(); + SpinLockInit(&PgArch->lock); } @@ -307,6 +326,8 @@ static void pgarch_MainLoop(void) { bool time_to_stop; + bool do_copy_loop; + TimestampTz last_archival_report_timestamp; /* * There shouldn't be anything for the archiver to do except to wait for a @@ -317,6 +338,8 @@ pgarch_MainLoop(void) { ResetLatch(MyLatch); + INJECTION_POINT("pgarch-main-loop", NULL); + /* When we get SIGUSR2, we do one more archive cycle, then exit */ time_to_stop = ready_to_stop; @@ -342,8 +365,45 @@ pgarch_MainLoop(void) break; } - /* Do what we're here for */ - pgarch_ArchiverCopyLoop(); + if (RecoveryInProgress() && XLogArchivingShared()) + { + int64 fallback_ms; + + SpinLockAcquire(&PgArch->lock); + last_archival_report_timestamp = PgArch->last_archival_report_timestamp; + SpinLockRelease(&PgArch->lock); + + /* + * Compute the fallback timeout in 64-bit arithmetic: + * XLogArchiveStatusReportInterval can be as large as INT_MAX / 2, + * so multiplying it as an int would overflow. + */ + fallback_ms = (int64) XLogArchiveStatusReportInterval * + PGARCH_SHARED_FALLBACK_INTERVALS; + + /* + * Only archive locally once we have gone long enough without a + * report from the primary. Note that an incoming report does not + * set our latch, so we may notice the timeout up to + * PGARCH_AUTOWAKE_INTERVAL late; that coarse granularity is fine + * for a fallback path. + */ + do_copy_loop = GetCurrentTimestamp() > + TimestampTzPlusMilliseconds(last_archival_report_timestamp, + fallback_ms); + } + else + { + do_copy_loop = true; + } + + if (do_copy_loop) + { + /* Do what we're here for */ + pgarch_ArchiverCopyLoop(); + + INJECTION_POINT("pgarch-main-loop-after-copy", NULL); + } /* * Sleep until a signal is received, or until a poll is forced by diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 90c7c4528e..4a9c063698 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -3378,14 +3378,23 @@ LaunchMissingBackgroundProcesses(void) } /* - * If WAL archiving is enabled always, we are allowed to start archiver - * even during recovery. + * Decide whether the archiver may run in the current postmaster state. + * It always runs while the server is running normally. With + * archive_mode=always or =shared it also runs during recovery, since in + * those modes the standby has archiving work of its own to do. */ - if (PgArchPMChild == NULL && - ((XLogArchivingActive() && pmState == PM_RUN) || - (XLogArchivingAlways() && (pmState == PM_RECOVERY || pmState == PM_HOT_STANDBY))) && - PgArchCanRestart()) - PgArchPMChild = StartChildProcess(B_ARCHIVER); + if (PgArchPMChild == NULL && PgArchCanRestart()) + { + bool start_archiver = false; + + if (pmState == PM_RUN) + start_archiver = XLogArchivingActive(); + else if (pmState == PM_RECOVERY || pmState == PM_HOT_STANDBY) + start_archiver = XLogArchivingAlways() || XLogArchivingShared(); + + if (start_archiver) + PgArchPMChild = StartChildProcess(B_ARCHIVER); + } /* * If we need to start a slot sync worker, try to do that now @@ -3753,7 +3762,7 @@ process_pm_pmsignal(void) * files. */ Assert(PgArchPMChild == NULL); - if (XLogArchivingAlways()) + if (XLogArchivingAlways() || XLogArchivingShared()) PgArchPMChild = StartChildProcess(B_ARCHIVER); /* diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c index e438cba137..f980a7f54b 100644 --- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c +++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c @@ -22,6 +22,7 @@ #include #include +#include "access/xlog.h" #include "common/connect.h" #include "funcapi.h" #include "libpq-fe.h" @@ -154,6 +155,7 @@ libpqrcv_connect(const char *conninfo, bool replication, bool logical, const char *vals[6]; int i = 0; char *options_val = NULL; + char *archive_interval_val = NULL; /* * Re-validate connection string. The validation already happened at DDL @@ -219,8 +221,9 @@ libpqrcv_connect(const char *conninfo, bool replication, bool logical, if (expect_archive_reports) { - keys[++i] = "archive_status_reports"; - vals[i] = "true"; + archive_interval_val = psprintf("%d", XLogArchiveStatusReportInterval); + keys[++i] = "archive_status_report_interval"; + vals[i] = archive_interval_val; } keys[++i] = NULL; @@ -239,6 +242,8 @@ libpqrcv_connect(const char *conninfo, bool replication, bool logical, if (options_val != NULL) pfree(options_val); + if (archive_interval_val != NULL) + pfree(archive_interval_val); if (PQstatus(conn->streamConn) != CONNECTION_OK) goto bad_connection_errmsg; diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 145e60d5fc..ea69e8c498 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -855,6 +855,7 @@ static void XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len, TimeLineID tli) { int hdrlen; + TimestampTz now; XLogRecPtr dataStart; XLogRecPtr walEnd; TimestampTz sendTime; @@ -930,8 +931,11 @@ XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len, TimeLineID tli) errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg_internal("unexpected character in primary's last archived filename")); + now = GetCurrentTimestamp(); + SpinLockAcquire(&PgArch->lock); memcpy(PgArch->primary_last_archived, primary_last_archived, sizeof(PgArch->primary_last_archived)); + PgArch->last_archival_report_timestamp = now; SpinLockRelease(&PgArch->lock); ProcessArchivalReport(primary_last_archived); diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index bfe43ef621..3881b54d97 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -135,7 +135,9 @@ WalSnd *MyWalSnd = NULL; bool am_walsender = false; /* Am I a walsender process? */ bool am_cascading_walsender = false; /* Am I cascading WAL to another * standby? */ -bool am_archive_status_walsender = false; /* Am I replying with archive status reports? */ +int archive_status_report_interval = 0; /* interval (ms) between + * archive status reports; 0 + * disables them */ bool am_db_walsender = false; /* Connected to a database? */ /* GUC variables */ @@ -2870,7 +2872,7 @@ WalSndArchivalReport(void) char last_archived[MAX_XFN_CHARS + 1]; /* Only send reports when requested */ - if (!am_archive_status_walsender) + if (archive_status_report_interval <= 0) return; if (MyWalSnd->state != WALSNDSTATE_CATCHUP && @@ -2892,7 +2894,7 @@ WalSndArchivalReport(void) * This avoids excessive pgstat access. */ if (now < TimestampTzPlusMilliseconds(last_archival_report_timestamp, - XLogArchiveStatusReportInterval)) + archive_status_report_interval)) return; last_archival_report_timestamp = now; diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c index 6df50d3c98..23114f0f84 100644 --- a/src/backend/tcop/backend_startup.c +++ b/src/backend/tcop/backend_startup.c @@ -35,6 +35,7 @@ #include "tcop/backend_startup.h" #include "tcop/tcopprot.h" #include "utils/builtins.h" +#include "utils/guc.h" #include "utils/guc_hooks.h" #include "utils/injection_point.h" #include "utils/memutils.h" @@ -803,15 +804,20 @@ ProcessStartupPacket(Port *port) valptr), errhint("Valid values are: \"false\", 0, \"true\", 1, \"database\"."))); } - else if (strcmp(nameptr, "archive_status_reports") == 0) + else if (strcmp(nameptr, "archive_status_report_interval") == 0) { - if (!parse_bool(valptr, &am_archive_status_walsender)) + if (!parse_int(valptr, &archive_status_report_interval, 0, NULL)) ereport(FATAL, errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid value for parameter \"%s\": \"%s\"", - "archive_status_reports", + "archive_status_report_interval", valptr), - errhint("Valid values are: \"false\", 0, \"true\", 1.")); + errhint("Value must be a non-negative integer.")); + if (archive_status_report_interval > 0 && !XLogArchivingActive()) + ereport(FATAL, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("archive status report requested, but archiving is not enabled"), + errhint("Change parameter %s.", "archive_mode")); } else if (strncmp(nameptr, "_pq_.", 5) == 0) { diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 2aeb8ce14b..e71606e60f 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -106,6 +106,9 @@ extern PGDLLIMPORT bool XLogLogicalInfo; /* Is WAL archiving enabled always (even during recovery)? */ #define XLogArchivingAlways() \ (AssertMacro(XLogArchiveMode == ARCHIVE_MODE_OFF || wal_level >= WAL_LEVEL_REPLICA), XLogArchiveMode == ARCHIVE_MODE_ALWAYS) +/* Is WAL archiving coordinated between primary and standby? */ +#define XLogArchivingShared() \ + (AssertMacro(XLogArchiveMode == ARCHIVE_MODE_OFF || wal_level >= WAL_LEVEL_REPLICA), XLogArchiveMode == ARCHIVE_MODE_SHARED) /* * Is WAL-logging necessary for archival or log-shipping, or can we skip diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h index 1d8571fc5a..070de3c74b 100644 --- a/src/include/postmaster/pgarch.h +++ b/src/include/postmaster/pgarch.h @@ -45,6 +45,8 @@ typedef struct PgArchData /* Last archived WAL segment file reported by the primary */ char primary_last_archived[MAX_XFN_CHARS + 1]; + TimestampTz last_archival_report_timestamp; + /* * Forces a directory scan in pgarch_readyXlog(). */ diff --git a/src/include/replication/walsender.h b/src/include/replication/walsender.h index f2fbada75d..c0c583511a 100644 --- a/src/include/replication/walsender.h +++ b/src/include/replication/walsender.h @@ -27,7 +27,7 @@ typedef enum /* global state */ extern PGDLLIMPORT bool am_walsender; extern PGDLLIMPORT bool am_cascading_walsender; -extern PGDLLIMPORT bool am_archive_status_walsender; +extern PGDLLIMPORT int archive_status_report_interval; extern PGDLLIMPORT bool am_db_walsender; extern PGDLLIMPORT bool wake_wal_senders; diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 9b1929fc39..8dfcea76f8 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -380,9 +380,9 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Replication", "D", 5, offsetof(struct pg_conn, replication)}, - {"archive_status_reports", NULL, NULL, NULL, - "Replication", "D", 5, - offsetof(struct pg_conn, archive_status_reports)}, + {"archive_status_report_interval", NULL, NULL, NULL, + "Replication", "D", 10, + offsetof(struct pg_conn, archive_status_report_interval)}, {"target_session_attrs", "PGTARGETSESSIONATTRS", DefaultTargetSessionAttrs, NULL, @@ -5122,7 +5122,7 @@ freePGconn(PGconn *conn) free(conn->fbappname); free(conn->dbName); free(conn->replication); - free(conn->archive_status_reports); + free(conn->archive_status_report_interval); free(conn->pgservice); free(conn->pgservicefile); free(conn->pguser); diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index ed7a3f6b9d..ae0011b9f3 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -2513,8 +2513,8 @@ build_startup_packet(const PGconn *conn, char *packet, ADD_STARTUP_OPTION("database", conn->dbName); if (conn->replication && conn->replication[0]) ADD_STARTUP_OPTION("replication", conn->replication); - if (conn->archive_status_reports && conn->archive_status_reports[0]) - ADD_STARTUP_OPTION("archive_status_reports", conn->archive_status_reports); + if (conn->archive_status_report_interval && conn->archive_status_report_interval[0]) + ADD_STARTUP_OPTION("archive_status_report_interval", conn->archive_status_report_interval); if (conn->pgoptions && conn->pgoptions[0]) ADD_STARTUP_OPTION("options", conn->pgoptions); if (conn->send_appname) diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 8115db5e33..7219deacdb 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -391,7 +391,10 @@ struct pg_conn char *fbappname; /* fallback application name */ char *dbName; /* database name */ char *replication; /* connect as the replication standby? */ - char *archive_status_reports; /* do we send primary last archived WAL to the standby? */ + char *archive_status_report_interval; /* interval (ms) between + * archive status reports the + * primary sends to the + * standby; 0 disables them */ char *pgservice; /* Postgres service, if any */ char *pgservicefile; /* path to a service file containing * service(s) */ diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index 7e0c6ba67c..efb0c555f1 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -65,6 +65,7 @@ tests += { 't/054_unlogged_sequence_promotion.pl', 't/055_archive_shared.pl', 't/056_archive_shared_checkpoint.pl', + 't/057_archive_shared_hazards.pl', ], }, } diff --git a/src/test/recovery/t/055_archive_shared.pl b/src/test/recovery/t/055_archive_shared.pl index 24fc739e23..f1f5ee4892 100644 --- a/src/test/recovery/t/055_archive_shared.pl +++ b/src/test/recovery/t/055_archive_shared.pl @@ -8,6 +8,11 @@ use Test::More; use File::Path qw(rmtree); +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + # Initialize primary node with archiving my $archive_dir = PostgreSQL::Test::Utils::tempdir(); my $primary = PostgreSQL::Test::Cluster->new('primary'); @@ -30,6 +35,16 @@ # Test 1: Basic testing ############################################################################### +# Check if the extension injection_points is available, as it may be +# possible that this script is run with installcheck, where the module +# would not be installed by default. +if (!$primary->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} + +$primary->safe_psql('postgres', q(CREATE EXTENSION injection_points)); + # Ensure WAL activity exists in the current segment before switching. # pg_switch_wal() is a no-op when called at the very start of a segment, # so we write a bump transaction counter @@ -63,6 +78,10 @@ "); $standby->start; +# Pause the standby's archiver so it stays idle during recovery. +$standby->safe_psql('postgres', + q{SELECT injection_points_attach('pgarch-main-loop', 'wait')}); + # Wait for standby to catch up $primary->wait_for_catchup($standby); @@ -204,8 +223,9 @@ # Test 3: Standby promotion - verify archiver activates ############################################################################### -# Before promotion, verify archiver is not running on standby (shared mode during recovery) -# In shared mode, the standby's archiver should not be archiving during recovery +# Before promotion, verify the archiver has not archived anything on the standby. +# In shared mode, the standby's archiver stays idle during recovery as long as +# the primary's archival reports arrive frequently enough. my $archived_before = $standby->safe_psql('postgres', "SELECT archived_count FROM pg_stat_archiver"); @@ -222,6 +242,13 @@ # Generate WAL on new primary (former standby) $standby->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); +# Resume the archiver on the promoted standby. +$standby->safe_psql( + 'postgres', qq[ +SELECT injection_points_detach('pgarch-main-loop'); +SELECT injection_points_wakeup('pgarch-main-loop'); +]); + # Wait for archiver to activate and archive the new WAL # Check pg_stat_archiver to verify archiving is happening $standby->poll_query_until('postgres', diff --git a/src/test/recovery/t/056_archive_shared_checkpoint.pl b/src/test/recovery/t/056_archive_shared_checkpoint.pl index 8083ee9951..52f8ec6863 100644 --- a/src/test/recovery/t/056_archive_shared_checkpoint.pl +++ b/src/test/recovery/t/056_archive_shared_checkpoint.pl @@ -17,6 +17,11 @@ use PostgreSQL::Test::Utils; use Test::More; +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + # Use 1 MB WAL segments so we can generate many segments cheaply. my $wal_segsize = 1; @@ -52,6 +57,16 @@ }); $primary->start; +# Check if the extension injection_points is available, as it may be +# possible that this script is run with installcheck, where the module +# would not be installed by default. +if (!$primary->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} + +$primary->safe_psql('postgres', q(CREATE EXTENSION injection_points)); + my $backup_name = 'standby_backup'; $primary->backup($backup_name); @@ -66,6 +81,10 @@ }); $standby->start; +# Pause the standby's archiver so received segments keep their .ready files. +$standby->safe_psql('postgres', + q{SELECT injection_points_attach('pgarch-main-loop', 'wait')}); + $primary->wait_for_catchup($standby); ############################################################################### diff --git a/src/test/recovery/t/057_archive_shared_hazards.pl b/src/test/recovery/t/057_archive_shared_hazards.pl new file mode 100644 index 0000000000..efdf504178 --- /dev/null +++ b/src/test/recovery/t/057_archive_shared_hazards.pl @@ -0,0 +1,316 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Tests for configuration hazards involving archive_mode=shared. +# +# Test 1 checks that a shared-mode standby whose upstream does not archive +# (archive_mode=off) is rejected at connection startup, since the upstream +# cannot provide the archival status reports the standby relies on. +# +# Test 2 exercises a mixed archiving topology: +# +# primary (archive_mode=on) +# | +# standby (archive_mode=on) +# | +# cascade (archive_mode=shared) +# +# The cascading standby runs in shared mode and therefore asks its upstream +# (the standby) for archival status reports. The standby itself runs in plain +# "on" mode: it does not request reports from the primary, so it has nothing +# to relay downstream. As a result the cascading standby never receives an +# archival report: its received segments stay as .ready and +# pg_stat_wal_receiver.primary_last_archived stays empty. The test then shows +# that a shared-mode standby falls back to archiving segments itself: once its +# own archiver runs, it marks the segment as .done and starts archiving. +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +# Shared archive directory used by every node that archives. +my $archive_dir = PostgreSQL::Test::Utils::tempdir(); +my $archive_command = + $PostgreSQL::Test::Utils::windows_os + ? qq{copy "%p" "$archive_dir\\%f"} + : qq{cp "%p" "$archive_dir/%f"}; + + + +############################################################################### +# Test 1: Primary archive_mode=off, standby archive_mode=shared +# +# A standby running in shared mode connects requesting archival status +# reports. If the primary does not archive at all (archive_mode=off), it +# cannot provide such reports, so the primary must reject the connection with +# a FATAL error and replication must never start. +############################################################################### + +my $noarch_primary = PostgreSQL::Test::Cluster->new('noarch_primary'); +$noarch_primary->init(allows_streaming => 1); +$noarch_primary->append_conf('postgresql.conf', " +archive_mode = off +wal_keep_size = 128MB +"); +$noarch_primary->start; + +# Sanity check: archiving is really disabled on the primary +is($noarch_primary->safe_psql('postgres', "SHOW archive_mode;"), 'off', + "primary has archive_mode=off"); + +my $noarch_backup = 'noarch_backup'; +$noarch_primary->backup($noarch_backup); + +my $shared_standby = PostgreSQL::Test::Cluster->new('shared_standby'); +$shared_standby->init_from_backup($noarch_primary, $noarch_backup, + has_streaming => 1); +$shared_standby->append_conf('postgresql.conf', " +archive_mode = shared +archive_status_report_interval = 10ms +archive_command = '$archive_command' +wal_receiver_status_interval = 1s +"); + +# Note the current log position so wait_for_log() only inspects new output. +my $logstart = -s $shared_standby->logfile; +$shared_standby->start; + +# The primary must reject the walreceiver's connection because it cannot +# provide archival status reports while archive_mode=off. +$shared_standby->wait_for_log( + qr/FATAL: archive status report requested, but archiving is not enabled/, + $logstart); + +# Generate WAL on the primary; it must not reach the standby since the +# walreceiver connection is refused. +$noarch_primary->safe_psql('postgres', + "SELECT txid_current();SELECT pg_switch_wal();"); + +# The standby's walreceiver must never establish a connection. +is($shared_standby->safe_psql('postgres', + "SELECT count(*) FROM pg_stat_wal_receiver;"), + '0', + "standby has no active wal receiver when connection is rejected"); + +# ... and the primary must see no walsender for it either. +is($noarch_primary->safe_psql('postgres', + "SELECT count(*) FROM pg_stat_replication;"), + '0', + "primary has no replication connection when standby is rejected"); + + +############################################################################### +# Test 2: Primary with archive_mode=on, standby with archive_mode=on streaming +# from the primary, and a cascading standby with archive_mode=shared streaming +# from the standby. +# +# The cascading standby running in shared mode connects requesting archival +# status reports from the standby, which has no report from the primary to +# relay. +############################################################################### + +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init(has_archiving => 1, allows_streaming => 1); +$primary->append_conf('postgresql.conf', " +archive_mode = on +archive_command = '$archive_command' +archive_status_report_interval = 10ms +wal_keep_size = 128MB +"); +$primary->start; + +# Check if the extension injection_points is available, as it may be +# possible that this script is run with installcheck, where the module +# would not be installed by default. +if (!$primary->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} + +$primary->safe_psql('postgres', q(CREATE EXTENSION injection_points)); + +is($primary->safe_psql('postgres', "SHOW archive_mode;"), 'on', + "primary has archive_mode=on"); + +# Make sure the primary actually archives something. +$primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); +$primary->poll_query_until('postgres', + "SELECT archived_count > 0 FROM pg_stat_archiver") + or die "timed out waiting for primary to archive"; + +############################################################################### +# Standby with archive_mode=on, streaming from the primary +############################################################################### + +my $standby_backup = 'standby_backup'; +$primary->backup($standby_backup); + +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, $standby_backup, has_streaming => 1); +$standby->append_conf('postgresql.conf', " +archive_mode = on +archive_command = '$archive_command' +archive_status_report_interval = 10ms +wal_receiver_status_interval = 1s +"); +$standby->start; + +$primary->wait_for_catchup($standby); + +is($standby->safe_psql('postgres', "SELECT count(*) FROM pg_stat_wal_receiver;"), + '1', "standby wal receiver is connected to the primary"); + +############################################################################### +# Cascading standby with archive_mode=shared, streaming from the standby +############################################################################### + +my $cascade_backup = 'cascade_backup'; +$standby->backup($cascade_backup); + +my $cascade = PostgreSQL::Test::Cluster->new('cascade'); +$cascade->init_from_backup($standby, $cascade_backup, has_streaming => 1); +$cascade->append_conf('postgresql.conf', " +archive_mode = shared +archive_status_report_interval = 10ms +archive_command = '$archive_command' +wal_receiver_status_interval = 1s +"); +$cascade->start; + +# Pause the cascading standby's archiver so we can first observe the state +# before it archives anything on its own, then let it run in a controlled way. +$cascade->safe_psql('postgres', + q{SELECT injection_points_attach('pgarch-main-loop', 'wait')}); +$cascade->safe_psql('postgres', + q{SELECT injection_points_attach('pgarch-main-loop-after-copy', 'wait')}); + +# The cascading standby connects successfully: its upstream (the standby) has +# archive_mode=on, so XLogArchivingActive() is true there and the connection +# is not rejected. +$standby->wait_for_catchup($cascade); +is($cascade->safe_psql('postgres', "SELECT count(*) FROM pg_stat_wal_receiver;"), + '1', "cascading standby wal receiver is connected to the standby"); + +############################################################################### +# Generate WAL, archive it on the primary, and observe the cascade +############################################################################### + +my $before = $primary->safe_psql('postgres', + "SELECT archived_count FROM pg_stat_archiver"); + +my $current_walfile = $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); + +my $walfile_ready = "$current_walfile.ready"; +my $walfile_done = "$current_walfile.done"; + +$primary->safe_psql('postgres', "SELECT txid_current();SELECT pg_switch_wal();"); +$primary->poll_query_until('postgres', + "SELECT archived_count > $before FROM pg_stat_archiver") + or die "timed out waiting for primary to archive new segment"; + +# Propagate the new WAL all the way down the chain. +$primary->wait_for_catchup($standby); +$standby->wait_for_catchup($cascade); + +my $cascade_status = $cascade->data_dir . '/pg_wal/archive_status'; + +# The cascading standby must create status files for received WAL. +my $ready_seen = 0; +for (my $i = 0; $i < $PostgreSQL::Test::Utils::timeout_default; $i++) +{ + $ready_seen = 0; + if (opendir(my $dh, $cascade_status)) + { + $ready_seen = scalar(grep { /\.ready$/ } readdir($dh)); + closedir($dh); + } + last if $ready_seen > 0; + sleep(1); +} + +# The cascading standby must have created a .ready status file for the segment. +ok( -f "$cascade_status/$walfile_ready", + ".ready file exists on cascade replica for WAL segment $current_walfile"); + +# The intermediate standby runs in "on" mode and never requested reports from +# the primary, so it cannot relay any report. With the cascading standby's +# archiver still paused, the segment must remain .ready and no report can have +# marked it .done. +ok( -f "$cascade_status/$walfile_ready", + "segment stays .ready on cascading standby without an archival report"); +ok( !-f "$cascade_status/$walfile_done", + "segment is not marked .done without an archival report"); + +# And its view column must stay empty: no report ever arrived. +is($cascade->safe_psql('postgres', + "SELECT primary_last_archived FROM pg_stat_wal_receiver;"), + '', + "cascading standby primary_last_archived stays empty"); + +# Let the cascading standby's archiver run. Since no archival report will ever +# arrive, a shared-mode standby must fall back to archiving the segment itself. +$cascade->safe_psql( + 'postgres', qq[ +SELECT injection_points_detach('pgarch-main-loop'); +SELECT injection_points_wakeup('pgarch-main-loop'); +]); + +# Helper: Wait for a session to hit an injection point. +# Optional second argument is timeout in seconds. +# Returns true if found, false if timeout. +# On timeout, logs diagnostic information about all active queries. +sub wait_for_injection_point +{ + my ($node, $point_name, $timeout) = @_; + $timeout //= $PostgreSQL::Test::Utils::timeout_default / 2; + + for (my $elapsed = 0; $elapsed < $timeout * 10; $elapsed++) + { + my $pid = $node->safe_psql( + 'postgres', qq[ + SELECT pid FROM pg_stat_activity + WHERE wait_event_type = 'InjectionPoint' + AND wait_event = '$point_name' + LIMIT 1; + ]); + return 1 if $pid ne ''; + sleep(1); + } + + # Timeout - report diagnostic information + my $activity = $node->safe_psql( + 'postgres', q[ + SELECT format('pid=%s, state=%s, wait_event_type=%s, wait_event=%s, backend_xmin=%s, backend_xid=%s, query=%s', + pid, state, wait_event_type, wait_event, backend_xmin, backend_xid, left(query, 100)) + FROM pg_stat_activity + ORDER BY pid; + ]); + diag( "wait_for_injection_point timeout waiting for: $point_name\n" + . "Current queries in pg_stat_activity:\n$activity"); + + return 0; +} + +# Wait until the archiver has run one copy cycle and parked on the injection +# point that follows it. +wait_for_injection_point($cascade, 'pgarch-main-loop-after-copy'); + +# The archiver archived the segment on its own: .ready is gone and .done exists. +ok( !-f "$cascade_status/$walfile_ready", + "segment no longer .ready after cascading standby archives it itself"); +ok( -f "$cascade_status/$walfile_done", + "segment marked .done after cascading standby archives it itself"); + +# And pg_stat_archiver must reflect the self-archived segment. +is($cascade->safe_psql('postgres', + "SELECT archived_count FROM pg_stat_archiver"), + '1', + "cascading standby starts archiving on its own"); + +done_testing();