Re: pgsql: Generalize hash and ordering support in amapi

On Thu, Feb 27, 2025 at 8:27 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Peter Eisentraut <peter(at)eisentraut(dot)org> writes:
> > Generalize hash and ordering support in amapi
> > Stop comparing access method OID values against HASH_AM_OID and
> > BTREE_AM_OID, and instead check the IndexAmRoutine for an index to see
> > if it advertises its ability to perform the necessary ordering,
> > hashing, or cross-type comparing functionality. A field amcanorder
> > already existed, this uses it more widely. Fields amcanhash and
> > amcancrosscompare are added for the other purposes.
>
> AFAICS, this patch sets amcancrosscompare true only for btree,
> which means this change to equality_ops_are_compatible is surely wrong:
>
> - /* must be btree or hash */
> - if (op_form->amopmethod == BTREE_AM_OID ||
> - op_form->amopmethod == HASH_AM_OID)
> + if (amroutine->amcancrosscompare)
>

It seems you are right. hashhandler()'s amroutine should have this true,
also.

>
> More generally, I think that "amcancrosscompare" is a horribly
> underspecified and misleadingly-named concept. Most of our
> AMs are capable of supporting cross-type operators, though for
> some it's more about what the per-opclass infrastructure can do
> than what the AM does. So what does that flag *really* mean?
>

The comments in amapi.h seem to have gotten shortened since v19 of the
patch which had them as:

+ /*
+ * Does AM support hashing the indexed column's value, including
providing
+ * all hash semantics functions for HASHSTANDARD_PROC and
HASHEXTENDED_PROC
+ * conforming to the same calling conventions as those of the hash AM?
+ */
+ bool amcanhash;
+ /*
+ * Does AM have equality semantics that are compatible across all
equality
+ * operators within an operator family?
+ */
+ bool amcancrosscompare;

The logic in equality_ops_are_compatible() was trusting that equality
operators found in an opfamily for btree or hash were ok, but not trusting
operators found in opfamilies of other AMs. Now, after the patch, other
AMs can be marked as suitable. That's really the core of what the flag
means: "Can the system trust that equality operators found in opfamilies
of the AM are well-behaved", or something like that. I agree that this
should really be more a feature of an opfamily than an AM, but the system
already does it on AM-level granularity, and I don't think it's the
responsibility of this patch to rearchitect all that.

> I also object strongly to the fact that the comments for
> equality_ops_are_compatible and comparison_ops_are_compatible
> were not modified:
>
> * This is trivially true if they are the same operator. Otherwise,
> * we look to see if they can be found in the same btree or hash opfamily.
>
> * This is trivially true if they are the same operator. Otherwise,
> * we look to see if they can be found in the same btree opfamily.
>

I agree these comments need updating.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company