From a7bdcc5589b471ad7b43e43d25aa6ed8e802767e Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Tue, 9 Jun 2026 15:57:56 +0800 Subject: [PATCH v1] Reject negative max_retention_duration values The subscription option max_retention_duration accepts an integer value, where zero disables the retention timeout behavior. Negative values do not have a useful meaning, but were accepted and stored in the subscription catalog. Reject negative values when parsing subscription options, covering both CREATE SUBSCRIPTION and ALTER SUBSCRIPTION. Also treat non-positive values as disabled in the apply worker as a defensive check. Add regression tests for negative max_retention_duration values in both CREATE SUBSCRIPTION and ALTER SUBSCRIPTION. Author: Chao Li --- src/backend/commands/subscriptioncmds.c | 5 +++++ src/backend/replication/logical/worker.c | 2 +- src/test/regress/expected/subscription.out | 6 ++++++ src/test/regress/sql/subscription.sql | 6 ++++++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index fd026b304c2..87311f683e9 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -356,6 +356,11 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, opts->specified_opts |= SUBOPT_MAX_RETENTION_DURATION; opts->maxretention = defGetInt32(defel); + + if (opts->maxretention < 0) + ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("max_retention_duration cannot be negative")); } else if (IsSet(supported_opts, SUBOPT_ORIGIN) && strcmp(defel->defname, "origin") == 0) diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index a3f2406ed83..b5dc7ca02c2 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -4796,7 +4796,7 @@ should_stop_conflict_info_retention(RetainDeadTuplesData *rdt_data) Assert(rdt_data->phase == RDT_WAIT_FOR_PUBLISHER_STATUS || rdt_data->phase == RDT_WAIT_FOR_LOCAL_FLUSH); - if (!MySubscription->maxretention) + if (MySubscription->maxretention <= 0) return false; /* diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 8481056a702..a1b3cc96d83 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -518,6 +518,9 @@ DROP SUBSCRIPTION regress_testsub; -- fail - max_retention_duration must be integer CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, max_retention_duration = foo); ERROR: max_retention_duration requires an integer value +-- fail - max_retention_duration must be non-negative +CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, max_retention_duration = -1); +ERROR: max_retention_duration cannot be negative -- ok CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, max_retention_duration = 1000); NOTICE: max_retention_duration is ineffective when retain_dead_tuples is disabled @@ -530,6 +533,9 @@ HINT: To initiate replication, you must manually create the replication slot, e regress_testsub | regress_subscription_user | f | {testpub} | f | parallel | d | f | any | t | f | f | | f | 1000 | f | off | dbname=regress_doesnotexist | -1 | 0/00000000 | (1 row) +-- fail - max_retention_duration must be non-negative +ALTER SUBSCRIPTION regress_testsub SET (max_retention_duration = -1); +ERROR: max_retention_duration cannot be negative -- ok ALTER SUBSCRIPTION regress_testsub SET (max_retention_duration = 0); \dRs+ diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 374fad6aa7b..528a10b5481 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -378,11 +378,17 @@ DROP SUBSCRIPTION regress_testsub; -- fail - max_retention_duration must be integer CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, max_retention_duration = foo); +-- fail - max_retention_duration must be non-negative +CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, max_retention_duration = -1); + -- ok CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, max_retention_duration = 1000); \dRs+ +-- fail - max_retention_duration must be non-negative +ALTER SUBSCRIPTION regress_testsub SET (max_retention_duration = -1); + -- ok ALTER SUBSCRIPTION regress_testsub SET (max_retention_duration = 0); -- 2.50.1 (Apple Git-155)