Re: Rethinking opclass member checks and dependency strength

On Sun, Aug 18, 2019 at 10:00 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Here's a preliminary patch along these lines. It adds an AM callback
> that can adjust the dependency types before they're entered into
> pg_depend. There's a lot of stuff that's open for debate and/or
> remains to be done:
>
> * Is the parameter list of amcheckmembers() sufficient, or should we
> pass more info (if so, what)? In principle, the AM can always look
> up anything else it needs to know from the provided OIDs, but that
> would be cumbersome if many AMs need the same info.

Looks sufficient to me. I didn't yet imagine something else useful.

> * Do we need any more flexibility in the set of ways that the pg_depend
> entries can be set up than what I've provided here?

Flexibility also looks sufficient to me.

> * Are the specific ways that the entries are getting set up appropriate?
> Note in particular that I left btree/hash alone, feeling that the default
> (historical) behavior was designed for them and is not unreasonable; but
> maybe we should switch them to the cross-type-vs-not-cross-type behavior
> proposed above. Also I didn't touch BRIN because I don't know enough
> about it to be sure what would be best, and I didn't touch contrib/bloom
> because I don't care too much about it.

I think we need ability to remove GiST fetch proc. Presence of this
procedure is used to determine whether GiST index supports index only
scan (IOS). We need to be able to remove this proc to drop IOS
support.

> * I refactored things a little bit in opclasscmds.c, mostly by adding
> an is_func boolean to OpFamilyMember and getting rid of parameters
> equivalent to that. This is based on the thought that AMs might prefer
> to process the structs based on such a flag rather than by keeping them
> in separate lists. We could go further and merge the operator and
> function structs into one list, forcing the is_func flag to be used;
> but I'm not sure whether that'd be an improvement.

I'm also not sure about this. Two lists look OK to me.

> * I'm not at all impressed with the name, location, or concept of
> opfam_internal.h. I think we should get rid of that header and put
> the OpFamilyMember struct somewhere else. Given that this patch
> makes it part of the AM API, it wouldn't be unreasonable to move it
> to amapi.h. But I've not done that here.

+1

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