From 595062c914badc972b3dd9d0004208fbbd91c3cc Mon Sep 17 00:00:00 2001 From: Haibo Yan Date: Thu, 10 Sep 2026 22:30:47 -0700 Subject: [PATCH v1 2/2] Reject nonzero command option values that round to sentinel zero WAIT FOR treats a zero timeout as "wait indefinitely", and VACUUM treats BUFFER_USAGE_LIMIT 0 as "no ring buffer size limit". Parsing rounds, so a small nonzero value reaches these options as zero and silently selects the sentinel behavior, the opposite of what was asked for: WAIT FOR LSN '...' WITH (timeout '0.0001s'); -- waits indefinitely VACUUM (BUFFER_USAGE_LIMIT '512B') t; -- runs with no limit For VACUUM the validation was effectively inverted, since '513B' was rejected for being below the 128kB minimum while the smaller '512B' was accepted. WAIT FOR also tested the sign of the timeout after rounding, so '-0.4ms' passed the "cannot be negative" check and waited indefinitely as well. Use the input sign reported by parse_int_with_sign() and parse_real_with_sign() to tell a value written as zero from a nonzero value that rounded to zero, and reject the latter. A value written as zero keeps its meaning. WAIT FOR errors out rather than clamping up to 1ms: turning a finite timeout into an indefinite wait is not acceptable, but silently substituting a timeout that was not asked for is a choice too, so leave it to the user. WAIT FOR has not appeared in a released version, so there is no compatibility concern. --- src/backend/commands/vacuum.c | 9 +++++-- src/backend/commands/wait.c | 19 +++++++++++--- src/test/recovery/t/049_wait_for_lsn.pl | 35 +++++++++++++++++++++++++ src/test/regress/expected/vacuum.out | 11 ++++++++ src/test/regress/sql/vacuum.sql | 7 +++++ 5 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index d8c2f33..8ac2728 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -211,14 +211,19 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) const char *hintmsg; int result; char *vac_buffer_size; + int input_sign; vac_buffer_size = defGetString(opt); /* * Check that the specified value is valid and the size falls - * within the hard upper and lower limits if it is not 0. + * within the hard upper and lower limits if it is not 0. Only a + * value written as zero selects the unlimited behavior, so a + * nonzero size that rounded to zero is rejected too. */ - if (!parse_int(vac_buffer_size, &result, GUC_UNIT_KB, &hintmsg) || + if (!parse_int_with_sign(vac_buffer_size, &result, GUC_UNIT_KB, + &hintmsg, &input_sign) || + (result == 0 && input_sign != 0) || (result != 0 && (result < MIN_BAS_VAC_RING_SIZE_KB || result > MAX_BAS_VAC_RING_SIZE_KB))) { diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c index 9ba4c75..b8fb168 100644 --- a/src/backend/commands/wait.c +++ b/src/backend/commands/wait.c @@ -93,6 +93,7 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, char *timeout_str; const char *hintmsg; double dval; + int input_sign; if (timeout_specified) errorConflictingDefElem(defel, pstate); @@ -100,7 +101,8 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, timeout_str = defGetString(defel); - if (!parse_real(timeout_str, &dval, GUC_UNIT_MS, &hintmsg)) + if (!parse_real_with_sign(timeout_str, &dval, GUC_UNIT_MS, + &hintmsg, &input_sign)) { ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -108,6 +110,12 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, hintmsg ? errhint("%s", _(hintmsg)) : 0); } + /* Test the sign as written, since rounding can hide it */ + if (input_sign < 0) + ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("timeout cannot be negative")); + /* * Get rid of any fractional part in the input. This is so we * don't fail on just-out-of-range values that would round into @@ -121,10 +129,15 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("timeout value is out of range")); - if (dval < 0) + /* + * Zero means to wait indefinitely, so a nonzero timeout that + * rounded to zero would mean the opposite of what was asked for. + */ + if (input_sign > 0 && dval == 0) ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("timeout cannot be negative")); + errmsg("timeout value \"%s\" is less than 1ms", + timeout_str)); timeout = (int64) dval; } diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl index cb7d4d4..c4324b3 100644 --- a/src/test/recovery/t/049_wait_for_lsn.pl +++ b/src/test/recovery/t/049_wait_for_lsn.pl @@ -343,6 +343,30 @@ $node_standby->psql( stderr => \$stderr); ok($stderr =~ /timeout cannot be negative/, "get error for negative timeout"); +# A negative value small enough to round to zero must be rejected too, rather +# than turning into an indefinite wait. +$node_standby->psql( + 'postgres', + "WAIT FOR LSN '${test_lsn}' WITH (timeout '-0.4ms');", + stderr => \$stderr); +ok($stderr =~ /timeout cannot be negative/, + "get error for negative timeout that rounds to zero"); + +# Likewise a positive value below the 1ms resolution. +$node_standby->psql( + 'postgres', + "WAIT FOR LSN '${test_lsn}' WITH (timeout '0.0001s');", + stderr => \$stderr); +ok($stderr =~ /timeout value "0.0001s" is less than 1ms/, + "get error for sub-millisecond timeout given in seconds"); + +$node_standby->psql( + 'postgres', + "WAIT FOR LSN '${test_lsn}' WITH (timeout '0.4ms');", + stderr => \$stderr); +ok($stderr =~ /timeout value "0.4ms" is less than 1ms/, + "get error for sub-millisecond timeout"); + # Test unknown parameter with WITH clause $node_standby->psql( 'postgres', @@ -407,6 +431,17 @@ $output = $node_standby->safe_psql( ok($output eq "timeout", "WAIT FOR WITH clause returns correct timeout status"); +# Values just above the 1ms resolution round up to a real, bounded timeout +# rather than to zero, which would mean waiting indefinitely. +$output = $node_standby->safe_psql( + 'postgres', qq[ + WAIT FOR LSN '${lsn3}' WITH (timeout '1ms', no_throw);]); +ok($output eq "timeout", "WAIT FOR honors a 1ms timeout"); +$output = $node_standby->safe_psql( + 'postgres', qq[ + WAIT FOR LSN '${lsn3}' WITH (timeout '0.6ms', no_throw);]); +ok($output eq "timeout", "WAIT FOR rounds 0.6ms up to a 1ms timeout"); + # Test WITH clause error case - invalid option $node_standby->psql( 'postgres', diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index d4696bc..b8745f2 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -542,6 +542,17 @@ ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB VACUUM (BUFFER_USAGE_LIMIT 10000000000) vac_option_tab; ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB HINT: Value exceeds integer range. +-- only a value written as zero means "no ring buffer limit"; a nonzero size +-- that rounds to zero, or a negative one, must be rejected +VACUUM (BUFFER_USAGE_LIMIT '512B') vac_option_tab; +ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB +VACUUM (BUFFER_USAGE_LIMIT '-512B') vac_option_tab; +ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB +VACUUM (BUFFER_USAGE_LIMIT '513B') vac_option_tab; +ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB +VACUUM (BUFFER_USAGE_LIMIT '127kB') vac_option_tab; +ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB +VACUUM (BUFFER_USAGE_LIMIT '128kB') vac_option_tab; -- incompatible with VACUUM FULL error VACUUM (BUFFER_USAGE_LIMIT '512 kB', FULL) vac_option_tab; ERROR: BUFFER_USAGE_LIMIT cannot be specified for VACUUM FULL diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index 247b8e2..8a385cf 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -402,6 +402,13 @@ VACUUM (BUFFER_USAGE_LIMIT 16777220) vac_option_tab; VACUUM (BUFFER_USAGE_LIMIT 120) vac_option_tab; -- integer overflow error VACUUM (BUFFER_USAGE_LIMIT 10000000000) vac_option_tab; +-- only a value written as zero means "no ring buffer limit"; a nonzero size +-- that rounds to zero, or a negative one, must be rejected +VACUUM (BUFFER_USAGE_LIMIT '512B') vac_option_tab; +VACUUM (BUFFER_USAGE_LIMIT '-512B') vac_option_tab; +VACUUM (BUFFER_USAGE_LIMIT '513B') vac_option_tab; +VACUUM (BUFFER_USAGE_LIMIT '127kB') vac_option_tab; +VACUUM (BUFFER_USAGE_LIMIT '128kB') vac_option_tab; -- incompatible with VACUUM FULL error VACUUM (BUFFER_USAGE_LIMIT '512 kB', FULL) vac_option_tab; -- 2.54.0