Message-ID: <965933b9-bba2-42d0-8c9a-6b1727728a55@iki.fi>
Date: Tue, 16 Jun 2026 09:37:42 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows
 int32 on deep ltree comparisons, returning the wrong sign
To: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Cc: =?UTF-8?B?546L6LeD5p6X?= <violin0613@tju.edu.cn>,
 pgsql-bugs@postgresql.org
References: 
 <AI6AnABgKW93Qbx1jVzi84r9.8.1781322625756.Hmail.3020001251@tju.edu.cn>
 <CAJTYsWUurV6dBhnyd6R7W1w7Fy6FiG9-F_Swk_GDXQQpHoiciA@mail.gmail.com>
 <a4a639c9-01bc-4d58-b942-32ada123feae@iki.fi>
 <CAJTYsWXhViez1AjDHeTUux-XoE2fUwJyt=Vzm4T2=BDnxvKo6w@mail.gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: 
 <CAJTYsWXhViez1AjDHeTUux-XoE2fUwJyt=Vzm4T2=BDnxvKo6w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/965933b9-bba2-42d0-8c9a-6b1727728a55%40iki.fi>
Precedence: bulk

On 15/06/2026 18:24, Ayush Tiwari wrote:
> On Mon, 15 Jun 2026 at 20:38, Heikki Linnakangas <hlinnaka@iki.fi 
> <mailto: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.

Great, committed. Thanks!

- Heikki