Re: [PATCH] New [relation] option engine

В письме от среда, 18 мая 2022 г. 11:10:08 MSK пользователь Alvaro Herrera
написал:
> forbid_realloc is only tested in an assert. There needs to be an "if"
> test for it somewhere (suppose some extension author uses this API and
> only runs it in assert-disabled environment; they'll never know they
> made a mistake). But do we really need this option? Why do we need a
> hardcoded limit in the number of options?

General idea was that it is better to allocate as many option_spec items as we
are going to use. It is optional, so if you do not want to allocate exact
number of options, then realloc will be used, when we adding one more item,
and all allocated items are used.

But when you explicitly specify number of items, it is better not to forget to
++ it when you add extra option in the code. That was the purpose of
forbid_realloc: to remind. It worked well for, while working with the patch
several options were added in the upstream, and this assert reminded me that I
should also allocate extra item.

If it is run in production without asserts, it is also no big deal, we will
just have another realloc.

But you are right, variable name forbid_realloc is misleading. It does not
really forbid anything, so I changed it to assert_on_realloc, so the name
tells what this flag really do.

> In allocateOptionsSpecSet there's a new error message with a typo
> "grater" which should be "greater". But I think the message is
> confusingly worded. Maybe a better wording is "the value of parameter
> XXX may not be greater than YYY".

This error message is really from bloom index. And here I was not as careful
and watchful as I should, because this error message is from the check that
was not there in original code. And this patch should not change behaviour at
all. So I removed this check completely, and will submit it later.

My original patch has a bunch of changes like that. I've removed them all, but
missed one in the contrib... :-(

Thank you for pointing to it.
--
Nikolay Shaplov aka Nataraj
Fuzzing Engineer at Postgres Professional
Matrix IM: @dhyan:nataraj.su