Re: pgsql: Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option

Поиск
Список
Период
Сортировка
От David Rowley
Тема Re: pgsql: Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option
Дата
Msg-id CAApHDvrvCZ3P1DahDfCWwJ2Hevjw9ZCM9GZHGWSEVhtUpyw_QA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: pgsql: Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Ответы Re: pgsql: Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Список pgsql-committers
On Tue, 11 Apr 2023 at 15:02, Kyotaro Horiguchi <horikyota.ntt@gmail.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?

> 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.

For example, both of these are not upper casing the option name:

ereport(ERROR,
        (errcode(ERRCODE_SYNTAX_ERROR),
        errmsg("parallel option requires a value between 0 and %d",
        MAX_PARALLEL_WORKER_LIMIT),
        parser_errposition(pstate, opt->location)));

ereport(ERROR,
        (errcode(ERRCODE_SYNTAX_ERROR),
        errmsg("parallel workers for vacuum must be between 0 and %d",
        MAX_PARALLEL_WORKER_LIMIT),
        parser_errposition(pstate, opt->location)));

But if you wanted to change that, you'll need to raise a thread on
-hackers as that code has been there for a while.

(actually, it seems pointless to have two error messages for that any
not just one)

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.

David



В списке pgsql-committers по дате отправления:

Предыдущее
От: Kyotaro Horiguchi
Дата:
Сообщение: Re: pgsql: Support invalidating replication slots due to horizon and wal_le
Следующее
От: "Drouvot, Bertrand"
Дата:
Сообщение: Re: pgsql: Allow logical decoding on standbys