Feedback-ID: ie0a040ee:Fastmail
Message-ID: <5cc4192a-7c28-4a69-a879-477b5ece600c@eisentraut.org>
Date: Sat, 1 Mar 2025 09:23:57 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: pgsql: Generalize hash and ordering support in amapi
To: Mark Dilger <mark.dilger@enterprisedb.com>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers@lists.postgresql.org
References: <E1tngY6-0000UL-2n@gemulon.postgresql.org>
 <2557074.1740673653@sss.pgh.pa.us>
 <CAHgHdKuCQ3Dh8wt9QxWRmezgE62qzYW86T9Mv1n5s3fOJ4G2dQ@mail.gmail.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: 
 <CAHgHdKuCQ3Dh8wt9QxWRmezgE62qzYW86T9Mv1n5s3fOJ4G2dQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/5cc4192a-7c28-4a69-a879-477b5ece600c%40eisentraut.org>
Precedence: bulk

On 27.02.25 23:17, Mark Dilger wrote:
> 
> On Thu, Feb 27, 2025 at 8:27 AM Tom Lane <tgl@sss.pgh.pa.us 
> <mailto:tgl@sss.pgh.pa.us>> wrote:
> 
>     Peter Eisentraut <peter@eisentraut.org
>     <mailto: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.

I have fixed that.  I will come back to the rest of the discussion in a bit.