public inbox for [email protected]
help / color / mirror / Atom feedFrom: Heikki Linnakangas <[email protected]>
To: Ayush Tiwari <[email protected]>
Cc: 王跃林 <[email protected]>
Cc: [email protected]
Subject: Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign
Date: Tue, 16 Jun 2026 09:37:42 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAJTYsWXhViez1AjDHeTUux-XoE2fUwJyt=Vzm4T2=BDnxvKo6w@mail.gmail.com>
References: <AI6AnABgKW93Qbx1jVzi84r9.8.1781322625756.Hmail.3020001251@tju.edu.cn>
<CAJTYsWUurV6dBhnyd6R7W1w7Fy6FiG9-F_Swk_GDXQQpHoiciA@mail.gmail.com>
<[email protected]>
<CAJTYsWXhViez1AjDHeTUux-XoE2fUwJyt=Vzm4T2=BDnxvKo6w@mail.gmail.com>
On 15/06/2026 18:24, Ayush Tiwari wrote:
> On Mon, 15 Jun 2026 at 20:38, Heikki Linnakangas <[email protected]
> <mailto:[email protected]>> wrote:
>
> On 13/06/2026 09:12, Ayush Tiwari wrote:
> > This looks like a classic case of integer overflow that's
> > happening in ltree_compare function in ltree_op.c.
> >
> > return (al->len - bl->len) * 10 * (an + 1);
> > return res * 10 * (an + 1);
> > return (a->numlevel - b->numlevel) * 10 * (an + 1);
> >
> > I think the calculation should be done as int64, something of
> this sort:
>
> Yeah, that works. However, I note that the multiplication is only
> really
> needed by the ltree_penalty() caller. All the other callers just check
> if the return value is less than, equal, or greater than zero. It feels
> a little silly to do all that work of multiplication and clamping for
> those callers. And for ltree_penalty(), the caller actually converts
> the
> return value to a float, so clamping it to int32 range feels a little
> silly for that too. So I propose the attached, which splits the
> ltree_compare() function into two variants: one for ltree_penalty()
> that
> returns a float, and one for others that don't care about the
> "magnitude". It duplicates a little code, but I think it's easier to
> reason about. What do you think?
>
>
> I had thought initially of using the method you have added,
> (not with float return-type though, I thought of planning to create
> a duplicate function with int64 type just for ltree_penalty(), but float
> type
> is better) noting the same thing that rest callers just care about
> comparison with 0.
>
> But thought backpatching it would be harder, hence I used the int64
> method. However, your patch looks much better than the ugly
> behaviour and I agree it did not make sense to do those
> multiplications at all other comparison functions.
Great, committed. Thanks!
- Heikki
view thread (5+ messages)
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]
Subject: Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign
In-Reply-To: <[email protected]>
* 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