Hi Nikolay,
Thanks for the revised patch. It applies now no problem, and seems to work fine.
For me, I still find the relopts area quite odd. I wonder if your patch doesn’t
go far enough?
For example, take log_autovacuum_min_duration. It’s described intRelOpts, which
implicitly defines its type (an int), and explicitly sets its min/max, default,
and lock level, as well as it’s kind (HEAP | TOAST). But then it’s described
again in the relopt_parse_elt[] in toast_reloptions() and heap_reloptions(),
which implicitly defines it to apply to HEAP | TOAST, and fact of it being an
int. (It’s, of course, same for all reloptions.)
The relopt_parse_elt[] is hugely entirely duplicative. I wonder if it is not
best simply to consolidate — either push all info into the static
{bool,int,real,string}RelOpts[] structures, or push all into the
relopt_parse_elt[].
I notice the thread earlier talks of some of the APIs being public interfaces,
which may be used by EXTENSIONs. I’m not sure I buy that in its fullest sense.
For sure, an EXTENSION may invoke the APIs. But no EXTENSION can define
new/alter existing options, because the {bool,int,real,string}RelOpts[] are not
currently runtime-extendable. I guess we probably should preserve the API’s
functionality, and allow fillRelOptions(), if provided with a tab, for it to
restrict filling to only those supplied in the tab. But in general core code,
my opinion is that fillRelOptions() could be provided with a NULL tab, and for
it to scavenge all needed information from the static
{bool,int,real,string}RelOpts[] structures.
That links to what I initially found most confusing: AUTOVACUUM_RELOPTIONS. I
understand it’s there because there are a bunch of shared reloptions.
Pre-patch, default_reloptions() meant there was no need for the macro, but your
patch drops default_reloptions(). default_reloptions() is horrible, but I feel
the macro approach is worse. :-)
Sorry for the delay providing the feedback. It took me a few sittings to grok
what was going on, and to work out what I though about it.
denty.
> On 12 Jul 2019, at 15:13, Nikolay Shaplov <[email protected]> wrote:
>
> В письме от четверг, 4 июля 2019 г. 19:44:42 MSK пользователь Dent John
> написал:
>> Hi Nikolay,
>>
>> I have had a crack at re-basing the current patch against 12b2, but I didn’t
>> trivially succeed.
>>
>> It’s probably more my bodged fixing of the rejections than a fundamental
>> problem. But I now get compile fails in — and seems like only — vacuum.c.
>
> I am really sorry for such big delay. Two new relation options were added, it
> needed careful checking while importing them back to the patch.
> I did not find proper timeslot for doing this quick enough.
> Hope I am not terribly late.
> Here goes an updated version.
>
>
>
> <get-rid-of-StrRdOptions_5.diff>
