Re: Using 128-bit integers for sum, avg and statistics aggregates - Mailing list pgsql-hackers

From Oskari Saarenmaa
Subject Re: Using 128-bit integers for sum, avg and statistics aggregates
Date
Msg-id 20141222224747.GC17054@saarenmaa.fi
Whole thread Raw
In response to Re: Using 128-bit integers for sum, avg and statistics aggregates  (Andreas Karlsson <andreas@proxel.se>)
Responses Re: Using 128-bit integers for sum, avg and statistics aggregates  (Andreas Karlsson <andreas@proxel.se>)
List 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



pgsql-hackers by date:

Previous
From: Oskari Saarenmaa
Date:
Subject: Re: pg_basebackup fails with long tablespace paths
Next
From: Christoph Berg
Date:
Subject: pg_upgrade needs postmaster [sic]