Re: avoid negating LONG_MIN in cash_out() - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: avoid negating LONG_MIN in cash_out()
Date
Msg-id CALNJ-vRwqcQvwF9MtwHfwwmd7=VeSzf9RM77U0UoMdqf7uKJ5w@mail.gmail.com
Whole thread Raw
In response to Re: avoid negating LONG_MIN in cash_out()  (David Rowley <dgrowleyml@gmail.com>)
Responses Re: avoid negating LONG_MIN in cash_out()
List pgsql-hackers


On Thu, Aug 11, 2022 at 12:55 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Fri, 12 Aug 2022 at 05:58, Zhihong Yu <zyu@yugabyte.com> wrote:
> Here is sample output with patch:
>
> # SELECT '-92233720368547758.085'::money;
> ERROR:  value "-92233720368547758.085" is out of range for type money
> LINE 1: SELECT '-92233720368547758.085'::money;

I'm struggling to follow along here. There are already overflow checks
for this in cash_in(), which is exactly where they should be.

The above case already fails on master, there's even a regression test
to make sure it does for this exact case, just look at money.out:356.
So, if we're already stopping this from happening in cash_in(), why do
you think it also needs to happen in cash_out()?

I'm also not sure why you opted to use LONG_MIN for your check. The C
type "Cash" is based on int64, that's not long.

David

Hi, David:
I am very sorry for not having looked closer at the sample SQL statement earlier.
Indeed, the previous statement didn't trigger cash_out().

I think this was due to the fact that sanitizer assertion may be separated from the statement triggering the assertion.
I am still going over the test output, trying to pinpoint the statement.

Meanwhile, I want to thank you for pointing out the constant shouldn't be used for the boundary check.

How about patch v2 which uses the same check from cash_in() ?
I will see which statement triggers the assertion.

Cheers
Attachment

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Postgres NOT IN vs NOT EXISTS optimization
Next
From: Andres Freund
Date:
Subject: Re: hash_xlog_split_allocate_page: failed to acquire cleanup lock