Re: GiST operator class for bool

On 11/7/21 20:53, Tomas Vondra wrote:
> Hi,
> 
> On 11/7/21 17:44, Tom Lane wrote:
>> Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
>>> Pushed, after adding some simple EXPLAIN to the regression test.
>>
>> skink is reporting that this has some valgrind issues [1].
>> I suspect sloppy conversion between bool and Datum, but
>> didn't go looking.
>>
> 
> It's actually a bit worse than that :-( The opclass is somewhat confused 
> about the type it should use for storage. The gbtree_ninfo struct says 
> it's using gbtreekey4, the SQL script claims the params are gbtreekey8, 
> and it should actually use gbtreekey2. Sorry for not noticing that.
> 
> The attached patch fixes the valgrind error for me.
> 

I've pushed the fix, hopefully that'll make skink happy.

What surprised me a bit is that the opclass used gbtreekey4 storage, the 
equality support proc was defined as using gbtreekey8

     FUNCTION    7    gbt_bool_same (gbtreekey8, gbtreekey8, internal),

yet the gistvalidate() did not report this. Turns out this is because

     ok = check_amproc_signature(procform->amproc, INTERNALOID, false,
                                 3, 3, opckeytype, opckeytype,
                                 INTERNALOID);

i.e. with exact=false, so these type differences are ignored. Changing 
it to true reports the issue (and no other issues in check-world).

But maybe there are reasons to keep using false?

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company