From 68804988701962e4fe97709ba437058373d1653c Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Mon, 21 Sep 2026 17:31:14 +0800 Subject: [PATCH v4 2/2] Fix inaccurate inclusive-bound error messages. ALTER SUBSCRIPTION ... SKIP accepts a skip LSN equal to the replication origin's progress, but its error message said that the skip LSN must be greater than the origin LSN. Likewise, ParseVariableDouble() accepts values at either end of its documented [min,max] range, but its messages said that they had to be strictly inside the range. Adjust the messages to describe the inclusive bounds accurately, and add coverage for both cases. Suggested-by: Bharath Rupireddy Author: Chao Li Reviewed-by: Kiran Kaki Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com --- src/backend/commands/subscriptioncmds.c | 2 +- src/bin/psql/t/001_basic.pl | 15 +++++++++++++++ src/bin/psql/variables.c | 4 ++-- src/test/regress/expected/subscription.out | 12 ++++++++++++ src/test/regress/sql/subscription.sql | 12 ++++++++++++ 5 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 22a61dca65d..27a5b55e964 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -2419,7 +2419,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, if (XLogRecPtrIsValid(remote_lsn) && opts.lsn < remote_lsn) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("skip WAL location (LSN %X/%08X) must be greater than origin LSN %X/%08X", + errmsg("skip WAL location (LSN %X/%08X) must be greater than or equal to origin LSN %X/%08X", LSN_FORMAT_ARGS(opts.lsn), LSN_FORMAT_ARGS(remote_lsn)))); } diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl index 028df33ce8a..3ea0e226a36 100644 --- a/src/bin/psql/t/001_basic.pl +++ b/src/bin/psql/t/001_basic.pl @@ -452,6 +452,21 @@ psql_fails_like( '\set WATCH_INTERVAL 1e500', qr/is out of range/, 'WATCH_INTERVAL variable is out of range'); +psql_like( + $node, + '\set WATCH_INTERVAL 1000000' . "\n" . '\echo :WATCH_INTERVAL', + qr/^1000000$/m, + 'WATCH_INTERVAL accepts its upper bound'); +psql_fails_like( + $node, + '\set WATCH_INTERVAL -1', + qr/must be greater than or equal to 0\.00/, + 'WATCH_INTERVAL variable is below its lower bound'); +psql_fails_like( + $node, + '\set WATCH_INTERVAL 1000001', + qr/must be less than or equal to 1000000\.00/, + 'WATCH_INTERVAL variable is above its upper bound'); psql_like($node, '\echo :WATCH_INTERVAL', qr/^2$/m, 'WATCH_INTERVAL variable was not altered'); diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c index 8060f2959cc..2c9a191a69b 100644 --- a/src/bin/psql/variables.c +++ b/src/bin/psql/variables.c @@ -215,14 +215,14 @@ ParseVariableDouble(const char *value, const char *name, double *result, double if (dblval < min) { if (name) - pg_log_error("invalid value \"%s\" for variable \"%s\": must be greater than %.2f", + pg_log_error("invalid value \"%s\" for variable \"%s\": must be greater than or equal to %.2f", value, name, min); return false; } else if (dblval > max) { if (name) - pg_log_error("invalid value \"%s\" for variable \"%s\": must be less than %.2f", + pg_log_error("invalid value \"%s\" for variable \"%s\": must be less than or equal to %.2f", value, name, max); return false; } diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 681dc66dca6..d2ca288f9e2 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -294,6 +294,18 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345'); -- ok - with lsn = NONE ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE); +-- fail - LSN must not be behind the replication origin +RESET SESSION AUTHORIZATION; +SELECT pg_replication_origin_advance( + 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'), + '0/12346') \gset +-- ok - LSN equal to the replication origin is accepted +ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12346'); +ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE); +-- fail - LSN behind the replication origin is rejected +ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345'); +ERROR: skip WAL location (LSN 0/00012345) must be greater than or equal to origin LSN 0/00012346 +SET SESSION AUTHORIZATION regress_subscription_user; -- fail ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0'); ERROR: invalid WAL location (LSN): 0/0 diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index cfeebaf9302..0ac491a70b6 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -231,6 +231,18 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345'); -- ok - with lsn = NONE ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE); +-- fail - LSN must not be behind the replication origin +RESET SESSION AUTHORIZATION; +SELECT pg_replication_origin_advance( + 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'), + '0/12346') \gset +-- ok - LSN equal to the replication origin is accepted +ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12346'); +ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE); +-- fail - LSN behind the replication origin is rejected +ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345'); +SET SESSION AUTHORIZATION regress_subscription_user; + -- fail ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0'); -- 2.50.1 (Apple Git-155)