Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign - Mailing list pgsql-bugs

From Ayush Tiwari
Subject Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign
Date
Msg-id CAJTYsWXhViez1AjDHeTUux-XoE2fUwJyt=Vzm4T2=BDnxvKo6w@mail.gmail.com
Whole thread
In response to Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign
List pgsql-bugs
Hi,

On Mon, 15 Jun 2026 at 20:38, Heikki Linnakangas <hlinnaka@iki.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

pgsql-bugs by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign
Next
From: Michael Paquier
Date:
Subject: Re: BUG #19520: PANIC when concurrently manipulating stored procedures with pg_stat_statements and track_functions =