Thread: Re: Improving and extending int128.h to more of numeric.c

On Mon, Jun 23, 2025 at 3:01 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> 0001 is a trivial bug fix for the test code in src/tools/testint128.c
> -- it was using "union" instead of "struct" for test128.hl, which
> meant that it was only ever setting and checking half of each 128-bit
> integer in the tests.

Hi Dean, I went to take a look at this and got stuck at building the
test file. The usual pointing gcc to the src and build include
directories didn't cut it. How did you get it to work?

--
John Naylor
Amazon Web Services

On Wed, 9 Jul 2025 at 07:41, John Naylor <johncnaylorls@gmail.com> wrote:
>
> Hi Dean, I went to take a look at this and got stuck at building the
> test file. The usual pointing gcc to the src and build include
> directories didn't cut it. How did you get it to work?
>

Yes, it wasn't immediately obvious how to do it. I first built
postgres as normal, including the  pg_config tool, and then used that
to compile the test as follows:

gcc -O3 -g \
    src/tools/testint128.c \
    -I$(pg_config --includedir-server) \
    -o src/tools/testint128 \
    $(pg_config --libs)

It actually only needs -lpgcommon -lpgport -lm, but it seemed easier
just to include all of the pg_config --libs.

Regards,
Dean

Hi,

On 2025-07-09 10:38:31 +0100, Dean Rasheed wrote:
> On Wed, 9 Jul 2025 at 07:41, John Naylor <johncnaylorls@gmail.com> wrote:
> >
> > Hi Dean, I went to take a look at this and got stuck at building the
> > test file. The usual pointing gcc to the src and build include
> > directories didn't cut it. How did you get it to work?
> >
>
> Yes, it wasn't immediately obvious how to do it. I first built
> postgres as normal, including the  pg_config tool, and then used that
> to compile the test as follows:
>
> gcc -O3 -g \
>     src/tools/testint128.c \
>     -I$(pg_config --includedir-server) \
>     -o src/tools/testint128 \
>     $(pg_config --libs)
>
> It actually only needs -lpgcommon -lpgport -lm, but it seemed easier
> just to include all of the pg_config --libs.

I think we should wire this up to the buildsystem and our testsuite...  Having
testcode that is not run automatically may be helpful while originally
developing something, but it doesn't do anything to detect portability issues
or regressions.

Greetings,

Andres Freund

On Wed, 9 Jul 2025 at 18:27, Andres Freund <andres@anarazel.de> wrote:
>
> I think we should wire this up to the buildsystem and our testsuite...  Having
> testcode that is not run automatically may be helpful while originally
> developing something, but it doesn't do anything to detect portability issues
> or regressions.

Yes, perhaps we should convert src/tools/testint128.c into a new test
extension, src/test/modules/test_int128

Regards,
Dean

On Wed, 9 Jul 2025 at 22:31, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> On Wed, 9 Jul 2025 at 18:27, Andres Freund <andres@anarazel.de> wrote:
> >
> > I think we should wire this up to the buildsystem and our testsuite...  Having
> > testcode that is not run automatically may be helpful while originally
> > developing something, but it doesn't do anything to detect portability issues
> > or regressions.
>
> Yes, perhaps we should convert src/tools/testint128.c into a new test
> extension, src/test/modules/test_int128

Here's an update doing that (in 0001). 0002-0005 are unchanged.

Regards,
Dean

On Thu, 10 Jul 2025 at 15:06, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> > Yes, perhaps we should convert src/tools/testint128.c into a new test
> > extension, src/test/modules/test_int128
>
> Here's an update doing that (in 0001). 0002-0005 are unchanged.

v3 attached, fixing a couple of issues revealed by the cfbot:

1. The tests, as currently written, require a native int128 type to
run. To fix that, for now at least, skip the tests if the platform
lacks a native int128 type. We could perhaps improve on that by using
numerics to compute the expected results.

2. Fix Visual Studio compiler warning about applying a unary minus
operator to an unsigned type.

Regards,
Dean