Re: Reloptions for table access methods

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 1 September 2020 20:21, Jeff Davis <pgsql@j-davis.com> wrote:


>
> I'm fine removing the "validate" parameter completely for the sake of
> consistency.

FWIW, the more I think about this, I would agree with the removal.
However, isn't this mechanism used for other things too, e.g. attribute_reloptions,
tablespace_reloptions, that rely on the validation at that layer?. If my understanding
is correct, then simply removing the parameter would not cut it and a more extensive
refactoring would be needed.

>
> > [snip]
>
> The problem case would be a situation like the following:
>
> 1.  Someone loads an extension that creates a new reloption
>     "custom_reloption" of kind RELOPT_KIND_HEAP for their table access
>     method "my_tableam".
>
> 2.  They then create a table and forget to specify "USING my_tableam",
>     but use the option "custom_reloption=123".
>
>     Ideally, that would throw an error because "custom_reloption" is only
>     valid for "my_tableam"; but with my patch, no error would be thrown
>     because the extension has already added the reloption. It would just
>     create a normal heap and "custom_reloption=123" would be ignored.

This is something that I struggle to understand as an "error". In the example,
the set RELOPT_KIND_HEAP was extended for everyone. Regardless of whether
the newly added member will be used or not.

I mean, if the intention was to add reloptions specific to the extension,
shouldn't a new RELOPT_KIND_XXX be introduced? You seem to be thinking
along the same lines. Please, correct me if I understand you wrong.

What I am trying to say, is that with the current patch, I feel the behaviour
is not strange nor unexpected.


> I went with the simple approach because fixing that problem seemed a
> bit over-engineered.

Fixing that problem seems worth it on the long run. I do see the benefit of
the simple approach on the meantime.

//Georgios

>
>     Regards,
>     Jeff Davis
>