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

From: Andreas Karlsson <andreas(at)proxel(dot)se>
To: Oskari Saarenmaa <os(at)ohmu(dot)fi>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Using 128-bit integers for sum, avg and statistics aggregates
Date: 2014-12-23 23:41:13
Message-ID: 5499FD99.5080109@proxel.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12/22/2014 11:47 PM, Oskari Saarenmaa wrote:
> __int128_t and __uint128_t are GCC extensions and are not related to
> stdint.h.
>
>> [...]
>
> 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.

Thanks for the feedback. These two issues will be fixed in the next version.

>> *** 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.

Perhaps. I switched opinion on this several times while coding. On one
side there is consistency, on the other there is the risk of confusing
the different meanings of int16. I am still not sure which of these I
think is the least bad.

>> 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".

At the time of submitting my patch I had not noticed the slow change
from git-external-diff to regular git diffs. The change snuck up on me.
The new version of the patch will be submitted in the standard git
format which is what I am more used to work with.

--
Andreas Karlsson

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2014-12-24 01:06:25 Re: Proposal "VACUUM SCHEMA"
Previous Message Andres Freund 2014-12-23 21:51:40 Re: Lockless StrategyGetBuffer() clock sweep