From 08c6780374668d5e9fe3be547b7f6465b55d0451 Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Thu, 16 Dec 2021 14:17:15 +0500 Subject: [PATCH v4 4/4] Review notes from Emre Hasegeli --- contrib/btree_gist/btree_bit.c | 12 +++++++++++- contrib/btree_gist/btree_text.c | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/contrib/btree_gist/btree_bit.c b/contrib/btree_gist/btree_bit.c index 947e3b63acd..6b70bcbd0db 100644 --- a/contrib/btree_gist/btree_bit.c +++ b/contrib/btree_gist/btree_bit.c @@ -216,7 +216,17 @@ gbt_bit_sort_build_cmp(Datum a, Datum b, SortSupport ssup) { GBT_VARKEY_R ra = gbt_var_key_readable((GBT_VARKEY *) PG_DETOAST_DATUM(a)); GBT_VARKEY_R rb = gbt_var_key_readable((GBT_VARKEY *) PG_DETOAST_DATUM(b)); - /* Use byteacmp(), like gbt_bitcmp() does */ + /* + * Use byteacmp(), like gbt_bitcmp() does. + * In btree_gist, the *_cmp() function operates on non-leaf values, and + * *_lt(), *_gt() et al operate on leaf values. For all other datatypes, + * the leaf and non-leaf representation is the same, but for bit/varbit, + * the non-leaf representation is different. + * The leaf representation is VarBit, and non-leaf is just the bits without + * the 'bit_len' field. That's why it is indeed correct for gbt_bitcmp() to + * just use byteacmp(), whereas gbt_bitlt() et al compares the 'bit_len' + * field separately. + */ return DatumGetInt32(DirectFunctionCall2(byteacmp, PointerGetDatum(ra.lower), PointerGetDatum(rb.lower))); diff --git a/contrib/btree_gist/btree_text.c b/contrib/btree_gist/btree_text.c index 2deb8dd76fc..69490788cec 100644 --- a/contrib/btree_gist/btree_text.c +++ b/contrib/btree_gist/btree_text.c @@ -260,6 +260,10 @@ gbt_text_sortsupport(PG_FUNCTION_ARGS) { SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0); + /* + * Text has abbreviation routines in varlena.c, but we don't try to use + * them here. Maybe later. + */ ssup->comparator = gbt_text_sort_build_cmp; ssup->abbrev_converter = NULL; ssup->abbrev_abort = NULL; -- 2.33.1