[PATCH] psql: Make ParseVariableDouble reject values above max
Hello,
ParseVariableDouble() in src/bin/psql/variables.c is asymmetric in how
it handles the [min, max] bounds it documents. The lower-bound branch
correctly returns false, but the upper-bound branch logs the error and
then falls through to assign *result and return true. The function's
contract ("the value must be within the range [min,max] in order to be
considered valid"; "if unsuccessful, *result isn't clobbered") is
broken on the upper-bound path.
The only caller today is watch_interval_hook, so the user-visible
effect is that an out-of-range WATCH_INTERVAL is reported as invalid
yet still assigned.
Reproducer:
$ psql
# \set WATCH_INTERVAL 99999999
invalid value "99999999" for variable "WATCH_INTERVAL": must be less than
1000000.00
# \echo :WATCH_INTERVAL
99999999
The error is printed, but the variable is set anyway.
Regards,
Sven Klemm
Attachments:
0001-psql-Make-ParseVariableDouble-reject-values-above-ma.patchtext/x-patch; charset=US-ASCII; name=0001-psql-Make-ParseVariableDouble-reject-values-above-ma.patchDownload+1-1
On 8 May 2026, at 17:39, Sven Klemm <sven@tigerdata.com> wrote:
Hello,
ParseVariableDouble() in src/bin/psql/variables.c is asymmetric in how
it handles the [min, max] bounds it documents. The lower-bound branch
correctly returns false, but the upper-bound branch logs the error and
then falls through to assign *result and return true. The function's
contract ("the value must be within the range [min,max] in order to be
considered valid"; "if unsuccessful, *result isn't clobbered") is
broken on the upper-bound path.
Indeed, that's a silly bug, not sure how I could have missed that. We are
currently in freeze for the upcoming minor releases but I have this staged to
go in directly after. Thanks for the report.
--
Daniel Gustafsson
On 9 May 2026, at 11:44, Daniel Gustafsson <daniel@yesql.se> wrote:
On 8 May 2026, at 17:39, Sven Klemm <sven@tigerdata.com> wrote:
Hello,
ParseVariableDouble() in src/bin/psql/variables.c is asymmetric in how
it handles the [min, max] bounds it documents. The lower-bound branch
correctly returns false, but the upper-bound branch logs the error and
then falls through to assign *result and return true. The function's
contract ("the value must be within the range [min,max] in order to be
considered valid"; "if unsuccessful, *result isn't clobbered") is
broken on the upper-bound path.Indeed, that's a silly bug, not sure how I could have missed that. We are
currently in freeze for the upcoming minor releases but I have this staged to
go in directly after. Thanks for the report.
Pushed and backpatched to v18, with the addition of a test for this behaviour.
--
Daniel Gustafsson