MIME-Version: 1.0
References: 
 <AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.Hmail.3020001251@tju.edu.cn>
 <CAJTYsWWSx+5-kVaivB_znaQCk3TmHrO4QNNM-CD3AN-oOSmG8w@mail.gmail.com>
 <AGoA0AD5KvwQ8G7e6ofMyap6.3.1781760245047.Hmail.3020001251@tju.edu.cn>
 <3748108.1783003096@sss.pgh.pa.us>
In-Reply-To: <3748108.1783003096@sss.pgh.pa.us>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Thu, 2 Jul 2026 21:13:30 +0530
Message-ID: 
 <CAJTYsWX1C5JLj92QdqEcArWs4XtXPog+yvyb6=X=_cghjXnkYQ@mail.gmail.com>
Subject: Re: Fw:Re: Fw: gbt_var_consistent in
 contrib/btree_gist/btree_utils_var.c
 has internal-node type confusion on the <> strategy,
 bypassing exclusion constraints
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: =?UTF-8?B?546L6LeD5p6X?= <violin0613@tju.edu.cn>,
	pgsql-bugs <pgsql-bugs@postgresql.org>, 3764353996 <3764353996@qq.com>
Content-Type: multipart/alternative; boundary="0000000000003161120655a2ac23"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWX1C5JLj92QdqEcArWs4XtXPog%2Byvyb6%3DX%3D_cghjXnkYQ%40mail.gmail.com>
Precedence: bulk

--0000000000003161120655a2ac23
Content-Type: text/plain; charset="UTF-8"

Hi,

On Thu, 2 Jul 2026 at 20:08, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> =?UTF-8?B?546L6LeD5p6X?= <violin0613@tju.edu.cn> writes:
> > The analysis and fix look correct. The BtreeGistNotEqual branch is
> > the only strategy that bypasses the is_leaf check and passes
> > potentially truncated internal-node keys directly to f_eq, which is
> > unsafe for types like bit and varbit that require a minimum valid
> > header size. Returning true for internal nodes is the right
> > conservative choice and is consistent with how the other strategies
> > handle the internal-node case.
>
> None of this passes the smell test for me.  If it's unsafe to
> call the type's f_eq function on a truncated key, how is it
> any safer to call the f_cmp function?  IOW, why aren't *all* the
> cases in gbt_var_consistent() broken?
>

I think the difference is that for bit/varbit f_eq and f_cmp aren't the
same kind of function.  f_eq is biteq (reads bit_len, does VARSIZE-8),
while f_cmp is byteacmp (plain byte-wise, only needs the varlena
header).  On internal pages every other strategy goes through
f_cmp/byteacmp, and only the <> branch reaches f_eq.  So I believe
that's why the byte-wise cases stay safe on a truncated key and <>
doesn't.


> It looks to me like gbt_var_node_truncate adjusts the length words
> of the truncated keys so that they are still valid, just shorter.
> Or at least it's trying to.  That works fine for text and bytea,
> but it's not fine for bit/varbit because (a) it fails to update the
> "bit_len" field that follows the length word, and (b) the common
> prefix length selection logic doesn't know that it mustn't truncate
> away any part of the bit_len field.  So my own thought about fixing
> this is that type bit needs a custom truncation method.
>

That was my first assumption too, but it looks like the node key has
no bit_len to preserve: gbt_bit_l2n/gbt_bit_xfrm seem to drop it and
keep only the raw bits before truncation runs.  If so, a custom
truncation may not help, since the crash seems to come from <> calling
biteq rather than a damaged bit_len.  Though I'm not fully sure I'm
reading the l2n step right.


> (I continue to regret that we ever accepted such underdocumented code.
> I think we ought to reverse-engineer a comment explaining what
> gbt_var_consistent is doing, eg, why are all of the tests seemingly
> reversed?)
>

Agreed, we can add a comment there.

Regards,
Ayush

--0000000000003161120655a2ac23
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi,</div><br><div class=3D"gmail_quote gm=
ail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Thu, 2 Jul 20=
26 at 20:08, Tom Lane &lt;<a href=3D"mailto:tgl@sss.pgh.pa.us">tgl@sss.pgh.=
pa.us</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"ma=
rgin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:=
1ex">=3D?UTF-8?B?546L6LeD5p6X?=3D &lt;<a href=3D"mailto:violin0613@tju.edu.=
cn" target=3D"_blank">violin0613@tju.edu.cn</a>&gt; writes:<br>
&gt; The analysis and fix look correct. The BtreeGistNotEqual branch is<br>
&gt; the only strategy that bypasses the is_leaf check and passes<br>
&gt; potentially truncated internal-node keys directly to f_eq, which is<br=
>
&gt; unsafe for types like bit and varbit that require a minimum valid<br>
&gt; header size. Returning true for internal nodes is the right<br>
&gt; conservative choice and is consistent with how the other strategies<br=
>
&gt; handle the internal-node case.<br>
<br>
None of this passes the smell test for me.=C2=A0 If it&#39;s unsafe to<br>
call the type&#39;s f_eq function on a truncated key, how is it<br>
any safer to call the f_cmp function?=C2=A0 IOW, why aren&#39;t *all* the<b=
r>
cases in gbt_var_consistent() broken?<br></blockquote><div><br></div>I thin=
k the difference is that for bit/varbit f_eq and f_cmp aren&#39;t the<br>sa=
me kind of function. =C2=A0f_eq is biteq (reads bit_len, does VARSIZE-8),<b=
r>while f_cmp is byteacmp (plain byte-wise, only needs the varlena<br>heade=
r).=C2=A0 On internal pages every other strategy goes through<br>f_cmp/byte=
acmp, and only the &lt;&gt; branch reaches f_eq.=C2=A0 So I believe<br>that=
&#39;s why the byte-wise cases stay safe on a truncated key and &lt;&gt;<br=
><div><span style=3D"background-color:transparent">doesn&#39;t.<br></span>=
=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0=
.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
It looks to me like gbt_var_node_truncate adjusts the length words<br>
of the truncated keys so that they are still valid, just shorter.<br>
Or at least it&#39;s trying to.=C2=A0 That works fine for text and bytea,<b=
r>
but it&#39;s not fine for bit/varbit because (a) it fails to update the<br>
&quot;bit_len&quot; field that follows the length word, and (b) the common<=
br>
prefix length selection logic doesn&#39;t know that it mustn&#39;t truncate=
<br>
away any part of the bit_len field.=C2=A0 So my own thought about fixing<br=
>
this is that type bit needs a custom truncation method.<br></blockquote><di=
v><br>That was my first assumption too, but it looks like the node key has<=
br>no bit_len to preserve: gbt_bit_l2n/gbt_bit_xfrm seem to drop it and<br>=
keep only the raw bits before truncation runs.=C2=A0 If so, a custom<br>tru=
ncation may not help, since the crash seems to come from &lt;&gt; calling<b=
r>biteq rather than a damaged bit_len.=C2=A0 Though I&#39;m not fully sure =
I&#39;m<br>reading the l2n step right.<br></div><div>=C2=A0</div><blockquot=
e class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px s=
olid rgb(204,204,204);padding-left:1ex">
(I continue to regret that we ever accepted such underdocumented code.<br>
I think we ought to reverse-engineer a comment explaining what<br>
gbt_var_consistent is doing, eg, why are all of the tests seemingly<br>
reversed?)<br></blockquote><div><br>Agreed, we can add a comment there.<br>=
<br>Regards,<br>Ayush<br>=C2=A0</div></div></div>

--0000000000003161120655a2ac23--