Hi,

> +# Check if platform support gcc style 128-bit integers.
> +AC_CHECK_TYPES([__int128_t, __uint128_t], [], [], [])

Hm, I'm not sure that's sufficent. Three things:

a) Afaics only __int128/unsigned __int128 is defined. See
   https://gcc.gnu.org/onlinedocs/gcc/_005f_005fint128.html

b) I'm doubtful that AC_CHECK_TYPES is a sufficiently good test on all
   platforms. IIRC gcc will generate calls to functions to do the actual
   arithmetic, and I don't think they're guranteed to be available on
   platforms. That how it .e.g. behaves for atomic operations. So my
   guess is that you'll need to check that a program that does actual
   arithmetic still links.

c) Personally I don't see the point of testing __uint128_t. That's just
   a pointless test that makes configure run for longer.

> +#ifdef HAVE_INT128
> +typedef struct Int16AggState
> +{
> +     bool    calcSumX2;      /* if true, calculate sumX2 */
> +     int64   N;                      /* count of processed numbers */
> +     int128  sumX;           /* sum of processed numbers */
> +     int128  sumX2;          /* sum of squares of processed numbers */
> +} Int16AggState;

Not the biggest fan of the variable naming here, but that's not your
fault, you're just consistent which is good.


> @@ -3030,6 +3139,18 @@ int8_avg_accum(PG_FUNCTION_ARGS)
>  Datum
>  int2_accum_inv(PG_FUNCTION_ARGS)
>  {
> +#ifdef HAVE_INT128
> +     Int16AggState *state;
> +
> +     state = PG_ARGISNULL(0) ? NULL : (Int16AggState *) PG_GETARG_POINTER(0);
> +
> +     /* Should not get here with no state */
> +     if (state == NULL)
> +             elog(ERROR, "int2_accum_inv called with NULL state");
> +
> +     if (!PG_ARGISNULL(1))
> +             do_int16_discard(state, (int128) PG_GETARG_INT16(1));
> +#else
>       NumericAggState *state;
>  
>       state = PG_ARGISNULL(0) ? NULL : (NumericAggState *) 
> PG_GETARG_POINTER(0);
> @@ -3049,6 +3170,7 @@ int2_accum_inv(PG_FUNCTION_ARGS)
>               if (!do_numeric_discard(state, newval))
>                       elog(ERROR, "do_numeric_discard failed unexpectedly");
>       }

Hm. It might be nicer to move the if (!state) elog() outside the ifdef,
and add curly parens inside the ifdef.

> --- a/src/include/pg_config.h.in
> +++ b/src/include/pg_config.h.in
> @@ -678,6 +678,12 @@
>  /* Define to 1 if your compiler understands __VA_ARGS__ in macros. */
>  #undef HAVE__VA_ARGS
>  
z> +/* Define to 1 if the system has the type `__int128_t'. */
> +#undef HAVE___INT128_T
> +
> +/* Define to 1 if the system has the type `__uint128_t'. */
> +#undef HAVE___UINT128_T

pg_config.h.win32 should be updated as well.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to