Re: Index-only scan for btree_gist turns bpchar to char

On Wed, 05 Jan 2022 at 03:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Lakhin <exclusion@gmail.com> writes:
>> While testing the index-only scan fix, I've discovered that replacing
>> the index-only scan with the index scan changes contrib/btree_gist
>> output because index-only scan for btree_gist returns a string without
>> padding.
>
> Ugh, yeah.  This seems to be because gbt_bpchar_compress() strips
> trailing spaces (using rtrim1) before storing the value.  The
> idea evidently is to simplify gbt_bpchar_consistent, but it's not
> acceptable if the opclass is supposed to support index-only scan.
>
> I see two ways to fix this:
>
> * Disallow index-only scan, by removing the fetch function for this
> opclass.  This'd require a module version bump, so people wouldn't
> get that fix automatically.
>
> * Change gbt_bpchar_compress to not trim spaces (it becomes just
> like gbt_text_compress), and adapt gbt_bpchar_consistent to cope.
> This does nothing for the problem immediately, unless you REINDEX
> affected indexes --- but over time an index's entries would get
> replaced with untrimmed versions.
>
> I also wondered if we could make the fetch function reconstruct the
> padding, but it doesn't seem to have access to the necessary info.
>

If we fix this In the second way, the range query has the same results
in both seq scan and index only scan.  However, it will incur other
problems.  For the following query:

SELECT *, octet_length(a) FROM chartmp WHERE a = '31b0';

Currently, we can get

   a  | octet_length
------+--------------
 31b0 |            4

After fixed, we cannot get any result.  For the equal condition,
we must put the extra spaces to make it work.

Here is a patch for POC testing.

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

diff --git a/contrib/btree_gist/btree_text.c b/contrib/btree_gist/btree_text.c
index 8019d11281..5fd425047f 100644
--- a/contrib/btree_gist/btree_text.c
+++ b/contrib/btree_gist/btree_text.c
@@ -121,16 +121,7 @@ gbt_bpchar_compress(PG_FUNCTION_ARGS)
     }
 
     if (entry->leafkey)
-    {
-
-        Datum        d = DirectFunctionCall1(rtrim1, entry->key);
-        GISTENTRY    trim;
-
-        gistentryinit(trim, d,
-                      entry->rel, entry->page,
-                      entry->offset, true);
-        retval = gbt_var_compress(&trim, &tinfo);
-    }
+        retval = gbt_var_compress(entry, &tinfo);
     else
         retval = entry;
 
@@ -179,7 +170,6 @@ gbt_bpchar_consistent(PG_FUNCTION_ARGS)
     bool        retval;
     GBT_VARKEY *key = (GBT_VARKEY *) DatumGetPointer(entry->key);
     GBT_VARKEY_R r = gbt_var_key_readable(key);
-    void       *trim = (void *) DatumGetPointer(DirectFunctionCall1(rtrim1, PointerGetDatum(query)));
 
     /* All cases served by this function are exact */
     *recheck = false;
@@ -189,7 +179,7 @@ gbt_bpchar_consistent(PG_FUNCTION_ARGS)
         tinfo.eml = pg_database_encoding_max_length();
     }
 
-    retval = gbt_var_consistent(&r, trim, strategy, PG_GET_COLLATION(),
+    retval = gbt_var_consistent(&r, query, strategy, PG_GET_COLLATION(),
                                 GIST_LEAF(entry), &tinfo, fcinfo->flinfo);
     PG_RETURN_BOOL(retval);
 }