Fix array coercion expressions to ensure that the correct volatility is

seen by code inspecting the expression.  The best way to do this seems
to be to drop the original representation as a function invocation, and
instead make a special expression node type that represents applying
the element-type coercion function to each array element.  In this way
the element function is exposed and will be checked for volatility.
Per report from Guillaume Smet.

5 files changed, 55 insertions, 266 deletions

diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index 43acdffcaf0..38a86452e3f 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.137 2007/02/27 23:48:07 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.138 2007/03/27 23:21:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -95,10 +95,6 @@ static void array_insert_slice(ArrayType *destArray, ArrayType *origArray,
 				   int *st, int *endp,
 				   int typlen, bool typbyval, char typalign);
 static int	array_cmp(FunctionCallInfo fcinfo);
-static Datum array_type_length_coerce_internal(ArrayType *src,
-								  int32 desttypmod,
-								  bool isExplicit,
-								  FmgrInfo *fmgr_info);
 
 
 /*
@@ -4094,222 +4090,6 @@ array_insert_slice(ArrayType *destArray,
 }
 
 /*
- * array_type_coerce -- allow explicit or assignment coercion from
- * one array type to another.
- *
- * array_type_length_coerce -- the same, for cases where both type and length
- * coercion are done by a single function on the element type.
- *
- * Caller should have already verified that the source element type can be
- * coerced into the target element type.
- */
-Datum
-array_type_coerce(PG_FUNCTION_ARGS)
-{
-	ArrayType  *src = PG_GETARG_ARRAYTYPE_P(0);
-	FmgrInfo   *fmgr_info = fcinfo->flinfo;
-
-	return array_type_length_coerce_internal(src, -1, false, fmgr_info);
-}
-
-Datum
-array_type_length_coerce(PG_FUNCTION_ARGS)
-{
-	ArrayType  *src = PG_GETARG_ARRAYTYPE_P(0);
-	int32		desttypmod = PG_GETARG_INT32(1);
-	bool		isExplicit = PG_GETARG_BOOL(2);
-	FmgrInfo   *fmgr_info = fcinfo->flinfo;
-
-	return array_type_length_coerce_internal(src, desttypmod,
-											 isExplicit, fmgr_info);
-}
-
-static Datum
-array_type_length_coerce_internal(ArrayType *src,
-								  int32 desttypmod,
-								  bool isExplicit,
-								  FmgrInfo *fmgr_info)
-{
-	Oid			src_elem_type = ARR_ELEMTYPE(src);
-	typedef struct
-	{
-		Oid			srctype;
-		Oid			desttype;
-		FmgrInfo	coerce_finfo;
-		ArrayMapState amstate;
-	} atc_extra;
-	atc_extra  *my_extra;
-	FunctionCallInfoData locfcinfo;
-
-	/*
-	 * We arrange to look up the coercion function only once per series of
-	 * calls, assuming the input data type doesn't change underneath us.
-	 * (Output type can't change.)
-	 */
-	my_extra = (atc_extra *) fmgr_info->fn_extra;
-	if (my_extra == NULL)
-	{
-		fmgr_info->fn_extra = MemoryContextAllocZero(fmgr_info->fn_mcxt,
-													 sizeof(atc_extra));
-		my_extra = (atc_extra *) fmgr_info->fn_extra;
-	}
-
-	if (my_extra->srctype != src_elem_type)
-	{
-		Oid			tgt_type = get_fn_expr_rettype(fmgr_info);
-		Oid			tgt_elem_type;
-		Oid			funcId;
-
-		if (tgt_type == InvalidOid)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("could not determine target array type")));
-
-		tgt_elem_type = get_element_type(tgt_type);
-		if (tgt_elem_type == InvalidOid)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("target type is not an array")));
-
-		/*
-		 * We don't deal with domain constraints yet, so bail out. This isn't
-		 * currently a problem, because we also don't support arrays of domain
-		 * type elements either. But in the future we might. At that point
-		 * consideration should be given to removing the check below and
-		 * adding a domain constraints check to the coercion.
-		 */
-		if (getBaseType(tgt_elem_type) != tgt_elem_type)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("array coercion to domain type elements not "
-							"currently supported")));
-
-		if (!find_coercion_pathway(tgt_elem_type, src_elem_type,
-								   COERCION_EXPLICIT, &funcId))
-		{
-			/* should never happen, but check anyway */
-			elog(ERROR, "no conversion function from %s to %s",
-				 format_type_be(src_elem_type),
-				 format_type_be(tgt_elem_type));
-		}
-		if (OidIsValid(funcId))
-			fmgr_info_cxt(funcId, &my_extra->coerce_finfo, fmgr_info->fn_mcxt);
-		else
-			my_extra->coerce_finfo.fn_oid = InvalidOid;
-		my_extra->srctype = src_elem_type;
-		my_extra->desttype = tgt_elem_type;
-	}
-
-	/*
-	 * If it's binary-compatible, modify the element type in the array header,
-	 * but otherwise leave the array as we received it.
-	 */
-	if (my_extra->coerce_finfo.fn_oid == InvalidOid)
-	{
-		ArrayType  *result;
-
-		result = (ArrayType *) DatumGetPointer(datumCopy(PointerGetDatum(src),
-														 false, -1));
-		ARR_ELEMTYPE(result) = my_extra->desttype;
-		PG_RETURN_ARRAYTYPE_P(result);
-	}
-
-	/*
-	 * Use array_map to apply the function to each array element.
-	 *
-	 * We pass on the desttypmod and isExplicit flags whether or not the
-	 * function wants them.
-	 */
-	InitFunctionCallInfoData(locfcinfo, &my_extra->coerce_finfo, 3,
-							 NULL, NULL);
-	locfcinfo.arg[0] = PointerGetDatum(src);
-	locfcinfo.arg[1] = Int32GetDatum(desttypmod);
-	locfcinfo.arg[2] = BoolGetDatum(isExplicit);
-	locfcinfo.argnull[0] = false;
-	locfcinfo.argnull[1] = false;
-	locfcinfo.argnull[2] = false;
-
-	return array_map(&locfcinfo, my_extra->srctype, my_extra->desttype,
-					 &my_extra->amstate);
-}
-
-/*
- * array_length_coerce -- apply the element type's length-coercion routine
- *		to each element of the given array.
- */
-Datum
-array_length_coerce(PG_FUNCTION_ARGS)
-{
-	ArrayType  *v = PG_GETARG_ARRAYTYPE_P(0);
-	int32		desttypmod = PG_GETARG_INT32(1);
-	bool		isExplicit = PG_GETARG_BOOL(2);
-	FmgrInfo   *fmgr_info = fcinfo->flinfo;
-	typedef struct
-	{
-		Oid			elemtype;
-		FmgrInfo	coerce_finfo;
-		ArrayMapState amstate;
-	} alc_extra;
-	alc_extra  *my_extra;
-	FunctionCallInfoData locfcinfo;
-
-	/* If no typmod is provided, shortcircuit the whole thing */
-	if (desttypmod < 0)
-		PG_RETURN_ARRAYTYPE_P(v);
-
-	/*
-	 * We arrange to look up the element type's coercion function only once
-	 * per series of calls, assuming the element type doesn't change
-	 * underneath us.
-	 */
-	my_extra = (alc_extra *) fmgr_info->fn_extra;
-	if (my_extra == NULL)
-	{
-		fmgr_info->fn_extra = MemoryContextAllocZero(fmgr_info->fn_mcxt,
-													 sizeof(alc_extra));
-		my_extra = (alc_extra *) fmgr_info->fn_extra;
-	}
-
-	if (my_extra->elemtype != ARR_ELEMTYPE(v))
-	{
-		Oid			funcId;
-
-		funcId = find_typmod_coercion_function(ARR_ELEMTYPE(v));
-
-		if (OidIsValid(funcId))
-			fmgr_info_cxt(funcId, &my_extra->coerce_finfo, fmgr_info->fn_mcxt);
-		else
-			my_extra->coerce_finfo.fn_oid = InvalidOid;
-		my_extra->elemtype = ARR_ELEMTYPE(v);
-	}
-
-	/*
-	 * If we didn't find a coercion function, return the array unmodified
-	 * (this should not happen in the normal course of things, but might
-	 * happen if this function is called manually).
-	 */
-	if (my_extra->coerce_finfo.fn_oid == InvalidOid)
-		PG_RETURN_ARRAYTYPE_P(v);
-
-	/*
-	 * Use array_map to apply the function to each array element.
-	 *
-	 * Note: we pass isExplicit whether or not the function wants it ...
-	 */
-	InitFunctionCallInfoData(locfcinfo, &my_extra->coerce_finfo, 3,
-							 NULL, NULL);
-	locfcinfo.arg[0] = PointerGetDatum(v);
-	locfcinfo.arg[1] = Int32GetDatum(desttypmod);
-	locfcinfo.arg[2] = BoolGetDatum(isExplicit);
-	locfcinfo.argnull[0] = false;
-	locfcinfo.argnull[1] = false;
-	locfcinfo.argnull[2] = false;
-
-	return array_map(&locfcinfo, ARR_ELEMTYPE(v), ARR_ELEMTYPE(v),
-					 &my_extra->amstate);
-}
-
-/*
  * accumArrayResult - accumulate one (more) Datum for an array result
  *
  *	astate is working state (NULL on first call)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index af363f4acff..b9a026c7ea1 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -15,7 +15,7 @@
  *
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.93 2007/03/25 19:45:14 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.94 2007/03/27 23:21:10 tgl Exp $
  *
  * ----------
  */
@@ -3871,6 +3871,7 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
 		Oid		lefttype,
 				righttype,
 				castfunc;
+		bool	arrayCoerce;
 
 		/* We always need to know how to call the equality operator */
 		fmgr_info_cxt(get_opcode(eq_opr), &entry->eq_opr_finfo,
@@ -3879,13 +3880,19 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
 		/*
 		 * If we chose to use a cast from FK to PK type, we may have to
 		 * apply the cast function to get to the operator's input type.
+		 *
+		 * XXX eventually it would be good to support array-coercion cases
+		 * here and in ri_AttributesEqual().  At the moment there is no
+		 * point because cases involving nonidentical array types will
+		 * be rejected at constraint creation time.
 		 */
 		op_input_types(eq_opr, &lefttype, &righttype);
 		Assert(lefttype == righttype);
 		if (typeid == lefttype)
 			castfunc = InvalidOid;				/* simplest case */
 		else if (!find_coercion_pathway(lefttype, typeid, COERCION_IMPLICIT,
-										&castfunc))
+										&castfunc, &arrayCoerce)
+				 || arrayCoerce)				/* XXX fixme */
 		{
 			/* If target is ANYARRAY, assume it's OK, else punt. */
 			if (lefttype != ANYARRAYOID)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index da5ab61e84b..46cf1dd45af 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.256 2007/03/18 16:50:42 neilc Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.257 2007/03/27 23:21:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3123,6 +3123,9 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags)
 		case T_RelabelType:
 			return isSimpleNode((Node *) ((RelabelType *) node)->arg,
 								node, prettyFlags);
+		case T_ArrayCoerceExpr:
+			return isSimpleNode((Node *) ((ArrayCoerceExpr *) node)->arg,
+								node, prettyFlags);
 		case T_ConvertRowtypeExpr:
 			return isSimpleNode((Node *) ((ConvertRowtypeExpr *) node)->arg,
 								node, prettyFlags);
@@ -3588,6 +3591,27 @@ get_rule_expr(Node *node, deparse_context *context,
 			}
 			break;
 
+		case T_ArrayCoerceExpr:
+			{
+				ArrayCoerceExpr *acoerce = (ArrayCoerceExpr *) node;
+				Node	   *arg = (Node *) acoerce->arg;
+
+				if (acoerce->coerceformat == COERCE_IMPLICIT_CAST &&
+					!showimplicit)
+				{
+					/* don't show the implicit cast */
+					get_rule_expr_paren(arg, context, false, node);
+				}
+				else
+				{
+					get_coercion_expr(arg, context,
+									  acoerce->resulttype,
+									  acoerce->resulttypmod,
+									  node);
+				}
+			}
+			break;
+
 		case T_ConvertRowtypeExpr:
 			{
 				ConvertRowtypeExpr *convert = (ConvertRowtypeExpr *) node;
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index a92317aeac1..f596220d5a4 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.230 2007/03/21 22:18:12 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.231 2007/03/27 23:21:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1454,53 +1454,25 @@ nulltestsel(PlannerInfo *root, NullTestType nulltesttype,
 /*
  * strip_array_coercion - strip binary-compatible relabeling from an array expr
  *
- * For array values, the parser doesn't generate simple RelabelType nodes,
- * but function calls of array_type_coerce() or array_type_length_coerce().
- * If we want to cope with binary-compatible situations we have to look
- * through these calls whenever the element-type coercion is binary-compatible.
+ * For array values, the parser normally generates ArrayCoerceExpr conversions,
+ * but it seems possible that RelabelType might show up.  Also, the planner
+ * is not currently tense about collapsing stacked ArrayCoerceExpr nodes,
+ * so we need to be ready to deal with more than one level.
  */
 static Node *
 strip_array_coercion(Node *node)
 {
-	/* could be more than one level, so loop */
 	for (;;)
 	{
-		if (node && IsA(node, RelabelType))
+		if (node && IsA(node, ArrayCoerceExpr) &&
+			((ArrayCoerceExpr *) node)->elemfuncid == InvalidOid)
 		{
-			/* We don't really expect this case, but may as well cope */
-			node = (Node *) ((RelabelType *) node)->arg;
+			node = (Node *) ((ArrayCoerceExpr *) node)->arg;
 		}
-		else if (node && IsA(node, FuncExpr))
+		else if (node && IsA(node, RelabelType))
 		{
-			FuncExpr   *fexpr = (FuncExpr *) node;
-			Node	   *arg1;
-			Oid			src_elem_type;
-			Oid			tgt_elem_type;
-			Oid			funcId;
-
-			/* must be the right function(s) */
-			if (!(fexpr->funcid == F_ARRAY_TYPE_COERCE ||
-				  fexpr->funcid == F_ARRAY_TYPE_LENGTH_COERCE))
-				break;
-
-			/* fetch source and destination array element types */
-			arg1 = (Node *) linitial(fexpr->args);
-			src_elem_type = get_element_type(exprType(arg1));
-			if (src_elem_type == InvalidOid)
-				break;			/* probably shouldn't happen */
-			tgt_elem_type = get_element_type(fexpr->funcresulttype);
-			if (tgt_elem_type == InvalidOid)
-				break;			/* probably shouldn't happen */
-
-			/* find out how to coerce */
-			if (!find_coercion_pathway(tgt_elem_type, src_elem_type,
-									   COERCION_EXPLICIT, &funcId))
-				break;			/* definitely shouldn't happen */
-
-			if (OidIsValid(funcId))
-				break;			/* non-binary-compatible coercion */
-
-			node = arg1;		/* OK to look through the node */
+			/* We don't really expect this case, but may as well cope */
+			node = (Node *) ((RelabelType *) node)->arg;
 		}
 		else
 			break;
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index c36d9fee134..a28acdc2ad8 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.104 2007/02/09 03:35:34 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.105 2007/03/27 23:21:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2061,6 +2061,8 @@ get_call_expr_argtype(Node *expr, int argnum)
 		args = ((DistinctExpr *) expr)->args;
 	else if (IsA(expr, ScalarArrayOpExpr))
 		args = ((ScalarArrayOpExpr *) expr)->args;
+	else if (IsA(expr, ArrayCoerceExpr))
+		args = list_make1(((ArrayCoerceExpr *) expr)->arg);
 	else if (IsA(expr, NullIfExpr))
 		args = ((NullIfExpr *) expr)->args;
 	else
@@ -2072,12 +2074,16 @@ get_call_expr_argtype(Node *expr, int argnum)
 	argtype = exprType((Node *) list_nth(args, argnum));
 
 	/*
-	 * special hack for ScalarArrayOpExpr: what the underlying function will
-	 * actually get passed is the element type of the array.
+	 * special hack for ScalarArrayOpExpr and ArrayCoerceExpr: what the
+	 * underlying function will actually get passed is the element type of
+	 * the array.
 	 */
 	if (IsA(expr, ScalarArrayOpExpr) &&
 		argnum == 1)
 		argtype = get_element_type(argtype);
+	else if (IsA(expr, ArrayCoerceExpr) &&
+			 argnum == 0)
+		argtype = get_element_type(argtype);
 
 	return argtype;
 }