Re: WIP: Rework access method interface

On Sun, Sep 20, 2015 at 5:02 PM, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:

> On 2015-09-18 14:58, Alexander Korotkov wrote:
>
>> On Wed, Sep 16, 2015 at 8:44 PM, Teodor Sigaev <teodor(at)sigaev(dot)ru
>> <mailto:teodor(at)sigaev(dot)ru>> wrote:
>>
>> validate_opclass was renamed to amvalidate.
>>
>>
>> It seems to me, that amvalidate method of AM should get as argument
>> only Oid of operator family. Layout and meaning of amproc/amop
>> fields are differ for different AM and there isn't an AM which
>> implements all possible features.
>>
>> After, further personal discussion with Teodor, we decided that
>> amvalidate is out of scope for this patch.
>> It's not evident what should we validate in amvalidate and which way. I
>> think if we need amvalidate it should be subject of separate patch.
>> The attached patch exposes index access method parameters to SQL using
>> following fucntions:
>> * get_am_param_oid(oid, text)
>> * get_am_param_int(oid, text)
>> * get_am_param_bool(oid, text)
>>
>>
> Hmm, we might want these functons in any case (although I think just one
> function which would return all am params would be better).
>
> But why is it not evident? We do the validations in regression tests, even
> if we just copy those then it's enough for a start
>

The reason is that those validations were used only in regression tests
yet. It wasn't used for user-defined operator classes. User might define
invalid opclass and then alter it to valid. Or user might upgrade opclass
in two steps where intermediate step is invalid. This is why I think
validating opclasses in CREATE/ALTER command is not evident since it
changes user visible behavior and compatibility.
Simultaneously, implementing new API function just for regression tests
doesn't seem to worth it for me.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company