public inbox for [email protected]
help / color / mirror / Atom feedFrom: 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: Fri, 3 Jul 2026 14:10:44 +0530
Message-ID: <CAJTYsWUg+ZH=W5EduH1AEDfJU0waO4uU4hakaiOF+KKUKFbR9g@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]>
<CAJTYsWX1C5JLj92QdqEcArWs4XtXPog+yvyb6=X=_cghjXnkYQ@mail.gmail.com>
<[email protected]>
<CAJTYsWVus+7hn846A0gMJYk+9uUGZxizk5Herbxb4CmhF3vndQ@mail.gmail.com>
<[email protected]>
Hi,
On Fri, 3 Jul 2026 at 04:32, Tom Lane <[email protected]> wrote:
> Ayush Tiwari <[email protected]> writes:
> > On Thu, 2 Jul 2026 at 22:24, Tom Lane <[email protected]> wrote:
> >> Anyway, I think part of our work here has got to be to raise the
> >> level of documentation of this code. I've had it with having
> >> to reverse-engineer details as fundamental as these.
>
> > I've added some documentation in code, patch attached.
>
> I had something considerably more ambitious in mind, so I had a
> go at that --- and was glad that I did so, because the review
> turned up an additional bug as well as some other opportunities
> for improvement.
>
Thanks for working on the patch!
v3-0001 is a documentation-improvement patch. I only touched
> the files associated with btree_utils_var.c. Maybe it's worth
> doing something similar for the btree_utils_num.c group, but
> I have the impression that those are way simpler.
>
0001 LGTM
> v3-0002 is your patch, editorialized a bit. I left out the
> test case because it seemed fairly expensive for something
> that won't expose a bug in normal builds. (BTW, I wonder
> if rather than documenting the comparisons as being reversed,
> we should just flip them all.)
>
I'd lean towards flipping them for readability, though I'm not sure
it's worth the backpatch churn? I guess we could start a new
thread for master on this.
v3-0003 fixes a bug I identified: index builds that use the
> gbt_bit_ssup_cmp infrastructure will sort the entries in a
> way that doesn't match the bit types' actual ordering,
> leading to what's probably a very inefficient index.
It looks right to me, leaf entries are real bit/varbit values, so bitcmp
is the right comparator there.
> v3-0004 and v3-0005 clean up some things that seem like
> dead code to me.
>
One small thing on 0005: once gbt_var_node_cp_len stops using
pg_mblen_range, the "#include "mb/pg_wchar.h"" in btree_utils_var.c
looks like it becomes unused. Might be worth dropping it too.
> Also, it occurs to me after another look at 0002 that what was
> perhaps intended was something like this for the non-leaf case:
>
> if (is_leaf)
> retval = !(tinfo->f_eq(query, key->lower, collation,
> flinfo));
> else
> retval = tinfo->trnc ||
> !(tinfo->f_cmp(query, key->lower, collation, flinfo)
> == 0 &&
> tinfo->f_cmp(query, key->upper, collation, flinfo)
> == 0);
>
> That is, if we're looking at non-truncated keys and lower == upper,
> then we know that all the keys below this node are exactly that
> value, so we can avoid descending if that value is equal to the query.
> Most of the time this would not pay off in any savings, but if you
> had an index on a column with only a few values, maybe there would be
> leaf pages like that? I would think that the planner would avoid
> choosing to implement a <> query with an indexscan unless there was
> a pretty large fraction of the table that <> would reject, so maybe
> this actually is worth doing.
>
I think that optimization is correct. For the non-truncatable types
the internal lower/upper are exact min/max (built via f_cmp in
gbt_var_bin_union with no truncation), so if both equal the query
then everything below does too, and pruning can't drop a match.
Are you planning on including it in the backpatch or to keep the
optimization just part of master, and the bug fix backpatched?
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: <CAJTYsWUg+ZH=W5EduH1AEDfJU0waO4uU4hakaiOF+KKUKFbR9g@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