From: | Petr Jelinek <petr(at)2ndquadrant(dot)com> |
---|---|
To: | Andreas Karlsson <andreas(at)proxel(dot)se>, Andres Freund <andres(at)anarazel(dot)de> |
Cc: | David Rowley <dgrowleyml(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-01-11 19:08:21 |
Message-ID: | 54B2CA25.7090503@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 11/01/15 05:07, Andreas Karlsson wrote:
> On 01/11/2015 02:36 AM, Andres Freund wrote:
>>> @@ -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.
>
I think Andres means you should move the NULL check before the ifdef and
then use curly braces inside the the ifdef/else so that you can define
the state variable there. That can be done with single ifdef.
int2_accum_inv(PG_FUNCTION_ARGS)
{
... null check ...
{
#ifdef HAVE_INT128
Int16AggState *state;
...
#else
NumericAggState *state;
...
#endif
PG_RETURN_POINTER(state);
}
}
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Petr Jelinek | 2015-01-11 19:17:09 | Re: Custom/Foreign-Join-APIs (Re: [v9.5] Custom Plan API) |
Previous Message | Christoph Berg | 2015-01-11 18:17:58 | Re: libpq 9.4 requires /etc/passwd? |