| From: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Cc: | 王跃林 <violin0613(at)tju(dot)edu(dot)cn>, pgsql-bugs(at)postgresql(dot)org |
| Subject: | Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign |
| Date: | 2026-06-15 15:24:56 |
| Message-ID: | CAJTYsWXhViez1AjDHeTUux-XoE2fUwJyt=Vzm4T2=BDnxvKo6w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Hi,
On Mon, 15 Jun 2026 at 20:38, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> 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.
Regards,
Ayush
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-06-15 17:44:31 | Re: BUG #19521: After a minor PostgreSQL update from 14.22 to 14.23, the database goes into an infinite loop. |
| Previous Message | Heikki Linnakangas | 2026-06-15 15:08:05 | Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign |