Support arrays over domains.

Allowing arrays with a domain type as their element type was left un-done
in the original domain patch, but not for any very good reason.  This
omission leads to such surprising results as array_agg() not working on
a domain column, because the parser can't identify a suitable output type
for the polymorphic aggregate.

In order to fix this, first clean up the APIs of coerce_to_domain() and
some internal functions in parse_coerce.c so that we consistently pass
around a CoercionContext along with CoercionForm.  Previously, we sometimes
passed an "isExplicit" boolean flag instead, which is strictly less
information; and coerce_to_domain() didn't even get that, but instead had
to reverse-engineer isExplicit from CoercionForm.  That's contrary to the
documentation in primnodes.h that says that CoercionForm only affects
display and not semantics.  I don't think this change fixes any live bugs,
but it makes things more consistent.  The main reason for doing it though
is that now build_coercion_expression() receives ccontext, which it needs
in order to be able to recursively invoke coerce_to_target_type().

Next, reimplement ArrayCoerceExpr so that the node does not directly know
any details of what has to be done to the individual array elements while
performing the array coercion.  Instead, the per-element processing is
represented by a sub-expression whose input is a source array element and
whose output is a target array element.  This simplifies life in
parse_coerce.c, because it can build that sub-expression by a recursive
invocation of coerce_to_target_type().  The executor now handles the
per-element processing as a compiled expression instead of hard-wired code.
The main advantage of this is that we can use a single ArrayCoerceExpr to
handle as many as three successive steps per element: base type conversion,
typmod coercion, and domain constraint checking.  The old code used two
stacked ArrayCoerceExprs to handle type + typmod coercion, which was pretty
inefficient, and adding yet another array deconstruction to do domain
constraint checking seemed very unappetizing.

In the case where we just need a single, very simple coercion function,
doing this straightforwardly leads to a noticeable increase in the
per-array-element runtime cost.  Hence, add an additional shortcut evalfunc
in execExprInterp.c that skips unnecessary overhead for that specific form
of expression.  The runtime speed of simple cases is within 1% or so of
where it was before, while cases that previously required two levels of
array processing are significantly faster.

Finally, create an implicit array type for every domain type, as we do for
base types, enums, etc.  Everything except the array-coercion case seems
to just work without further effort.

Tom Lane, reviewed by Andrew Dunstan

Discussion: https://postgr.es/m/9852.1499791473@sss.pgh.pa.us

Author: tglsfdc

contrib/pg_stat_statements/pg_stat_statements.c

@@ -2630,6 +2630,7 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)
 
 				APP_JUMB(acexpr->resulttype);
 				JumbleExpr(jstate, (Node *) acexpr->arg);
+				JumbleExpr(jstate, (Node *) acexpr->elemexpr);
 			}
 			break;
 		case T_ConvertRowtypeExpr:

doc/src/sgml/array.sgml

@@ -10,9 +10,8 @@
  <para>
   <productname>PostgreSQL</productname> allows columns of a table to be
   defined as variable-length multidimensional arrays. Arrays of any
-  built-in or user-defined base type, enum type, or composite type
-  can be created.
-  Arrays of domains are not yet supported.
+  built-in or user-defined base type, enum type, composite type, range type,
+  or domain can be created.
  </para>
 
  <sect2 id="arrays-declaration">

src/backend/catalog/dependency.c

@@ -1738,11 +1738,14 @@ find_expr_references_walker(Node *node,
 	{
 		ArrayCoerceExpr *acoerce = (ArrayCoerceExpr *) node;
 
-		if (OidIsValid(acoerce->elemfuncid))
-			add_object_address(OCLASS_PROC, acoerce->elemfuncid, 0,
-							   context->addrs);
+		/* as above, depend on type */
 		add_object_address(OCLASS_TYPE, acoerce->resulttype, 0,
 						   context->addrs);
+		/* the collation might not be referenced anywhere else, either */
+		if (OidIsValid(acoerce->resultcollid) &&
+			acoerce->resultcollid != DEFAULT_COLLATION_OID)
+			add_object_address(OCLASS_COLLATION, acoerce->resultcollid, 0,
+							   context->addrs);
 		/* fall through to examine arguments */
 	}
 	else if (IsA(node, ConvertRowtypeExpr))

src/backend/commands/typecmds.c

@@ -729,6 +729,7 @@ ObjectAddress
 DefineDomain(CreateDomainStmt *stmt)
 {
 	char	   *domainName;
+	char	   *domainArrayName;
 	Oid			domainNamespace;
 	AclResult	aclresult;
 	int16		internalLength;
@@ -757,6 +758,7 @@ DefineDomain(CreateDomainStmt *stmt)
 	Oid			basetypeoid;
 	Oid			old_type_oid;
 	Oid			domaincoll;
+	Oid			domainArrayOid;
 	Form_pg_type baseType;
 	int32		basetypeMod;
 	Oid			baseColl;
@@ -1027,6 +1029,9 @@ DefineDomain(CreateDomainStmt *stmt)
 		}
 	}
 
+	/* Allocate OID for array type */
+	domainArrayOid = AssignTypeArrayOid();
+
 	/*
 	 * Have TypeCreate do all the real work.
 	 */
@@ -1051,7 +1056,7 @@ DefineDomain(CreateDomainStmt *stmt)
 				   analyzeProcedure,	/* analyze procedure */
 				   InvalidOid,	/* no array element type */
 				   false,		/* this isn't an array */
-				   InvalidOid,	/* no arrays for domains (yet) */
+				   domainArrayOid,	/* array type we are about to create */
 				   basetypeoid, /* base type ID */
 				   defaultValue,	/* default type value (text) */
 				   defaultValueBin, /* default type value (binary) */
@@ -1063,6 +1068,48 @@ DefineDomain(CreateDomainStmt *stmt)
 				   typNotNull,	/* Type NOT NULL */
 				   domaincoll); /* type's collation */
 
+	/*
+	 * Create the array type that goes with it.
+	 */
+	domainArrayName = makeArrayTypeName(domainName, domainNamespace);
+
+	/* alignment must be 'i' or 'd' for arrays */
+	alignment = (alignment == 'd') ? 'd' : 'i';
+
+	TypeCreate(domainArrayOid,	/* force assignment of this type OID */
+			   domainArrayName, /* type name */
+			   domainNamespace, /* namespace */
+			   InvalidOid,		/* relation oid (n/a here) */
+			   0,				/* relation kind (ditto) */
+			   GetUserId(),		/* owner's ID */
+			   -1,				/* internal size (always varlena) */
+			   TYPTYPE_BASE,	/* type-type (base type) */
+			   TYPCATEGORY_ARRAY,	/* type-category (array) */
+			   false,			/* array types are never preferred */
+			   delimiter,		/* array element delimiter */
+			   F_ARRAY_IN,		/* input procedure */
+			   F_ARRAY_OUT,		/* output procedure */
+			   F_ARRAY_RECV,	/* receive procedure */
+			   F_ARRAY_SEND,	/* send procedure */
+			   InvalidOid,		/* typmodin procedure - none */
+			   InvalidOid,		/* typmodout procedure - none */
+			   F_ARRAY_TYPANALYZE,	/* analyze procedure */
+			   address.objectId,	/* element type ID */
+			   true,			/* yes this is an array type */
+			   InvalidOid,		/* no further array type */
+			   InvalidOid,		/* base type ID */
+			   NULL,			/* never a default type value */
+			   NULL,			/* binary default isn't sent either */
+			   false,			/* never passed by value */
+			   alignment,		/* see above */
+			   'x',				/* ARRAY is always toastable */
+			   -1,				/* typMod (Domains only) */
+			   0,				/* Array dimensions of typbasetype */
+			   false,			/* Type NOT NULL */
+			   domaincoll);		/* type's collation */
+
+	pfree(domainArrayName);
+
 	/*
 	 * Process constraints which refer to the domain ID returned by TypeCreate
 	 */
@@ -1139,6 +1186,7 @@ DefineEnum(CreateEnumStmt *stmt)
 					 errmsg("type \"%s\" already exists", enumName)));
 	}
 
+	/* Allocate OID for array type */
 	enumArrayOid = AssignTypeArrayOid();
 
 	/* Create the pg_type entry */

src/backend/executor/execExpr.c

@@ -1225,6 +1225,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state,
 			{
 				ArrayCoerceExpr *acoerce = (ArrayCoerceExpr *) node;
 				Oid			resultelemtype;
+				ExprState  *elemstate;
 
 				/* evaluate argument into step's result area */
 				ExecInitExprRec(acoerce->arg, parent, state, resv, resnull);
@@ -1234,42 +1235,49 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state,
 					ereport(ERROR,
 							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 							 errmsg("target type is not an array")));
-				/* Arrays over domains aren't supported yet */
-				Assert(getBaseType(resultelemtype) == resultelemtype);
 
-				scratch.opcode = EEOP_ARRAYCOERCE;
-				scratch.d.arraycoerce.coerceexpr = acoerce;
-				scratch.d.arraycoerce.resultelemtype = resultelemtype;
+				/*
+				 * Construct a sub-expression for the per-element expression;
+				 * but don't ready it until after we check it for triviality.
+				 * We assume it hasn't any Var references, but does have a
+				 * CaseTestExpr representing the source array element values.
+				 */
+				elemstate = makeNode(ExprState);
+				elemstate->expr = acoerce->elemexpr;
+				elemstate->innermost_caseval = (Datum *) palloc(sizeof(Datum));
+				elemstate->innermost_casenull = (bool *) palloc(sizeof(bool));
 
-				if (OidIsValid(acoerce->elemfuncid))
-				{
-					AclResult	aclresult;
+				ExecInitExprRec(acoerce->elemexpr, parent, elemstate,
+								&elemstate->resvalue, &elemstate->resnull);
 
-					/* Check permission to call function */
-					aclresult = pg_proc_aclcheck(acoerce->elemfuncid,
-												 GetUserId(),
-												 ACL_EXECUTE);
-					if (aclresult != ACLCHECK_OK)
-						aclcheck_error(aclresult, ACL_KIND_PROC,
-									   get_func_name(acoerce->elemfuncid));
-					InvokeFunctionExecuteHook(acoerce->elemfuncid);
+				if (elemstate->steps_len == 1 &&
+					elemstate->steps[0].opcode == EEOP_CASE_TESTVAL)
+				{
+					/* Trivial, so we need no per-element work at runtime */
+					elemstate = NULL;
+				}
+				else
+				{
+					/* Not trivial, so append a DONE step */
+					scratch.opcode = EEOP_DONE;
+					ExprEvalPushStep(elemstate, &scratch);
+					/* and ready the subexpression */
+					ExecReadyExpr(elemstate);
+				}
 
-					/* Set up the primary fmgr lookup information */
-					scratch.d.arraycoerce.elemfunc =
-						(FmgrInfo *) palloc0(sizeof(FmgrInfo));
-					fmgr_info(acoerce->elemfuncid,
-							  scratch.d.arraycoerce.elemfunc);
-					fmgr_info_set_expr((Node *) acoerce,
-									   scratch.d.arraycoerce.elemfunc);
+				scratch.opcode = EEOP_ARRAYCOERCE;
+				scratch.d.arraycoerce.elemexprstate = elemstate;
+				scratch.d.arraycoerce.resultelemtype = resultelemtype;
 
+				if (elemstate)
+				{
 					/* Set up workspace for array_map */
 					scratch.d.arraycoerce.amstate =
 						(ArrayMapState *) palloc0(sizeof(ArrayMapState));
 				}
 				else
 				{
-					/* Don't need workspace if there's no conversion func */
-					scratch.d.arraycoerce.elemfunc = NULL;
+					/* Don't need workspace if there's no subexpression */
 					scratch.d.arraycoerce.amstate = NULL;
 				}
 

src/backend/executor/execExprInterp.c

@@ -34,10 +34,8 @@
  *
  * For very simple instructions the overhead of the full interpreter
  * "startup", as minimal as it is, is noticeable.  Therefore
- * ExecReadyInterpretedExpr will choose to implement simple scalar Var
- * and Const expressions using special fast-path routines (ExecJust*).
- * Benchmarking shows anything more complex than those may as well use the
- * "full interpreter".
+ * ExecReadyInterpretedExpr will choose to implement certain simple
+ * opcode patterns using special fast-path routines (ExecJust*).
  *
  * Complex or uncommon instructions are not implemented in-line in
  * ExecInterpExpr(), rather we call out to a helper function appearing later
@@ -149,6 +147,7 @@ static Datum ExecJustConst(ExprState *state, ExprContext *econtext, bool *isnull
 static Datum ExecJustAssignInnerVar(ExprState *state, ExprContext *econtext, bool *isnull);
 static Datum ExecJustAssignOuterVar(ExprState *state, ExprContext *econtext, bool *isnull);
 static Datum ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull);
+static Datum ExecJustApplyFuncToCase(ExprState *state, ExprContext *econtext, bool *isnull);
 
 
 /*
@@ -184,10 +183,8 @@ ExecReadyInterpretedExpr(ExprState *state)
 
 	/*
 	 * Select fast-path evalfuncs for very simple expressions.  "Starting up"
-	 * the full interpreter is a measurable overhead for these.  Plain Vars
-	 * and Const seem to be the only ones where the intrinsic cost is small
-	 * enough that the overhead of ExecInterpExpr matters.  For more complex
-	 * expressions it's cheaper to use ExecInterpExpr always.
+	 * the full interpreter is a measurable overhead for these, and these
+	 * patterns occur often enough to be worth optimizing.
 	 */
 	if (state->steps_len == 3)
 	{
@@ -230,6 +227,13 @@ ExecReadyInterpretedExpr(ExprState *state)
 			state->evalfunc = ExecJustAssignScanVar;
 			return;
 		}
+		else if (step0 == EEOP_CASE_TESTVAL &&
+				 step1 == EEOP_FUNCEXPR_STRICT &&
+				 state->steps[0].d.casetest.value)
+		{
+			state->evalfunc = ExecJustApplyFuncToCase;
+			return;
+		}
 	}
 	else if (state->steps_len == 2 &&
 			 state->steps[0].opcode == EEOP_CONST)
@@ -1275,7 +1279,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 		EEO_CASE(EEOP_ARRAYCOERCE)
 		{
 			/* too complex for an inline implementation */
-			ExecEvalArrayCoerce(state, op);
+			ExecEvalArrayCoerce(state, op, econtext);
 
 			EEO_NEXT();
 		}
@@ -1811,6 +1815,43 @@ ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull)
 	return 0;
 }
 
+/* Evaluate CASE_TESTVAL and apply a strict function to it */
+static Datum
+ExecJustApplyFuncToCase(ExprState *state, ExprContext *econtext, bool *isnull)
+{
+	ExprEvalStep *op = &state->steps[0];
+	FunctionCallInfo fcinfo;
+	bool	   *argnull;
+	int			argno;
+	Datum		d;
+
+	/*
+	 * XXX with some redesign of the CaseTestExpr mechanism, maybe we could
+	 * get rid of this data shuffling?
+	 */
+	*op->resvalue = *op->d.casetest.value;
+	*op->resnull = *op->d.casetest.isnull;
+
+	op++;
+
+	fcinfo = op->d.func.fcinfo_data;
+	argnull = fcinfo->argnull;
+
+	/* strict function, so check for NULL args */
+	for (argno = 0; argno < op->d.func.nargs; argno++)
+	{
+		if (argnull[argno])
+		{
+			*isnull = true;
+			return (Datum) 0;
+		}
+	}
+	fcinfo->isnull = false;
+	d = op->d.func.fn_addr(fcinfo);
+	*isnull = fcinfo->isnull;
+	return d;
+}
+
 
 /*
  * Do one-time initialization of interpretation machinery.
@@ -2345,11 +2386,9 @@ ExecEvalArrayExpr(ExprState *state, ExprEvalStep *op)
  * Source array is in step's result variable.
  */
 void
-ExecEvalArrayCoerce(ExprState *state, ExprEvalStep *op)
+ExecEvalArrayCoerce(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
 {
-	ArrayCoerceExpr *acoerce = op->d.arraycoerce.coerceexpr;
 	Datum		arraydatum;
-	FunctionCallInfoData locfcinfo;
 
 	/* NULL array -> NULL result */
 	if (*op->resnull)
@@ -2361,7 +2400,7 @@ ExecEvalArrayCoerce(ExprState *state, ExprEvalStep *op)
 	 * If it's binary-compatible, modify the element type in the array header,
 	 * but otherwise leave the array as we received it.
 	 */
-	if (!OidIsValid(acoerce->elemfuncid))
+	if (op->d.arraycoerce.elemexprstate == NULL)
 	{
 		/* Detoast input array if necessary, and copy in any case */
 		ArrayType  *array = DatumGetArrayTypePCopy(arraydatum);
@@ -2372,23 +2411,12 @@ ExecEvalArrayCoerce(ExprState *state, ExprEvalStep *op)
 	}
 
 	/*
-	 * 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.
-	 *
-	 * Note: coercion functions are assumed to not use collation.
+	 * Use array_map to apply the sub-expression to each array element.
 	 */
-	InitFunctionCallInfoData(locfcinfo, op->d.arraycoerce.elemfunc, 3,
-							 InvalidOid, NULL, NULL);
-	locfcinfo.arg[0] = arraydatum;
-	locfcinfo.arg[1] = Int32GetDatum(acoerce->resulttypmod);
-	locfcinfo.arg[2] = BoolGetDatum(acoerce->isExplicit);
-	locfcinfo.argnull[0] = false;
-	locfcinfo.argnull[1] = false;
-	locfcinfo.argnull[2] = false;
-
-	*op->resvalue = array_map(&locfcinfo, op->d.arraycoerce.resultelemtype,
+	*op->resvalue = array_map(arraydatum,
+							  op->d.arraycoerce.elemexprstate,
+							  econtext,
+							  op->d.arraycoerce.resultelemtype,
 							  op->d.arraycoerce.amstate);
 }
 

src/backend/nodes/copyfuncs.c

@@ -1698,11 +1698,10 @@ _copyArrayCoerceExpr(const ArrayCoerceExpr *from)
 	ArrayCoerceExpr *newnode = makeNode(ArrayCoerceExpr);
 
 	COPY_NODE_FIELD(arg);
-	COPY_SCALAR_FIELD(elemfuncid);
+	COPY_NODE_FIELD(elemexpr);
 	COPY_SCALAR_FIELD(resulttype);
 	COPY_SCALAR_FIELD(resulttypmod);
 	COPY_SCALAR_FIELD(resultcollid);
-	COPY_SCALAR_FIELD(isExplicit);
 	COPY_SCALAR_FIELD(coerceformat);
 	COPY_LOCATION_FIELD(location);
 

src/backend/nodes/equalfuncs.c

@@ -513,11 +513,10 @@ static bool
 _equalArrayCoerceExpr(const ArrayCoerceExpr *a, const ArrayCoerceExpr *b)
 {
 	COMPARE_NODE_FIELD(arg);
-	COMPARE_SCALAR_FIELD(elemfuncid);
+	COMPARE_NODE_FIELD(elemexpr);
 	COMPARE_SCALAR_FIELD(resulttype);
 	COMPARE_SCALAR_FIELD(resulttypmod);
 	COMPARE_SCALAR_FIELD(resultcollid);
-	COMPARE_SCALAR_FIELD(isExplicit);
 	COMPARE_COERCIONFORM_FIELD(coerceformat);
 	COMPARE_LOCATION_FIELD(location);