public inbox for [email protected]  
help / color / mirror / Atom feed
From: Ayush Tiwari <[email protected]>
To: Tom Lane <[email protected]>
Cc: 王跃林 <[email protected]>
Cc: pgsql-bugs <[email protected]>
Cc: 3764353996 <[email protected]>
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
Date: Thu, 2 Jul 2026 21:13:30 +0530
Message-ID: <CAJTYsWX1C5JLj92QdqEcArWs4XtXPog+yvyb6=X=_cghjXnkYQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
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>
	<[email protected]>

Hi,

On Thu, 2 Jul 2026 at 20:08, Tom Lane <[email protected]> wrote:

> =?UTF-8?B?546L6LeD5p6X?= <[email protected]> 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


view thread (13+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  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
  In-Reply-To: <CAJTYsWX1C5JLj92QdqEcArWs4XtXPog+yvyb6=X=_cghjXnkYQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox