Re: Using 128-bit integers for sum, avg and statistics aggregates

From: Oskari Saarenmaa <os(at)ohmu(dot)fi>
To: Andreas Karlsson <andreas(at)proxel(dot)se>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Using 128-bit integers for sum, avg and statistics aggregates
Date: 2014-12-22 22:47:48
Message-ID: 20141222224747.GC17054@saarenmaa.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 14, 2014 at 01:57:16AM +0100, Andreas Karlsson wrote:
> *** a/configure.in
> --- b/configure.in
> *************** AC_DEFINE_UNQUOTED(MAXIMUM_ALIGNOF, $MAX
> *** 1751,1756 ****
> --- 1751,1759 ----
> AC_CHECK_TYPES([int8, uint8, int64, uint64], [], [],
> [#include <stdio.h>])
>
> + # Check if platform support gcc style 128-bit integers.
> + AC_CHECK_TYPES([__int128_t, __uint128_t], [], [], [#include <stdint.h>])
> +
> # We also check for sig_atomic_t, which *should* be defined per ANSI
> # C, but is missing on some old platforms.
> AC_CHECK_TYPES(sig_atomic_t, [], [], [#include <signal.h>])

__int128_t and __uint128_t are GCC extensions and are not related to
stdint.h.

> *** a/src/include/pg_config.h.in
> --- b/src/include/pg_config.h.in
> ***************
> *** 198,203 ****
> --- 198,209 ----
> /* Define to 1 if you have __sync_compare_and_swap(int64 *, int64, int64). */
> #undef HAVE_GCC__SYNC_INT64_CAS
>
> + /* Define to 1 if you have __int128_t */
> + #undef HAVE___INT128_T
> +
> + /* Define to 1 if you have __uint128_t */
> + #undef HAVE___UINT128_T
> +
> /* Define to 1 if you have the `getaddrinfo' function. */
> #undef HAVE_GETADDRINFO

These changes don't match what my autoconf does. Not a big deal I guess,
but if this is merged as-is the next time someone runs autoreconf it'll
write these lines differently to a different location of the file and the
change will end up as a part of an unrelated commit.

> *** a/src/backend/utils/adt/numeric.c
> --- b/src/backend/utils/adt/numeric.c
> *************** static void apply_typmod(NumericVar *var
> *** 402,407 ****
> --- 402,410 ----
> static int32 numericvar_to_int4(NumericVar *var);
> static bool numericvar_to_int8(NumericVar *var, int64 *result);
> static void int8_to_numericvar(int64 val, NumericVar *var);
> + #ifdef HAVE_INT128
> + static void int16_to_numericvar(int128 val, NumericVar *var);
> + #endif

Existing code uses int4 and int8 to refer to 32 and 64 bit integers as
they're also PG datatypes, but int16 isn't one and it looks a lot like
int16_t. I think it would make sense to just call it int128 internally
everywhere instead of int16 which isn't used anywhere else to refer to 128
bit integers.

> new file mode 100755

I guess src/tools/git-external-diff generated these bogus "new file mode"
lines? I know the project policy says that context diffs should be used,
but it seems to me that most people just use unified diffs these days so I'd
just use "git show" or "git format-patch -1" to generate the patches. The
ones generated by "git format-patch" can be easily applied by reviewers
using "git am".

/ Oskari

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Christoph Berg 2014-12-22 22:48:52 pg_upgrade needs postmaster [sic]
Previous Message Oskari Saarenmaa 2014-12-22 22:40:30 Re: pg_basebackup fails with long tablespace paths