public inbox for [email protected]  

help / color / mirror / Atom feed

From: Mark Dilger <[email protected]>
To: Tom Lane <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: [email protected]
Subject: Re: pgsql: Generalize hash and ordering support in amapi
Date: Thu, 27 Feb 2025 14:17:27 -0800
Message-ID: <CAHgHdKuCQ3Dh8wt9QxWRmezgE62qzYW86T9Mv1n5s3fOJ4G2dQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>

On Thu, Feb 27, 2025 at 8:27 AM Tom Lane <[email protected]> wrote:

> Peter Eisentraut <[email protected]> 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

view thread (10+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pgsql: Generalize hash and ordering support in amapi
  In-Reply-To: <CAHgHdKuCQ3Dh8wt9QxWRmezgE62qzYW86T9Mv1n5s3fOJ4G2dQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox