WAIT for LSN does not reject small negative timeouts
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253934psql -h localhost -U postgresBuilt from patchset v1 (message #1), October 22, 2026 at 03:49 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253934_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253934_1 && git checkout t253934_1Patchset v1 (message #1) is on t253934_1
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'.
SImple fix attached
--
Best regards,
Kirill Reshke
On 25 Sep 2026, at 07:53, Kirill Reshke <reshkekirill@gmail.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 requestI 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
On Fri, Sep 25, 2026 at 12:57 AM Daniel Gustafsson <daniel@yesql.se> wrote:
On 25 Sep 2026, at 07:53, Kirill Reshke <reshkekirill@gmail.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 requestI 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.
Yes. Since it uses parse_int(), some GUC parameters that accept an
integer value with a unit behaves the same way:
=# set statement_timeout to '-1ms';
ERROR: -1 ms is outside the valid range for parameter
"statement_timeout" (0 ms .. 2147483647 ms)
=# set statement_timeout to '-0.4ms';
SET
=# show statement_timeout ;
statement_timeout
-------------------
0
(1 row)
We agree it might be better to reject such negative values too, but we
agreed it's reasonable to keep the same behavior as other integer GUC
parameters, rather than adding code just to handle the WAIT command
case[1]/messages/by-id/CAD21AoDpeNo7ZLNcqp3u51FBoXbMft8gvaEn-FXcoGpEZz8V=g@mail.gmail.com.
Regards,
[1]: /messages/by-id/CAD21AoDpeNo7ZLNcqp3u51FBoXbMft8gvaEn-FXcoGpEZz8V=g@mail.gmail.com
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com