Re: Rethinking opclass member checks and dependency strength

On Wed, Aug 7, 2019 at 7:28 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Over in [1] we realized that it would be a good idea to remove the <@
> operator from contrib/intarray's GiST opclasses. Unfortunately, doing
> that isn't a simple matter of generating an extension update script
> containing ALTER OPERATOR FAMILY DROP OPERATOR, because that operator
> is marked as internally dependent on its opclass which means that
> dependency.c will object. We could do some direct hacking on
> pg_depend to let the DROP be allowed, but ugh.
>
> I started to wonder why GiST opclass operators are ever considered
> as required members of their opclass. The existing rule (cf.
> opclasscmds.c) is that everything mentioned in CREATE OPERATOR CLASS
> will have an internal dependency on the opclass, but if you add
> operators or functions with ALTER OPERATOR FAMILY ADD, those just
> have AUTO dependencies on their operator family. So the assumption
> is that opclass creators will only put the bare minimum of required
> stuff into CREATE OPERATOR CLASS and then add optional stuff with
> ALTER ... ADD. But none of our contrib modules do it like that,
> and I'd lay long odds against any third party code doing it either.

That's really odd. I don't think any extension SQL scripts does
really care about difference between operators defined in CREATE
OPERATOR CLASS and operators defined in ALTER OPERATOR FAMILY ADD.
But if they would care, then all GiST, GIN, SP-GiST and BRIN opclasses
would define all their operators using ALTER OPERATOR FAMILY ADD.

> This leads to the thought that maybe we could put some intelligence
> into an index-AM-specific callback instead. For example, for btree
> and hash the appropriate rule is probably that cross-type operators
> and functions should be tied to the opfamily while single-type
> members are internally tied to the associated opclass. For GiST,
> GIN, and SPGiST it's not clear to me that *any* operator deserves
> an INTERNAL dependency; only the implementation functions do.
>
> Furthermore, if we had an AM callback that were charged with
> deciding the right dependency links for all the operators/functions,
> we could also have it do some validity checking on those things,
> thus moving some of the checks made by amvalidate into a more
> useful place.

+1, sounds like a plan for me.

> If we went along this line, then a dump/restore or pg_upgrade
> would be enough to change an opclass's dependencies to the new
> style, which would get us to a place where intarray's problem
> could be fixed with ALTER OPERATOR FAMILY DROP OPERATOR and
> nothing else. Such an upgrade script wouldn't work in older
> releases, but I think we don't generally care about that.

+1

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