From a2912be11f0f4e9ec70e3feae9083b809a843f77 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Mon, 21 Sep 2026 17:31:14 +0800 Subject: [PATCH v5 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 Reviewed-by: Surya Poondla 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 | 16 ++++++++++++++++ src/bin/psql/variables.c | 4 ++-- src/test/regress/expected/subscription.out | 14 ++++++++++++++ src/test/regress/sql/subscription.sql | 9 +++++++++ 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..a36f6ba5b8e 100644 --- a/src/bin/psql/t/001_basic.pl +++ b/src/bin/psql/t/001_basic.pl @@ -452,6 +452,22 @@ 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 +\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..f2143bcd029 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -294,6 +294,20 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345'); -- ok - with lsn = NONE ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE); +SELECT pg_replication_origin_advance( + 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'), + '0/12346'); + pg_replication_origin_advance +------------------------------- + +(1 row) + +-- 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 -- 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..0150f9bd044 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -231,6 +231,15 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345'); -- ok - with lsn = NONE ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE); +SELECT pg_replication_origin_advance( + 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'), + '0/12346'); +-- 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'); + -- fail ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0'); -- 2.50.1 (Apple Git-155)