Re: GiST operator class for bool

[Tomas Vondra]

On 11/7/21 20:53, Tomas Vondra wrote:
Hi,
On 11/7/21 17:44, Tom Lane wrote:
Tomas Vondra <tomas.von...@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 Companydiff --git a/src/backend/access/gist/gistvalidate.c b/src/backend/access/gist/gistvalidate.c
index b885fa2b25..31cc4f0a21 100644
--- a/src/backend/access/gist/gistvalidate.c
+++ b/src/backend/access/gist/gistvalidate.c
@@ -131,7 +131,7 @@ gistvalidate(Oid opclassoid)
 											2, 2, INTERNALOID, INTERNALOID);
 				break;
 			case GIST_EQUAL_PROC:
-				ok = check_amproc_signature(procform->amproc, INTERNALOID, false,
+				ok = check_amproc_signature(procform->amproc, INTERNALOID, true,
 											3, 3, opckeytype, opckeytype,
 											INTERNALOID);
 				break;