Re: pgsql: Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option

At Tue, 11 Apr 2023 16:53:42 +1200, David Rowley <dgrowleyml(at)gmail(dot)com> wrote in
> On Tue, 11 Apr 2023 at 15:02, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > > postgres=# vacuum (buffer_usage_limit);
> > > ERROR: buffer_usage_limit requires a valid value
> >
> > The error message doesn't really make much sense to me. In the same
> > context, most of the code seems to use ("%s requires a parameter",
> > buffer_usage_limit) without "valid".
>
> The only other option I see that requires a value in this context is
> "parallel", and what you say is not true for that, so I'm not sure I
> follow what you're referring to with "most of the code". Can you quote
> the code you mean?

My point was just that "valid" seems redundant. (Sorry, but..)
However, looking again, many of the "cases" are like "requires a
<type> value", which looks womewhat similar to "requires a valid
value" when the <type> cannot be represented by a word. Some remaining
example of these "caes" are:

./commands/define.c54: errmsg("%s requires a parameter",
(the "parameter" means the option value for type name)

./libpq/hba.c1319: errmsg("authentication method \"%s\" requires argument \"%s\" to be set", \

But, since I'm not a native speaker, so I don't stress this further
more.

> > vacuum.c:347
> > > errmsg("BUFFER_USAGE_LIMIT cannot be specified for VACUUM FULL")));
> >
> > It seems like that the vacuum options are usually spelled in uppercase
> > at least for vacuum and analyze. In any case, shouldn't we need to
> > unify them in a certain area? (For example, command options for
> > CREATE SUBSCRIPTION is shown in lowercase in the documentation. But,
> > I'm not exactly sure what we should do about it.)
>
> I wouldn't object to making things more consistent in this area with
> regards to the casing of the options in the ERROR messages. However,
> it doesn't really seem like there is much consistency to follow that
> this new code is breaking.

...

> I do agree that it's not very good that I pushed the lowercase version
> of buffer_usage_limit and the same in uppercase for the VACUUM FULL
> conflict. I'll hold back from fixing that until we figure out the
> other stuff.

Thanks for the opinion! I agree to you.

Thanks!

--
Kyotaro Horiguchi
NTT Open Source Software Center