public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Peter Eisentraut <[email protected]>
Cc: [email protected]
Cc: Mark Dilger <[email protected]>
Subject: Re: pgsql: Generalize hash and ordering support in amapi
Date: Thu, 27 Feb 2025 11:27:33 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

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)

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?

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 do not think this was in shape to be committed.  For stuff
like this, clarity of thought and precision of specification
are critical, and this patch has neither.

			regards, tom lane

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

* 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