From: Nathan Bossart <nathandbossart@gmail.com>
To: Joseph Koshakow <koshy44@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Andres Freund <andres@anarazel.de>
Subject: Re: Remove dependence on integer wrapping
Date: Tue, 16 Jul 2024 12:57:47 -0500
Message-ID: <Zpa0m_OVw_iWYicV@nathan> (raw)
In-Reply-To: <CAAvxfHdYnTB7BjkRSPO8__wxB3880TiYD-TVU2uW8q1KbzymPg@mail.gmail.com>
References: <31ad2cd1-db94-bdb3-f91a-65ffdb4bef95@gmail.com>
<CAAvxfHfqJca-WGvWJQ3ZAtt2po=mkQzU8r9Qjzom4YqxQi84oA@mail.gmail.com>
<CAAvxfHdhEz4eF7z1wNn_4C4Gkt+e-h7OjzKUJXt7CE+CvubqcQ@mail.gmail.com>
<CAAvxfHdze21+UZPWo7ePfH19-9hvXe_9SEZk4HABPHuzaUitRQ@mail.gmail.com>
<CAAvxfHd22ujbY6uTraE+ZSTUN0xX0G-=NZcr0YnKSQuXthcM+g@mail.gmail.com>
<CAAvxfHcDQEziyQ-ehv0Bg05OovGPGO95VoO7-DyxJx6Tkq85vA@mail.gmail.com>
<ZpFeopUrhdB5GgNN@nathan>
<CAAvxfHdX-t03LvxROxbyodBrtzBzZQEKdKB0FCpEr+p1JnFxsA@mail.gmail.com>
<ZpVAvLbtUPnK1JKe@nathan>
<CAAvxfHdYnTB7BjkRSPO8__wxB3880TiYD-TVU2uW8q1KbzymPg@mail.gmail.com>
On Mon, Jul 15, 2024 at 07:55:22PM -0400, Joseph Koshakow wrote:
> On Mon, Jul 15, 2024 at 11:31 AM Nathan Bossart <nathandbossart@gmail.com>
> wrote:
>> I'm curious why you aren't using float8_mul/float8_div here, i.e.,
>>
>> fresult = rint(float8_mul((float8) c, f));
>> fresult = rint(float8_div((float8) c, f));
>
> I wrongly assumed that it was only meant to be used to implement
> multiplication and division for the built-in float types. I've updated
> the patch to use these functions.
The reason I suggested this is so that we could omit all the prerequisite
isinf(), isnan(), etc. checks in the cash_mul_float8() and friends. The
checks are slighly different, but from a quick glance it just looks like we
might end up relying on the FLOAT8_FITS_IN_INT64 check in more cases.
>> and "cash_sub_int64"
>
> Did you mean "cash_div_int64"? There's only a single function that
> subtracts cash and an integer, but there's multiple functions that
> divide cash by an integer. I've added a "cash_div_int64" in the updated
> patch.
My personal preference would be to add helper functions for each of these
so that all the overflow, etc. checks are centralized in one place and
don't clutter the calling code. Plus, it might help ensure error
handling/messages remain consistent.
+static Cash
+cash_mul_float8(Cash c, float8 f)
nitpick: Can you mark these "inline"? I imagine most compilers inline them
without any prompting, but we might as well make our intent clear.
--
nathan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, koshy44@gmail.com, exclusion@gmail.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org, andres@anarazel.de
Subject: Re: Remove dependence on integer wrapping
In-Reply-To: <Zpa0m_OVw_iWYicV@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox