Re: ERROR messages in VACUUM's PARALLEL option

(On Wed, 12 Apr 2023 at 01:58, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > Over in [1], Horiguchisan mentioned a few things about VACUUM's new
> > BUFFER_USAGE_LIMIT option.
>
> > 1) buffer_usage_limit in the ERROR messages should be consistently in uppercase.
>
> FWIW, I think this is exactly backward, and so is whatever code you
> based this on. Our usual habit is to write GUC names and suchlike
> in lower case in error messages. Add double quotes if you feel you
> want to set them off from the surrounding text. Here's a typical
> example of longstanding style:

Thanks for chipping in on this. Can you confirm if you think this
should apply to VACUUM options? We're not talking GUCs here.

I think there might be some precedents creeping in here for the legacy
VACUUM syntax. Roughly the current ERROR messages are using upper
case. e.g:

"PROCESS_TOAST required with VACUUM FULL"
"VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL"
"ANALYZE option must be specified when a column list is provided"

It's quite possible the newer options copied what VACUUM FULL did, and
VACUUM FULL was probably upper case from before we had the newer
syntax with the options in parenthesis.

I did notice that defGetString() did lower case, but that not exactly
terrible. It seems worse to have ExecVacuum() use a random assortment
of casings when reporting errors in the specified options. Unless
someone thinks are should edit all of those to lower case, then I
think the best option is just to make BUFFER_USAGE_LIMIT follow the
existing precedent of upper casing.

David