‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 1 September 2020 20:21, Jeff Davis <pg...@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
>
