From: | Joseph Koshakow <koshy44(at)gmail(dot)com> |
---|---|
To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
Cc: | Matthew Kim <matthewkmkim(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de> |
Subject: | Re: Remove dependence on integer wrapping |
Date: | 2024-08-08 01:40:46 |
Message-ID: | CAAvxfHckS2181hMcr+N2HehUa-HuN-fyJV3WGfth=8tSZGQb-g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Aug 7, 2024 at 11:08 AM Nathan Bossart <nathandbossart(at)gmail(dot)com>
wrote:
>
> I started looking at 0001 again with the intent of committing it, and this
> caught my eye:
>
> - /* make the amount positive for digit-reconstruction loop */
> - value = -value;
> + /*
> + * make the amount positive for digit-reconstruction loop, we can
> + * leave INT64_MIN unchanged
> + */
> + pg_neg_s64_overflow(value, &value);
>
> The comment mentions that we can leave the minimum value unchanged, but it
> doesn't explain why. Can we explain why?
I went back to try and figure this out and realized that it would be
much simpler to just convert value to an unsigned integer and not worry
about overflow. So I've updated the patch to do that.
> +static inline bool
> +pg_neg_s64_overflow(int64 a, int64 *result)
> +{
> + if (unlikely(a == PG_INT64_MIN))
> + {
> + return true;
> + }
> + else
> + {
> + *result = -a;
> + return false;
> + }
> +}
>
> Can we add a comment that these routines do not set "result" when true is
> returned?
I've added a comment to the top of the file where we describe the
return values of the other functions.
I also updated the implementations of the pg_abs_sX() functions to
something a bit simpler. This was based on feedback in another patch
[0], and more closely matches similar logic in other places.
Thanks,
Joseph Koshakow
[0]
https://postgr.es/m/CAAvxfHdTsMZPWEHUrZ=h3cky9Ccc3Mtx2whUHygY+ABP-mCmUw@mail.gmail.co
Attachment | Content-Type | Size |
---|---|---|
v17-0003-Handle-overflows-in-do_to_timestamp.patch | text/x-patch | 3.1 KB |
v17-0002-Remove-dependence-on-integer-wrapping-for-jsonb.patch | text/x-patch | 2.7 KB |
v17-0001-Remove-dependence-on-integer-wrapping.patch | text/x-patch | 14.4 KB |
v17-0004-Handle-negative-years-overflow-in-make_date.patch | text/x-patch | 1.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Junwang Zhao | 2024-08-08 01:47:20 | Re: Support specify tablespace for each merged/split partition |
Previous Message | Michael Paquier | 2024-08-08 00:59:15 | Re: Enable data checksums by default |