Re: [PATCH] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind for custom AM

В письме от 26 марта 2017 15:02:12 пользователь Alvaro Herrera написал:
> Nikolay Shaplov wrote:
> > If I would think about it now: we always know how many options we will
> > have. So we can just pass this number to palloc and assert if somebody
> > adds more options then expected... What do yo think about it.
>
> I think we need to preserve the ability to add custom options, because
> extensions may want to do that.

I've been thinking about it for a while... I think this might lead to reducing
the quality of the code...

For example: There was 10 options for some relation type. I decided to add one
more option, but i did not ++ the number of options for
allocateOptionsCatalog. So space for 10 options were allocated, and then when
11th option is added, optionCatalogAddItem would allocate space for ten more
options, and nine of them would not be used. And nobody will notice it.

So, I see here four solutions:

0. Leave it as it was. (We both do not like it)
1. Forbid dynamic number of options (you do not like it)
2. Allow dynamic number of options only for special cases, and in all other
cases make them strict, and asserting if option number is wrong. This can be
done in two ways:
2a. Add strict boolean flag, that tells if allow to add more options or not
2b. Do not add any flags, but if number of items is specified, then process
number of items in strict mode. If it is set to -1, then do as it is done now,
totally dynamically.

I would prefer 2b, if you sure that somebody will need dynamic number of
options.

PS. I hardly believe that there can be dynamic number of options, as this
options later are mapped into C-structure that is quite static. No use case
comes into my mind, where I would like to have dynamic number of options, not
knowing at build time, how many of them there would be.

--
Nikolay Shaplov, independent Perl & C/C++ developer. Available for hire.