Re: Using 128-bit integers for sum, avg and statistics aggregates
От | Andreas Karlsson |
---|---|
Тема | Re: Using 128-bit integers for sum, avg and statistics aggregates |
Дата | |
Msg-id | 54B1F6F1.7080805@proxel.se обсуждение исходный текст |
Ответ на | Re: Using 128-bit integers for sum, avg and statistics aggregates (Andres Freund <andres@anarazel.de>) |
Ответы |
Re: Using 128-bit integers for sum, avg and statistics aggregates
(Tom Lane <tgl@sss.pgh.pa.us>)
Re: Using 128-bit integers for sum, avg and statistics aggregates (Petr Jelinek <petr@2ndquadrant.com>) Re: Using 128-bit integers for sum, avg and statistics aggregates (Andres Freund <andres@anarazel.de>) |
Список | pgsql-hackers |
On 01/11/2015 02:36 AM, Andres Freund wrote: > a) Afaics only __int128/unsigned __int128 is defined. See > https://gcc.gnu.org/onlinedocs/gcc/_005f_005fint128.html Both GCC and Clang defines both of them. Which you use seems to just be a matter of preference. > 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. I too thought some about this and decided to look at how other projects handled this. The projects I have looked at seems to trust that if __int128_t is defined it will also work. https://github.com/python/cpython/blob/master/configure#L7778 http://cgit.freedesktop.org/cairo/tree/build/configure.ac.system#n88 But after some more searching now I notice that at least gstreamer does not trust this to be true. https://github.com/ensonic/gstreamer/blob/master/configure.ac#L382 Should I fix it to actually compile some code which uses the 128-bit types? > c) Personally I don't see the point of testing __uint128_t. That's just > a pointless test that makes configure run for longer. Ok, will remove it in the next version of the patch. >> @@ -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. The reason I did so was because the type of the state differs and I did not feel like having two ifdef blocks. I have no strong opinion about this though. > pg_config.h.win32 should be updated as well. Is it possible to update it without running Windows? -- Andreas Karlsson
В списке pgsql-hackers по дате отправления: