Re: [PATCH] btree_gist: fix union implementation for variable length columns - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [PATCH] btree_gist: fix union implementation for variable length columns
Date
Msg-id 378.1531330000@sss.pgh.pa.us
Whole thread Raw
In response to Re: [PATCH] btree_gist: fix union implementation for variable length columns  (Pavel Raiskup <praiskup@redhat.com>)
Responses Re: [PATCH] btree_gist: fix union implementation for variable length columns  (Pavel Raiskup <praiskup@redhat.com>)
Re: [PATCH] btree_gist: fix union implementation for variable lengthcolumns  (Teodor Sigaev <teodor@sigaev.ru>)
List pgsql-hackers
Pavel Raiskup <praiskup@redhat.com> writes:
> On Monday, July 9, 2018 7:41:59 PM CEST Tom Lane wrote:
>> Hi Pavel!  For patches that purport to resolve bugs, we usually like to
>> add a regression test case that demonstrates the bug in unpatched code.
>> Can you provide a small test case that does so?  (The BZ you pointed to
>> doesn't seem to address this...)

> Turns out the problem is only related to bit/bit varying type (so the
> patch comments need to be reworded properly, at least) since those are the
> only types which have implemented the f_l2n() callback.

What I'm failing to wrap my head around is why this code exists at all.
AFAICS, gbt_bit_xfrm just forces the bitstring to be zero-padded out to
an INTALIGN boundary, which it wouldn't necessarily be otherwise.  But
why bother?  That should have no effect on the behavior of bit_cmp().
So I'm speculating that the reason nobody has noticed a problem is that
there is no problem because this code is useless and could be ripped out.

It would be easier to figure this out if the btree_gist code weren't
so desperately undocumented.  Teodor, do you remember why it's like
this?

            regards, tom lane


pgsql-hackers by date:

Previous
From: Emre Hasegeli
Date:
Subject: Re: [PATCH] Improve geometric types
Next
From: Alvaro Herrera
Date:
Subject: Re: no partition pruning when partitioning using array type