In plpgsql, unify duplicate variables for record and row cases.

plpgsql's function exec_move_row() handles assignment of a composite
source value to either a PLpgSQL_rec or PLpgSQL_row target variable.
Oddly, rather than taking a single target argument which it could do
run-time type detection on, it was coded to take two separate arguments
(only one of which is allowed to be non-NULL).  This choice had then
back-propagated into storing two separate target variables in various
plpgsql statement nodes, with lots of duplicative coding and awkward
interface logic to support that.  Simplify matters by folding those
pairs down to single variables, distinguishing the two cases only
where we must ... which turns out to be only in exec_move_row itself.
This is purely refactoring and should not change any behavior.

In passing, remove unused field PLpgSQL_stmt_open.returntype.

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

src/pl/plpgsql/src/pl_comp.c

@@ -545,7 +545,7 @@ do_compile(FunctionCallInfo fcinfo,
 				{
 					if (rettypeid == VOIDOID ||
 						rettypeid == RECORDOID)
-						/* okay */ ;
+						 /* okay */ ;
 					else if (rettypeid == TRIGGEROID || rettypeid == EVTTRIGGEROID)
 						ereport(ERROR,
 								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

src/pl/plpgsql/src/pl_funcs.c

@@ -1062,7 +1062,7 @@ static void
 dump_fors(PLpgSQL_stmt_fors *stmt)
 {
 	dump_ind();
-	printf("FORS %s ", (stmt->rec != NULL) ? stmt->rec->refname : stmt->row->refname);
+	printf("FORS %s ", stmt->var->refname);
 	dump_expr(stmt->query);
 	printf("\n");
 
@@ -1076,7 +1076,7 @@ static void
 dump_forc(PLpgSQL_stmt_forc *stmt)
 {
 	dump_ind();
-	printf("FORC %s ", stmt->rec->refname);
+	printf("FORC %s ", stmt->var->refname);
 	printf("curvar=%d\n", stmt->curvar);
 
 	dump_indent += 2;
@@ -1174,15 +1174,11 @@ dump_fetch(PLpgSQL_stmt_fetch *stmt)
 		dump_cursor_direction(stmt);
 
 		dump_indent += 2;
-		if (stmt->rec != NULL)
+		if (stmt->target != NULL)
 		{
 			dump_ind();
-			printf("    target = %d %s\n", stmt->rec->dno, stmt->rec->refname);
-		}
-		if (stmt->row != NULL)
-		{
-			dump_ind();
-			printf("    target = %d %s\n", stmt->row->dno, stmt->row->refname);
+			printf("    target = %d %s\n",
+				   stmt->target->dno, stmt->target->refname);
 		}
 		dump_indent -= 2;
 	}
@@ -1420,19 +1416,12 @@ dump_execsql(PLpgSQL_stmt_execsql *stmt)
 	printf("\n");
 
 	dump_indent += 2;
-	if (stmt->rec != NULL)
+	if (stmt->target != NULL)
 	{
 		dump_ind();
 		printf("    INTO%s target = %d %s\n",
 			   stmt->strict ? " STRICT" : "",
-			   stmt->rec->dno, stmt->rec->refname);
-	}
-	if (stmt->row != NULL)
-	{
-		dump_ind();
-		printf("    INTO%s target = %d %s\n",
-			   stmt->strict ? " STRICT" : "",
-			   stmt->row->dno, stmt->row->refname);
+			   stmt->target->dno, stmt->target->refname);
 	}
 	dump_indent -= 2;
 }
@@ -1446,19 +1435,12 @@ dump_dynexecute(PLpgSQL_stmt_dynexecute *stmt)
 	printf("\n");
 
 	dump_indent += 2;
-	if (stmt->rec != NULL)
-	{
-		dump_ind();
-		printf("    INTO%s target = %d %s\n",
-			   stmt->strict ? " STRICT" : "",
-			   stmt->rec->dno, stmt->rec->refname);
-	}
-	if (stmt->row != NULL)
+	if (stmt->target != NULL)
 	{
 		dump_ind();
 		printf("    INTO%s target = %d %s\n",
 			   stmt->strict ? " STRICT" : "",
-			   stmt->row->dno, stmt->row->refname);
+			   stmt->target->dno, stmt->target->refname);
 	}
 	if (stmt->params != NIL)
 	{
@@ -1485,8 +1467,7 @@ static void
 dump_dynfors(PLpgSQL_stmt_dynfors *stmt)
 {
 	dump_ind();
-	printf("FORS %s EXECUTE ",
-		   (stmt->rec != NULL) ? stmt->rec->refname : stmt->row->refname);
+	printf("FORS %s EXECUTE ", stmt->var->refname);
 	dump_expr(stmt->query);
 	printf("\n");
 	if (stmt->params != NIL)

src/pl/plpgsql/src/pl_gram.y

@@ -90,7 +90,7 @@ static  PLpgSQL_stmt	*make_case(int location, PLpgSQL_expr *t_expr,
 								   List *case_when_list, List *else_stmts);
 static	char			*NameOfDatum(PLwdatum *wdatum);
 static	void			 check_assignable(PLpgSQL_datum *datum, int location);
-static	void			 read_into_target(PLpgSQL_rec **rec, PLpgSQL_row **row,
+static	void			 read_into_target(PLpgSQL_variable **target,
 										  bool *strict);
 static	PLpgSQL_row		*read_into_scalar_list(char *initial_name,
 											   PLpgSQL_datum *initial_datum,
@@ -138,8 +138,7 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 			char *name;
 			int  lineno;
 			PLpgSQL_datum   *scalar;
-			PLpgSQL_rec		*rec;
-			PLpgSQL_row		*row;
+			PLpgSQL_datum   *row;
 		}						forvariable;
 		struct
 		{
@@ -1310,22 +1309,18 @@ for_control		: for_variable K_IN
 
 							new = palloc0(sizeof(PLpgSQL_stmt_dynfors));
 							new->cmd_type = PLPGSQL_STMT_DYNFORS;
-							if ($1.rec)
+							if ($1.row)
 							{
-								new->rec = $1.rec;
-								check_assignable((PLpgSQL_datum *) new->rec, @1);
-							}
-							else if ($1.row)
-							{
-								new->row = $1.row;
-								check_assignable((PLpgSQL_datum *) new->row, @1);
+								new->var = (PLpgSQL_variable *) $1.row;
+								check_assignable($1.row, @1);
 							}
 							else if ($1.scalar)
 							{
 								/* convert single scalar to list */
-								new->row = make_scalar_list1($1.name, $1.scalar,
-															 $1.lineno, @1);
-								/* no need for check_assignable */
+								new->var = (PLpgSQL_variable *)
+									make_scalar_list1($1.name, $1.scalar,
+													  $1.lineno, @1);
+								/* make_scalar_list1 did check_assignable */
 							}
 							else
 							{
@@ -1381,9 +1376,10 @@ for_control		: for_variable K_IN
 															 "LOOP");
 
 							/* create loop's private RECORD variable */
-							new->rec = plpgsql_build_record($1.name,
-															$1.lineno,
-															true);
+							new->var = (PLpgSQL_variable *)
+								plpgsql_build_record($1.name,
+													 $1.lineno,
+													 true);
 
 							$$ = (PLpgSQL_stmt *) new;
 						}
@@ -1504,22 +1500,18 @@ for_control		: for_variable K_IN
 
 								new = palloc0(sizeof(PLpgSQL_stmt_fors));
 								new->cmd_type = PLPGSQL_STMT_FORS;
-								if ($1.rec)
+								if ($1.row)
 								{
-									new->rec = $1.rec;
-									check_assignable((PLpgSQL_datum *) new->rec, @1);
-								}
-								else if ($1.row)
-								{
-									new->row = $1.row;
-									check_assignable((PLpgSQL_datum *) new->row, @1);
+									new->var = (PLpgSQL_variable *) $1.row;
+									check_assignable($1.row, @1);
 								}
 								else if ($1.scalar)
 								{
 									/* convert single scalar to list */
-									new->row = make_scalar_list1($1.name, $1.scalar,
-																 $1.lineno, @1);
-									/* no need for check_assignable */
+									new->var = (PLpgSQL_variable *)
+										make_scalar_list1($1.name, $1.scalar,
+														  $1.lineno, @1);
+									/* make_scalar_list1 did check_assignable */
 								}
 								else
 								{
@@ -1558,32 +1550,26 @@ for_variable	: T_DATUM
 					{
 						$$.name = NameOfDatum(&($1));
 						$$.lineno = plpgsql_location_to_lineno(@1);
-						if ($1.datum->dtype == PLPGSQL_DTYPE_ROW)
+						if ($1.datum->dtype == PLPGSQL_DTYPE_ROW ||
+							$1.datum->dtype == PLPGSQL_DTYPE_REC)
 						{
 							$$.scalar = NULL;
-							$$.rec = NULL;
-							$$.row = (PLpgSQL_row *) $1.datum;
-						}
-						else if ($1.datum->dtype == PLPGSQL_DTYPE_REC)
-						{
-							$$.scalar = NULL;
-							$$.rec = (PLpgSQL_rec *) $1.datum;
-							$$.row = NULL;
+							$$.row = $1.datum;
 						}
 						else
 						{
 							int			tok;
 
 							$$.scalar = $1.datum;
-							$$.rec = NULL;
 							$$.row = NULL;
 							/* check for comma-separated list */
 							tok = yylex();
 							plpgsql_push_back_token(tok);
 							if (tok == ',')
-								$$.row = read_into_scalar_list($$.name,
-															   $$.scalar,
-															   @1);
+								$$.row = (PLpgSQL_datum *)
+									read_into_scalar_list($$.name,
+														  $$.scalar,
+														  @1);
 						}
 					}
 				| T_WORD
@@ -1593,7 +1579,6 @@ for_variable	: T_DATUM
 						$$.name = $1.ident;
 						$$.lineno = plpgsql_location_to_lineno(@1);
 						$$.scalar = NULL;
-						$$.rec = NULL;
 						$$.row = NULL;
 						/* check for comma-separated list */
 						tok = yylex();
@@ -1620,15 +1605,10 @@ stmt_foreach_a	: opt_loop_label K_FOREACH for_variable foreach_slice K_IN K_ARRA
 						new->expr = $7;
 						new->body = $8.stmts;
 
-						if ($3.rec)
-						{
-							new->varno = $3.rec->dno;
-							check_assignable((PLpgSQL_datum *) $3.rec, @3);
-						}
-						else if ($3.row)
+						if ($3.row)
 						{
 							new->varno = $3.row->dno;
-							check_assignable((PLpgSQL_datum *) $3.row, @3);
+							check_assignable($3.row, @3);
 						}
 						else if ($3.scalar)
 						{
@@ -1981,8 +1961,7 @@ stmt_dynexecute : K_EXECUTE
 						new->query = expr;
 						new->into = false;
 						new->strict = false;
-						new->rec = NULL;
-						new->row = NULL;
+						new->target = NULL;
 						new->params = NIL;
 
 						/*
@@ -1999,7 +1978,7 @@ stmt_dynexecute : K_EXECUTE
 								if (new->into)			/* multiple INTO */
 									yyerror("syntax error");
 								new->into = true;
-								read_into_target(&new->rec, &new->row, &new->strict);
+								read_into_target(&new->target, &new->strict);
 								endtoken = yylex();
 							}
 							else if (endtoken == K_USING)
@@ -2107,11 +2086,10 @@ stmt_open		: K_OPEN cursor_variable
 stmt_fetch		: K_FETCH opt_fetch_direction cursor_variable K_INTO
 					{
 						PLpgSQL_stmt_fetch *fetch = $2;
-						PLpgSQL_rec	   *rec;
-						PLpgSQL_row	   *row;
+						PLpgSQL_variable *target;
 
 						/* We have already parsed everything through the INTO keyword */
-						read_into_target(&rec, &row, NULL);
+						read_into_target(&target, NULL);
 
 						if (yylex() != ';')
 							yyerror("syntax error");
@@ -2127,8 +2105,7 @@ stmt_fetch		: K_FETCH opt_fetch_direction cursor_variable K_INTO
 									 parser_errposition(@1)));
 
 						fetch->lineno = plpgsql_location_to_lineno(@1);
-						fetch->rec		= rec;
-						fetch->row		= row;
+						fetch->target	= target;
 						fetch->curvar	= $3->dno;
 						fetch->is_move	= false;
 
@@ -2842,8 +2819,7 @@ make_execsql_stmt(int firsttoken, int location)
 	IdentifierLookup	save_IdentifierLookup;
 	PLpgSQL_stmt_execsql *execsql;
 	PLpgSQL_expr		*expr;
-	PLpgSQL_row			*row = NULL;
-	PLpgSQL_rec			*rec = NULL;
+	PLpgSQL_variable	*target = NULL;
 	int					tok;
 	int					prev_tok;
 	bool				have_into = false;
@@ -2907,7 +2883,7 @@ make_execsql_stmt(int firsttoken, int location)
 			have_into = true;
 			into_start_loc = yylloc;
 			plpgsql_IdentifierLookup = IDENTIFIER_LOOKUP_NORMAL;
-			read_into_target(&rec, &row, &have_strict);
+			read_into_target(&target, &have_strict);
 			plpgsql_IdentifierLookup = IDENTIFIER_LOOKUP_EXPR;
 		}
 	}
@@ -2949,8 +2925,7 @@ make_execsql_stmt(int firsttoken, int location)
 	execsql->sqlstmt = expr;
 	execsql->into	 = have_into;
 	execsql->strict	 = have_strict;
-	execsql->rec	 = rec;
-	execsql->row	 = row;
+	execsql->target	 = target;
 
 	return (PLpgSQL_stmt *) execsql;
 }
@@ -3341,13 +3316,12 @@ check_assignable(PLpgSQL_datum *datum, int location)
  * INTO keyword.
  */
 static void
-read_into_target(PLpgSQL_rec **rec, PLpgSQL_row **row, bool *strict)
+read_into_target(PLpgSQL_variable **target, bool *strict)
 {
 	int			tok;
 
 	/* Set default results */
-	*rec = NULL;
-	*row = NULL;
+	*target = NULL;
 	if (strict)
 		*strict = false;
 
@@ -3368,22 +3342,11 @@ read_into_target(PLpgSQL_rec **rec, PLpgSQL_row **row, bool *strict)
 	switch (tok)
 	{
 		case T_DATUM:
-			if (yylval.wdatum.datum->dtype == PLPGSQL_DTYPE_ROW)
-			{
-				check_assignable(yylval.wdatum.datum, yylloc);
-				*row = (PLpgSQL_row *) yylval.wdatum.datum;
-
-				if ((tok = yylex()) == ',')
-					ereport(ERROR,
-							(errcode(ERRCODE_SYNTAX_ERROR),
-							 errmsg("record or row variable cannot be part of multiple-item INTO list"),
-							 parser_errposition(yylloc)));
-				plpgsql_push_back_token(tok);
-			}
-			else if (yylval.wdatum.datum->dtype == PLPGSQL_DTYPE_REC)
+			if (yylval.wdatum.datum->dtype == PLPGSQL_DTYPE_ROW ||
+				yylval.wdatum.datum->dtype == PLPGSQL_DTYPE_REC)
 			{
 				check_assignable(yylval.wdatum.datum, yylloc);
-				*rec = (PLpgSQL_rec *) yylval.wdatum.datum;
+				*target = (PLpgSQL_variable *) yylval.wdatum.datum;
 
 				if ((tok = yylex()) == ',')
 					ereport(ERROR,
@@ -3394,8 +3357,9 @@ read_into_target(PLpgSQL_rec **rec, PLpgSQL_row **row, bool *strict)
 			}
 			else
 			{
-				*row = read_into_scalar_list(NameOfDatum(&(yylval.wdatum)),
-											 yylval.wdatum.datum, yylloc);
+				*target = (PLpgSQL_variable *)
+					read_into_scalar_list(NameOfDatum(&(yylval.wdatum)),
+										  yylval.wdatum.datum, yylloc);
 			}
 			break;
 

src/pl/plpgsql/src/plpgsql.h

@@ -549,8 +549,7 @@ typedef struct PLpgSQL_stmt_forq
 	PLpgSQL_stmt_type cmd_type;
 	int			lineno;
 	char	   *label;
-	PLpgSQL_rec *rec;
-	PLpgSQL_row *row;
+	PLpgSQL_variable *var;		/* Loop variable (record or row) */
 	List	   *body;			/* List of statements */
 } PLpgSQL_stmt_forq;
 
@@ -562,8 +561,7 @@ typedef struct PLpgSQL_stmt_fors
 	PLpgSQL_stmt_type cmd_type;
 	int			lineno;
 	char	   *label;
-	PLpgSQL_rec *rec;
-	PLpgSQL_row *row;
+	PLpgSQL_variable *var;		/* Loop variable (record or row) */
 	List	   *body;			/* List of statements */
 	/* end of fields that must match PLpgSQL_stmt_forq */
 	PLpgSQL_expr *query;
@@ -577,8 +575,7 @@ typedef struct PLpgSQL_stmt_forc
 	PLpgSQL_stmt_type cmd_type;
 	int			lineno;
 	char	   *label;
-	PLpgSQL_rec *rec;
-	PLpgSQL_row *row;
+	PLpgSQL_variable *var;		/* Loop variable (record or row) */
 	List	   *body;			/* List of statements */
 	/* end of fields that must match PLpgSQL_stmt_forq */
 	int			curvar;
@@ -593,8 +590,7 @@ typedef struct PLpgSQL_stmt_dynfors
 	PLpgSQL_stmt_type cmd_type;
 	int			lineno;
 	char	   *label;
-	PLpgSQL_rec *rec;
-	PLpgSQL_row *row;
+	PLpgSQL_variable *var;		/* Loop variable (record or row) */
 	List	   *body;			/* List of statements */
 	/* end of fields that must match PLpgSQL_stmt_forq */
 	PLpgSQL_expr *query;
@@ -624,7 +620,6 @@ typedef struct PLpgSQL_stmt_open
 	int			lineno;
 	int			curvar;
 	int			cursor_options;
-	PLpgSQL_row *returntype;
 	PLpgSQL_expr *argquery;
 	PLpgSQL_expr *query;
 	PLpgSQL_expr *dynquery;
@@ -638,8 +633,7 @@ typedef struct PLpgSQL_stmt_fetch
 {
 	PLpgSQL_stmt_type cmd_type;
 	int			lineno;
-	PLpgSQL_rec *rec;			/* target, as record or row */
-	PLpgSQL_row *row;
+	PLpgSQL_variable *target;	/* target (record or row) */
 	int			curvar;			/* cursor variable to fetch from */
 	FetchDirection direction;	/* fetch direction */
 	long		how_many;		/* count, if constant (expr is NULL) */
@@ -750,8 +744,7 @@ typedef struct PLpgSQL_stmt_execsql
 								 * mod_stmt is set when we plan the query */
 	bool		into;			/* INTO supplied? */
 	bool		strict;			/* INTO STRICT flag */
-	PLpgSQL_rec *rec;			/* INTO target, if record */
-	PLpgSQL_row *row;			/* INTO target, if row */
+	PLpgSQL_variable *target;	/* INTO target (record or row) */
 } PLpgSQL_stmt_execsql;
 
 /*
@@ -764,8 +757,7 @@ typedef struct PLpgSQL_stmt_dynexecute
 	PLpgSQL_expr *query;		/* string expression */
 	bool		into;			/* INTO supplied? */
 	bool		strict;			/* INTO STRICT flag */
-	PLpgSQL_rec *rec;			/* INTO target, if record */
-	PLpgSQL_row *row;			/* INTO target, if row */
+	PLpgSQL_variable *target;	/* INTO target (record or row) */
 	List	   *params;			/* USING expressions */
 } PLpgSQL_stmt_dynexecute;