fix processing of out parameters of procedures

Author: okbob

expected/plpgsql_check_active-15.out

@@ -550,3 +550,41 @@ select * from plpgsql_check_function('prtest'); --ok
 (0 rows)
 
 drop procedure prtest();
+create function return_constant_refcursor() returns refcursor as $$
+declare
+    rc constant refcursor;
+begin
+    open rc for select a from rc_test;
+    return rc;
+end
+$$ language plpgsql;
+create table rc_test(a int);
+select * from plpgsql_check_function('return_constant_refcursor');
+                plpgsql_check_function                 
+-------------------------------------------------------
+ error:22005:5:OPEN:variable "rc" is declared CONSTANT
+(1 row)
+
+drop table rc_test;
+drop function return_constant_refcursor();
+create procedure p1(a int, out b int)
+as $$
+begin
+  b := a + 10;
+end;
+$$ language plpgsql;
+create function f1()
+returns void as $$
+declare b constant int;
+begin
+  call p1(10, b);
+end;
+$$ language plpgsql;
+select * from plpgsql_check_function('f1');
+                plpgsql_check_function                
+------------------------------------------------------
+ error:22005:4:CALL:variable "b" is declared CONSTANT
+(1 row)
+
+drop function f1();
+drop procedure p1(int, int);

sql/plpgsql_check_active-15.sql

@@ -325,3 +325,39 @@ $$ language plpgsql;
 select * from plpgsql_check_function('prtest'); --ok
 
 drop procedure prtest();
+
+create function return_constant_refcursor() returns refcursor as $$
+declare
+    rc constant refcursor;
+begin
+    open rc for select a from rc_test;
+    return rc;
+end
+$$ language plpgsql;
+
+create table rc_test(a int);
+
+select * from plpgsql_check_function('return_constant_refcursor');
+
+drop table rc_test;
+drop function return_constant_refcursor();
+
+create procedure p1(a int, out b int)
+as $$
+begin
+  b := a + 10;
+end;
+$$ language plpgsql;
+
+create function f1()
+returns void as $$
+declare b constant int;
+begin
+  call p1(10, b);
+end;
+$$ language plpgsql;
+
+select * from plpgsql_check_function('f1');
+
+drop function f1();
+drop procedure p1(int, int);

src/assign.c

@@ -95,8 +95,8 @@ plpgsql_check_row_or_rec(PLpgSQL_checkstate *cstate, PLpgSQL_row *row, PLpgSQL_r
 	}
 }
 
-static void
-exec_check_assignable(PLpgSQL_execstate *estate, int dno)
+void
+plpgsql_check_is_assignable(PLpgSQL_execstate *estate, int dno)
 {
 	PLpgSQL_datum *datum;
 
@@ -121,7 +121,7 @@ exec_check_assignable(PLpgSQL_execstate *estate, int dno)
 			break;
 		case PLPGSQL_DTYPE_RECFIELD:
 			/* assignable if parent record is */
-			exec_check_assignable(estate,
+			plpgsql_check_is_assignable(estate,
 								  ((PLpgSQL_recfield *) datum)->recparentno);
 			break;
 		default:
@@ -131,9 +131,6 @@ exec_check_assignable(PLpgSQL_execstate *estate, int dno)
 
 #else
 
-elog(NOTICE, "kuku");
-
-
 	if (datum->dtype == PLPGSQL_DTYPE_VAR)
 		if (((PLpgSQL_var *) datum)->isconst)
 			ereport(ERROR,
@@ -154,7 +151,7 @@ plpgsql_check_target(PLpgSQL_checkstate *cstate, int varno, Oid *expected_typoid
 {
 	PLpgSQL_datum *target = cstate->estate->datums[varno];
 
-	exec_check_assignable(cstate->estate, varno);
+	plpgsql_check_is_assignable(cstate->estate, varno);
 	plpgsql_check_record_variable_usage(cstate, varno, true);
 
 	switch (target->dtype)

src/plpgsql_check.h

@@ -181,6 +181,8 @@ extern void plpgsql_check_assign_tupdesc_row_or_rec(PLpgSQL_checkstate *cstate,
 extern void plpgsql_check_recval_assign_tupdesc(PLpgSQL_checkstate *cstate, PLpgSQL_rec *rec, TupleDesc tupdesc, bool is_null);
 extern void plpgsql_check_recval_init(PLpgSQL_rec *rec);
 extern void plpgsql_check_recval_release(PLpgSQL_rec *rec);
+extern void plpgsql_check_is_assignable(PLpgSQL_execstate *estate, int dno);
+
 
 /*
  * functions from format.c

src/typdesc.c

@@ -46,14 +46,14 @@ plpgsql_check_CallExprGetRowTarget(PLpgSQL_checkstate *cstate, PLpgSQL_expr *Cal
 	{
 		PLpgSQL_row *row;
 		CachedPlanSource *plansource;
-		HeapTuple		tuple;
-		List	   *funcargs;
+		CallStmt   *stmt;
+		HeapTuple	tuple;
 		Oid		   *argtypes;
 		char	  **argnames;
 		char	   *argmodes;
-		ListCell   *lc;
 		int			i;
 		int			nfields = 0;
+		int			numargs;
 
 		plansource = plpgsql_check_get_plan_source(cstate, CallExpr->plan);
 
@@ -64,7 +64,8 @@ plpgsql_check_CallExprGetRowTarget(PLpgSQL_checkstate *cstate, PLpgSQL_expr *Cal
 		if (!IsA(node, CallStmt))
 			elog(ERROR, "returned row from not a CallStmt");
 
-		funcexpr = castNode(CallStmt, node)->funcexpr;
+		stmt = castNode(CallStmt, node);
+		funcexpr = stmt->funcexpr;
 
 		/*
 		 * Get the argument modes
@@ -73,43 +74,45 @@ plpgsql_check_CallExprGetRowTarget(PLpgSQL_checkstate *cstate, PLpgSQL_expr *Cal
 		if (!HeapTupleIsValid(tuple))
 			elog(ERROR, "cache lookup failed for function %u", funcexpr->funcid);
 
-		/* Extract function arguments, and expand any named-arg notation */
-		funcargs = expand_function_arguments(funcexpr->args,
-#if PG_VERSION_NUM >= 140000
-											 true,
-#endif
-											 funcexpr->funcresulttype,
-											 tuple);
-
-		get_func_arg_info(tuple, &argtypes, &argnames, &argmodes);
+		/*
+		 * Get the argument names and modes, so that we can deliver on-point error
+		 * messages when something is wrong.
+		 */
+		numargs = get_func_arg_info(tuple, &argtypes, &argnames, &argmodes);
 
 		ReleaseSysCache(tuple);
 
-		row = palloc0(sizeof(PLpgSQL_row));
+		row = (PLpgSQL_row *) palloc0(sizeof(PLpgSQL_row));
 		row->dtype = PLPGSQL_DTYPE_ROW;
-		row->dno = -1;
 		row->refname = NULL;
-		row->lineno = 0;
-		row->varnos = palloc(sizeof(int) * list_length(funcargs));
+		row->dno = -1;
+		row->lineno = -1;
+		row->varnos = (int *) palloc(numargs * sizeof(int));
 
 		/*
-		 * Construct row
+		 * Examine procedure's argument list.  Each output arg position should be
+		 * an unadorned plpgsql variable (Datum), which we can insert into the row
+		 * Datum.
 		 */
-		i = 0;
-		foreach(lc, funcargs)
+		nfields = 0;
+		for (i = 0; i < numargs; i++)
 		{
-			Node	   *n = lfirst(lc);
-
 			if (argmodes &&
 				(argmodes[i] == PROARGMODE_INOUT ||
 				 argmodes[i] == PROARGMODE_OUT))
 			{
+				Node	   *n = list_nth(stmt->outargs, nfields);
+
 				if (IsA(n, Param))
 				{
 					Param	   *param = (Param *) n;
+					int			dno;
 
 					/* paramid is offset by 1 (see make_datum_param()) */
-					row->varnos[nfields++] = param->paramid - 1;
+					dno = param->paramid - 1;
+					/* must check assignability now, because grammar can't */
+					plpgsql_check_is_assignable(cstate->estate, dno);
+					row->varnos[nfields++] = dno;
 				}
 				else
 				{
@@ -126,9 +129,10 @@ plpgsql_check_CallExprGetRowTarget(PLpgSQL_checkstate *cstate, PLpgSQL_expr *Cal
 										i + 1)));
 				}
 			}
-			i++;
 		}
 
+		Assert(nfields == list_length(stmt->outargs));
+
 		row->nfields = nfields;
 
 		/* Don't return empty row variable */