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

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Andreas Karlsson <andreas(at)proxel(dot)se>
Cc: Peter Geoghegan <pg(at)heroku(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Using 128-bit integers for sum, avg and statistics aggregates
Date: 2015-03-19 18:08:26
Message-ID: 20150319180826.GH9495@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Add-optional-support-for-128bit-integers.patch text/x-patch 6.3 KB
0002-Use-128-bit-math-to-accelerate-some-aggregation-func.patch text/x-patch 36.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-03-19 18:13:59 Re: parallel mode and parallel contexts
Previous Message Robert Haas 2015-03-19 18:06:14 Re: [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...