v4-0004-Review-notes-from-Emre-Hasegeli.patch

From 08c6780374668d5e9fe3be547b7f6465b55d0451 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <x4m@flight.local>
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