On 04/03/15 23:51, Andreas Karlsson wrote:
On 01/29/2015 12:28 AM, Peter Geoghegan wrote:
* I'm not sure about the idea of "polymorphic" catalog functions (that
return the type "internal", but the actual struct returned varying
based on build settings).
I tend to think that things would be better if there was always a
uniform return type for such "internal" type returning functions, but
that *its* structure varied according to the availability of int128
(i.e. HAVE_INT128) at compile time. What we should probably do is have
a third aggregate struct, that encapsulates this idea of (say)
int2_accum() piggybacking on one of either Int128AggState or
NumericAggState directly. Maybe that would be called PolyNumAggState.
Then this common code is all that is needed on both types of build (at
the top of int4_accum(), for example):
PolyNumAggState *state;
state = PG_ARGISNULL(0) ? NULL : (PolyNumAggState *)
PG_GETARG_POINTER(0);
I'm not sure if I'd do this with a containing struct or a simple
"#ifdef HAVE_INT128 typedef #else ... ", but I think it's an
improvement either way.
Not entirely sure exactly what I mean but I have changed to something
like this, relying on typedef, in the attached version of the patch. I
think it looks better after the changes.
I think this made the patch much better indeed.
What I am wondering is if those numeric_int16_* functions that also deal
with either the Int128AggState or NumericAggState should be renamed in
similar fashion.
--
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
