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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
