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(dot)vondra(at)enterprisedb(dot)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