Fix type-safety problem with parallel aggregate serial/deserialization.

The original specification for this called for the deserialization function
to have signature "deserialize(serialtype) returns transtype", which is a
security violation if transtype is INTERNAL (which it always would be in
practice) and serialtype is not (which ditto).  The patch blithely overrode
the opr_sanity check for that, which was sloppy-enough work in itself,
but the indisputable reason this cannot be allowed to stand is that CREATE
FUNCTION will reject such a signature and thus it'd be impossible for
extensions to create parallelizable aggregates.

The minimum fix to make the signature type-safe is to add a second, dummy
argument of type INTERNAL.  But to lock it down a bit more and make misuse
of INTERNAL-accepting functions less likely, let's get rid of the ability
to specify a "serialtype" for an aggregate and just say that the only
useful serialtype is BYTEA --- which, in practice, is the only interesting
value anyway, due to the usefulness of the send/recv infrastructure for
this purpose.  That means we only have to allow "serialize(internal)
returns bytea" and "deserialize(bytea, internal) returns internal" as
the signatures for these support functions.

In passing fix bogus signature of int4_avg_combine, which I found thanks
to adding an opr_sanity check on combinefunc signatures.

catversion bump due to removing pg_aggregate.aggserialtype and adjusting
signatures of assorted built-in functions.

David Rowley and Tom Lane

Discussion: <27247.1466185504@sss.pgh.pa.us>

1 files changed, 21 insertions, 40 deletions

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 6592ef4d2d9..f0b3b87f4c3 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -3510,10 +3510,7 @@ numeric_avg_combine(PG_FUNCTION_ARGS)
 /*
  * numeric_avg_serialize
  *		Serialize NumericAggState for numeric aggregates that don't require
- *		sumX2. Serializes NumericAggState into bytea using the standard pq API.
- *
- * numeric_avg_deserialize(numeric_avg_serialize(state)) must result in a state
- * which matches the original input state.
+ *		sumX2.
  */
 Datum
 numeric_avg_serialize(PG_FUNCTION_ARGS)
@@ -3564,17 +3561,13 @@ numeric_avg_serialize(PG_FUNCTION_ARGS)
 
 /*
  * numeric_avg_deserialize
- *		Deserialize bytea into NumericAggState	for numeric aggregates that
- *		don't require sumX2. Deserializes bytea into NumericAggState using the
- *		standard pq API.
- *
- * numeric_avg_serialize(numeric_avg_deserialize(bytea)) must result in a value
- * which matches the original bytea value.
+ *		Deserialize bytea into NumericAggState for numeric aggregates that
+ *		don't require sumX2.
  */
 Datum
 numeric_avg_deserialize(PG_FUNCTION_ARGS)
 {
-	bytea	   *sstate = PG_GETARG_BYTEA_P(0);
+	bytea	   *sstate;
 	NumericAggState *result;
 	Datum		temp;
 	StringInfoData buf;
@@ -3582,6 +3575,8 @@ numeric_avg_deserialize(PG_FUNCTION_ARGS)
 	if (!AggCheckCallContext(fcinfo, NULL))
 		elog(ERROR, "aggregate function called in non-aggregate context");
 
+	sstate = PG_GETARG_BYTEA_P(0);
+
 	/*
 	 * Copy the bytea into a StringInfo so that we can "receive" it using the
 	 * standard pq API.
@@ -3619,11 +3614,7 @@ numeric_avg_deserialize(PG_FUNCTION_ARGS)
 /*
  * numeric_serialize
  *		Serialization function for NumericAggState for numeric aggregates that
- *		require sumX2. Serializes NumericAggState into bytea using the standard
- *		pq API.
- *
- * numeric_deserialize(numeric_serialize(state)) must result in a state which
- * matches the original input state.
+ *		require sumX2.
  */
 Datum
 numeric_serialize(PG_FUNCTION_ARGS)
@@ -3683,16 +3674,12 @@ numeric_serialize(PG_FUNCTION_ARGS)
 /*
  * numeric_deserialize
  *		Deserialization function for NumericAggState for numeric aggregates that
- *		require sumX2. Deserializes bytea into into NumericAggState using the
- *		standard pq API.
- *
- * numeric_serialize(numeric_deserialize(bytea)) must result in a value which
- * matches the original bytea value.
+ *		require sumX2.
  */
 Datum
 numeric_deserialize(PG_FUNCTION_ARGS)
 {
-	bytea	   *sstate = PG_GETARG_BYTEA_P(0);
+	bytea	   *sstate;
 	NumericAggState *result;
 	Datum		temp;
 	StringInfoData buf;
@@ -3700,6 +3687,8 @@ numeric_deserialize(PG_FUNCTION_ARGS)
 	if (!AggCheckCallContext(fcinfo, NULL))
 		elog(ERROR, "aggregate function called in non-aggregate context");
 
+	sstate = PG_GETARG_BYTEA_P(0);
+
 	/*
 	 * Copy the bytea into a StringInfo so that we can "receive" it using the
 	 * standard pq API.
@@ -3992,11 +3981,8 @@ numeric_poly_combine(PG_FUNCTION_ARGS)
 
 /*
  * numeric_poly_serialize
- *		Serialize PolyNumAggState into bytea using the standard pq API for
- *		aggregate functions which require sumX2.
- *
- * numeric_poly_deserialize(numeric_poly_serialize(state)) must result in a
- * state which matches the original input state.
+ *		Serialize PolyNumAggState into bytea for aggregate functions which
+ *		require sumX2.
  */
 Datum
 numeric_poly_serialize(PG_FUNCTION_ARGS)
@@ -4067,16 +4053,13 @@ numeric_poly_serialize(PG_FUNCTION_ARGS)
 
 /*
  * numeric_poly_deserialize
- *		Deserialize PolyNumAggState from bytea using the standard pq API for
- *		aggregate functions which require sumX2.
- *
- * numeric_poly_serialize(numeric_poly_deserialize(bytea)) must result in a
- * state which matches the original input state.
+ *		Deserialize PolyNumAggState from bytea for aggregate functions which
+ *		require sumX2.
  */
 Datum
 numeric_poly_deserialize(PG_FUNCTION_ARGS)
 {
-	bytea	   *sstate = PG_GETARG_BYTEA_P(0);
+	bytea	   *sstate;
 	PolyNumAggState *result;
 	Datum		sumX;
 	Datum		sumX2;
@@ -4085,6 +4068,8 @@ numeric_poly_deserialize(PG_FUNCTION_ARGS)
 	if (!AggCheckCallContext(fcinfo, NULL))
 		elog(ERROR, "aggregate function called in non-aggregate context");
 
+	sstate = PG_GETARG_BYTEA_P(0);
+
 	/*
 	 * Copy the bytea into a StringInfo so that we can "receive" it using the
 	 * standard pq API.
@@ -4226,9 +4211,6 @@ int8_avg_combine(PG_FUNCTION_ARGS)
 /*
  * int8_avg_serialize
  *		Serialize PolyNumAggState into bytea using the standard pq API.
- *
- * int8_avg_deserialize(int8_avg_serialize(state)) must result in a state which
- * matches the original input state.
  */
 Datum
 int8_avg_serialize(PG_FUNCTION_ARGS)
@@ -4286,14 +4268,11 @@ int8_avg_serialize(PG_FUNCTION_ARGS)
 /*
  * int8_avg_deserialize
  *		Deserialize bytea back into PolyNumAggState.
- *
- * int8_avg_serialize(int8_avg_deserialize(bytea)) must result in a value which
- * matches the original bytea value.
  */
 Datum
 int8_avg_deserialize(PG_FUNCTION_ARGS)
 {
-	bytea	   *sstate = PG_GETARG_BYTEA_P(0);
+	bytea	   *sstate;
 	PolyNumAggState *result;
 	StringInfoData buf;
 	Datum		temp;
@@ -4301,6 +4280,8 @@ int8_avg_deserialize(PG_FUNCTION_ARGS)
 	if (!AggCheckCallContext(fcinfo, NULL))
 		elog(ERROR, "aggregate function called in non-aggregate context");
 
+	sstate = PG_GETARG_BYTEA_P(0);
+
 	/*
 	 * Copy the bytea into a StringInfo so that we can "receive" it using the
 	 * standard pq API.