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

=?UTF-8?B?546L6LeD5p6X?= <violin0613(at)tju(dot)edu(dot)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?

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.

It might well be that gbt_var_consistent's not-equal case is
broken too, but this discussion hasn't established that IMO.
(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?)

regards, tom lane