Rationalize the APIs of array element/slice access functions.

The four functions array_ref, array_set, array_get_slice, array_set_slice
have traditionally declared their array inputs and results as being of type
"ArrayType *".  This is a lie, and has been since Berkeley days, because
they actually also support "fixed-length array" types such as "name" and
"point"; not to mention that the inputs could be toasted.  These values
should be declared Datum instead to avoid confusion.  The current coding
already risks possible misoptimization by compilers, and it'll get worse
when "expanded" array representations become a valid alternative.

However, there's a fair amount of code using array_ref and array_set with
arrays that *are* known to be ArrayType structures, and there might be more
such places in third-party code.  Rather than cluttering those call sites
with PointerGetDatum/DatumGetArrayTypeP cruft, what I did was to rename the
existing functions to array_get_element/array_set_element, fix their
signatures, then reincarnate array_ref/array_set as backwards compatibility
wrappers.

array_get_slice/array_set_slice have no such constituency in the core code,
and probably not in third-party code either, so I just changed their APIs.

src/backend/executor/execQual.c

@@ -252,12 +252,6 @@ static Datum ExecEvalCurrentOfExpr(ExprState *exprstate, ExprContext *econtext,
  *
  * NOTE: if we get a NULL result from a subscript expression, we return NULL
  * when it's an array reference, or raise an error when it's an assignment.
- *
- * NOTE: we deliberately refrain from applying DatumGetArrayTypeP() here,
- * even though that might seem natural, because this code needs to support
- * both varlena arrays and fixed-length array types.  DatumGetArrayTypeP()
- * only works for the varlena kind.  The routines we call in arrayfuncs.c
- * have to know the difference (that's what they need refattrlength for).
  *----------
  */
 static Datum
@@ -267,8 +261,7 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 				 ExprDoneCond *isDone)
 {
 	ArrayRef   *arrayRef = (ArrayRef *) astate->xprstate.expr;
-	ArrayType  *array_source;
-	ArrayType  *resultArray;
+	Datum		array_source;
 	bool		isAssignment = (arrayRef->refassgnexpr != NULL);
 	bool		eisnull;
 	ListCell   *l;
@@ -278,11 +271,10 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 				lower;
 	int		   *lIndex;
 
-	array_source = (ArrayType *)
-		DatumGetPointer(ExecEvalExpr(astate->refexpr,
+	array_source = ExecEvalExpr(astate->refexpr,
 								econtext,
 								isNull,
-									 isDone));
+								isDone);
 
 	/*
 	 * If refexpr yields NULL, and it's a fetch, then result is NULL. In the
@@ -390,7 +382,8 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 			}
 			else if (lIndex == NULL)
 			{
-				econtext->caseValue_datum = array_ref(array_source, i,
+				econtext->caseValue_datum =
+					array_get_element(array_source, i,
 									  upper.indx,
 									  astate->refattrlength,
 									  astate->refelemlength,
@@ -400,13 +393,13 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 			}
 			else
 			{
-				resultArray = array_get_slice(array_source, i,
+				econtext->caseValue_datum =
+					array_get_slice(array_source, i,
 									upper.indx, lower.indx,
 									astate->refattrlength,
 									astate->refelemlength,
 									astate->refelembyval,
 									astate->refelemalign);
-				econtext->caseValue_datum = PointerGetDatum(resultArray);
 				econtext->caseValue_isNull = false;
 			}
 		}
@@ -435,7 +428,7 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 		 */
 		if (astate->refattrlength > 0)	/* fixed-length array? */
 			if (eisnull || *isNull)
-				return PointerGetDatum(array_source);
+				return array_source;
 
 		/*
 		 * For assignment to varlena arrays, we handle a NULL original array
@@ -445,12 +438,12 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 		 */
 		if (*isNull)
 		{
-			array_source = construct_empty_array(arrayRef->refelemtype);
+			array_source = PointerGetDatum(construct_empty_array(arrayRef->refelemtype));
 			*isNull = false;
 		}
 
 		if (lIndex == NULL)
-			resultArray = array_set(array_source, i,
+			return array_set_element(array_source, i,
 									 upper.indx,
 									 sourceData,
 									 eisnull,
@@ -459,34 +452,31 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 									 astate->refelembyval,
 									 astate->refelemalign);
 		else
-			resultArray = array_set_slice(array_source, i,
+			return array_set_slice(array_source, i,
 								   upper.indx, lower.indx,
-								   (ArrayType *) DatumGetPointer(sourceData),
+								   sourceData,
 								   eisnull,
 								   astate->refattrlength,
 								   astate->refelemlength,
 								   astate->refelembyval,
 								   astate->refelemalign);
-		return PointerGetDatum(resultArray);
 	}
 
 	if (lIndex == NULL)
-		return array_ref(array_source, i, upper.indx,
+		return array_get_element(array_source, i,
+								 upper.indx,
 								 astate->refattrlength,
 								 astate->refelemlength,
 								 astate->refelembyval,
 								 astate->refelemalign,
 								 isNull);
 	else
-	{
-		resultArray = array_get_slice(array_source, i,
+		return array_get_slice(array_source, i,
 							   upper.indx, lower.indx,
 							   astate->refattrlength,
 							   astate->refelemlength,
 							   astate->refelembyval,
 							   astate->refelemalign);
-		return PointerGetDatum(resultArray);
-	}
 }
 
 /*

src/backend/rewrite/rewriteHandler.c

@@ -664,7 +664,7 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index)
  *			UPDATE table SET foo[2] = 42, foo[4] = 43;
  * We can merge such operations into a single assignment op.  Essentially,
  * the expression we want to produce in this case is like
- *		foo = array_set(array_set(foo, 2, 42), 4, 43)
+ *		foo = array_set_element(array_set_element(foo, 2, 42), 4, 43)
  *
  * 4. Sort the tlist into standard order: non-junk fields in order by resno,
  * then junk fields (these in no particular order).

src/include/utils/array.h

@@ -248,18 +248,25 @@ extern Datum array_remove(PG_FUNCTION_ARGS);
 extern Datum array_replace(PG_FUNCTION_ARGS);
 extern Datum width_bucket_array(PG_FUNCTION_ARGS);
 
-extern Datum array_ref(ArrayType *array, int nSubscripts, int *indx,
+extern Datum array_get_element(Datum arraydatum, int nSubscripts, int *indx,
 				  int arraytyplen, int elmlen, bool elmbyval, char elmalign,
 				  bool *isNull);
-extern ArrayType *array_set(ArrayType *array, int nSubscripts, int *indx,
+extern Datum array_set_element(Datum arraydatum, int nSubscripts, int *indx,
 				  Datum dataValue, bool isNull,
 				  int arraytyplen, int elmlen, bool elmbyval, char elmalign);
-extern ArrayType *array_get_slice(ArrayType *array, int nSubscripts,
+extern Datum array_get_slice(Datum arraydatum, int nSubscripts,
 				int *upperIndx, int *lowerIndx,
 				int arraytyplen, int elmlen, bool elmbyval, char elmalign);
-extern ArrayType *array_set_slice(ArrayType *array, int nSubscripts,
+extern Datum array_set_slice(Datum arraydatum, int nSubscripts,
 				int *upperIndx, int *lowerIndx,
-				ArrayType *srcArray, bool isNull,
+				Datum srcArrayDatum, bool isNull,
+				int arraytyplen, int elmlen, bool elmbyval, char elmalign);
+
+extern Datum array_ref(ArrayType *array, int nSubscripts, int *indx,
+		  int arraytyplen, int elmlen, bool elmbyval, char elmalign,
+		  bool *isNull);
+extern ArrayType *array_set(ArrayType *array, int nSubscripts, int *indx,
+		  Datum dataValue, bool isNull,
 		  int arraytyplen, int elmlen, bool elmbyval, char elmalign);
 
 extern Datum array_map(FunctionCallInfo fcinfo, Oid inpType, Oid retType,

src/pl/plpgsql/src/pl_exec.c

@@ -4233,12 +4233,11 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				PLpgSQL_expr *subscripts[MAXDIM];
 				int			subscriptvals[MAXDIM];
 				Datum		oldarraydatum,
+							newarraydatum,
 							coerced_value;
 				bool		oldarrayisnull;
 				Oid			parenttypoid;
 				int32		parenttypmod;
-				ArrayType  *oldarrayval;
-				ArrayType  *newarrayval;
 				SPITupleTable *save_eval_tuptable;
 				MemoryContext oldcontext;
 
@@ -4378,18 +4377,16 @@ exec_assign_value(PLpgSQL_execstate *estate,
 					(oldarrayisnull || *isNull))
 					return;
 
-				/* oldarrayval and newarrayval should be short-lived */
+				/* empty array, if any, and newarraydatum are short-lived */
 				oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
 
 				if (oldarrayisnull)
-					oldarrayval = construct_empty_array(arrayelem->elemtypoid);
-				else
-					oldarrayval = (ArrayType *) DatumGetPointer(oldarraydatum);
+					oldarraydatum = PointerGetDatum(construct_empty_array(arrayelem->elemtypoid));
 
 				/*
 				 * Build the modified array value.
 				 */
-				newarrayval = array_set(oldarrayval,
+				newarraydatum = array_set_element(oldarraydatum,
 												  nsubscripts,
 												  subscriptvals,
 												  coerced_value,
@@ -4409,7 +4406,7 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				 */
 				*isNull = false;
 				exec_assign_value(estate, target,
-								  PointerGetDatum(newarrayval),
+								  newarraydatum,
 								  arrayelem->arraytypoid, isNull);
 				break;
 			}