Re: Allow default \watch interval in psql to be configured

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: Greg Sabino Mullane <htamfids(at)gmail(dot)com>, Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Allow default \watch interval in psql to be configured
Date: 2025-03-20 20:45:03
Message-ID: 64BFB32E-C7A2-42EC-970C-C6D85E395E6A@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 17 Mar 2025, at 13:37, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:

> 0 is an accepted value for interval, even though it might look insensible.
>
> The behaviour should be same in both cases \watch i=<some value> and
> \set WATCH_INTERVAL <some value> \watch. In this case it's not.

Having a watch interval of zero is IMHO somewhat nonsensical, but since it was
done intentionally in 6f9ee74d45 (which I had missed) I agree that the default
should support it as well. Fixed.

> In fact, we should use the same validation code in both the cases. Why
> don't we perform the same additional validations in
> ParseVariableDouble() in exec_watch_command() as well?

Well, they don't use the same code since they are two different things
(variables and command input, while using the same syscalls they have different
errorhandling requirements). If executing \watch would use ParseVariableDouble
it would make errorhandling quite awkward at best. I added a comment in
exec_command_watch that any changes in internval parsing should take default
intervals into consideration.

> The test only validate default variable value. We need a test where we
> see variable value being honored lie tests between 369 to 376 in the
> same file.

I'm not sure it's worth spending test cycles on as this code doesn't affect the
execution of \watch at all. That being said, I added a testcase which sets a
default and then executes \watch. It doesn't test that the interval was
correct (we don't do that for any), but at least it will catch if setting a
default totally breaks \watch.

--
Daniel Gustafsson

Attachment Content-Type Size
v7-0001-psql-Make-default-watch-interval-configurable.patch application/octet-stream 10.1 KB
unknown_filename text/plain 1 byte

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2025-03-20 20:50:57 Re: [PoC] Federated Authn/z with OAUTHBEARER
Previous Message Jacob Champion 2025-03-20 20:33:26 Re: [PoC] Federated Authn/z with OAUTHBEARER