Re: pgsql: Generalize hash and ordering support in amapi - Mailing list pgsql-committers
| From | Mark Dilger |
|---|---|
| Subject | Re: pgsql: Generalize hash and ordering support in amapi |
| Date | |
| Msg-id | CAHgHdKuCQ3Dh8wt9QxWRmezgE62qzYW86T9Mv1n5s3fOJ4G2dQ@mail.gmail.com Whole thread Raw |
| In response to | Re: pgsql: Generalize hash and ordering support in amapi (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: pgsql: Generalize hash and ordering support in amapi
Re: pgsql: Generalize hash and ordering support in amapi |
| List | pgsql-committers |
On Thu, Feb 27, 2025 at 8:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter@eisentraut.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;
+ * 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.
pgsql-committers by date: