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),
andexplicitly sets its min/max, default, and lock level, as well as it’s kind (HEAP | TOAST). But then it’s described
againin 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
allinfo 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
sureI buy that in its fullest sense. For sure, an EXTENSION may invoke the APIs. But no EXTENSION can define new/alter
existingoptions, because the {bool,int,real,string}RelOpts[] are not currently runtime-extendable. I guess we probably
shouldpreserve the API’s functionality, and allow fillRelOptions(), if provided with a tab, for it to restrict filling
toonly those supplied in the tab. But in general core code, my opinion is that fillRelOptions() could be provided with
aNULL 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
bunchof 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
thoughabout it.
denty.
> On 12 Jul 2019, at 15:13, Nikolay Shaplov <dhyan@nataraj.su> 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>
