Hi,
Working on committing this:
* Converted the configure test to AC_LINK_IFELSE
* I dislike the way the configure test and the resulting HAVE_* is
named. This imo shouldn't be so gcc specific, even if it right now
only detects gcc support. Changed.
* Furthermore does the test use 64bit literals without marking them as
such. That doesn't strike me as a great idea.
* Stuff like:
static int32 numericvar_to_int4(NumericVar *var);
static bool numericvar_to_int8(NumericVar *var, int64 *result);
static void int64_to_numericvar(int64 val, NumericVar *var);
#ifdef HAVE_INT128
static void int128_to_numericvar(int128 val, NumericVar *var);
#endif
is beyond ugly. Imnsho the only int2/4/8 functions that should keep
their name are the SQL callable ones. It's surely one to have
numericvar_to_int8 and int64_to_numericvar.
* I'm not a fan at all of the c.h comment you added. That comment seems,
besides being oddly formatted, to be designed to be outdated ;) I'll
just rip it out and replace it by something shorter. This shouldn't be
so overly specific for this patch.
* This thread is long, I'm not sure who to list as reviewers. Please
check whether those are appropriate.
* I've split off the int128 support from the aggregate changes.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services