Re: WAIT for LSN does not reject small negative timeouts

> On 25 Sep 2026, at 07:53, Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
>
> repro:
>
> reshke=# wait for lsn '1/01C96E58' with (timeout '-1ms');
> ERROR: timeout cannot be negative
> reshke=# wait for lsn '1/01C96E58' with (timeout '-0.4ms');
> ^CCancel request sent
> ERROR: canceling statement due to user request
>
> I think we need to reject '-0.4ms' in the same way as '-1ms'.

This can also be seen as working as documented based on the following paragraph
(which might then need a s/down/ applied):

"Fractional values are rounded to the nearest millisecond. Note that a
<parameter>timeout</parameter> of half a millisecond or less therefore
rounds down to zero, which means waiting indefinitely."

The passed value is fractional and is rounded to zero with an indefinite wait.
I'm not convinced we need to spend code on handling a value which works as
documented, and doesn't make sense as a timeout in the first place.

That being said, I did have a look at the patch:

+ if (*cp == '-')
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("timeout cannot be negative"),
+ parser_errposition(pstate, defel->location));

Duplicating the ereport seems a bit wasteful, this would be better by setting a
flag value or even setting timeout to -1 and allow the next conditional to
catch it.

+# Test negative sub-millisecond timeout: rint() would round these to zero
+# before the < 0 check, silently turning them into an infinite wait
+for my $neg_timeout (qw(-0.4ms -0.49ms -0.4999999ms -0.000001s -0))
+{
+ $node_standby->psql(
+ 'postgres',
+ "WAIT FOR LSN '${test_lsn}' WITH (timeout '${neg_timeout}');",
+ stderr => \$stderr);
+ ok($stderr =~ /timeout cannot be negative/,
+ "get error for negative sub-millisecond timeout '${neg_timeout}'");
+
+ $node_standby->psql(
+ 'postgres',
+ "WAIT FOR LSN '${test_lsn}' WITH (timeout ' ${neg_timeout} ');",
+ stderr => \$stderr);
+ ok($stderr =~ /timeout cannot be negative/,
+ "get error for negative timeout '${neg_timeout}' with leading space");
+}

This seems a bit expensive for testing a cornercase which is unlikely to ever
happen, if we want test coverage for parse_int rounding it should be done
specifically for that function. A single test for ' -0.4' seems sufficient
here.

--
Daniel Gustafsson