Fix display of SQL-standard function's arguments in INSERT/SELECT.

tglsfdc · tglsfdc · commit a8d8445a7b2f · 2021-11-17T11:31:31.000-05:00
If a SQL-standard function body contains an INSERT ... SELECT statement, any function parameters referenced within the SELECT were always printed in $N style, rather than using the parameter name if any. While not strictly incorrect, this wasn't the intention, and it's inconsistent with the way that such parameters would be printed in any other kind of statement. The cause is that the recursion to get_query_def from get_insert_query_def neglected to pass down the context->namespaces list, passing constant NIL instead. This is a very ancient oversight, but AFAICT it had no visible consequences before commit e717a9a added an outermost namespace with function parameters. We don't allow INSERT ... SELECT as a sub-query, except in a top-level WITH clause, where it couldn't contain any outer references that might need to access upper namespaces. So although that's arguably a bug, I don't see any point in changing it before v14. In passing, harden the code added to get_parameter by e717a9a so that it won't crash if a PARAM_EXTERN Param appears in an unexpected place. Per report from Erki Eessaar. Code fix by me, regression test case by Masahiko Sawada. Discussion: https://postgr.es/m/AM9PR01MB8268347BED344848555167FAFE949@AM9PR01MB8268.eurprd01.prod.exchangelabs.com
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
@@ -6566,7 +6566,7 @@ get_insert_query_def(Query *query, deparse_context *context)
 	if (select_rte)
 	{
 		/* Add the SELECT */
-		get_query_def(select_rte->subquery, buf, NIL, NULL,
+		get_query_def(select_rte->subquery, buf, context->namespaces, NULL,
 					  context->prettyFlags, context->wrapColumn,
 					  context->indentLevel);
 	}
@@ -7919,10 +7919,12 @@ get_parameter(Param *param, deparse_context *context)
 	 * If it's an external parameter, see if the outermost namespace provides
 	 * function argument names.
 	 */
-	if (param->paramkind == PARAM_EXTERN)
+	if (param->paramkind == PARAM_EXTERN && context->namespaces != NIL)
 	{
-		dpns = lfirst(list_tail(context->namespaces));
-		if (dpns->argnames)
+		dpns = llast(context->namespaces);
+		if (dpns->argnames &&
+			param->paramid > 0 &&
+			param->paramid <= dpns->numargs)
 		{
 			char	   *argname = dpns->argnames[param->paramid - 1];
 
diff --git a/src/test/regress/expected/create_function_3.out b/src/test/regress/expected/create_function_3.out
@@ -279,6 +279,13 @@ CREATE FUNCTION functest_S_13() RETURNS boolean
         SELECT 1;
         SELECT false;
     END;
+-- check display of function argments in sub-SELECT
+CREATE TABLE functest1 (i int);
+CREATE FUNCTION functest_S_16(a int, b int) RETURNS void
+    LANGUAGE SQL
+    BEGIN ATOMIC
+        INSERT INTO functest1 SELECT a + $2;
+    END;
 -- error: duplicate function body
 CREATE FUNCTION functest_S_xxx(x int) RETURNS int
     LANGUAGE SQL
@@ -417,6 +424,20 @@ SELECT pg_get_functiondef('functest_S_15'::regproc);
  
 (1 row)
 
+SELECT pg_get_functiondef('functest_S_16'::regproc);
+                              pg_get_functiondef                               
+-------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION temp_func_test.functest_s_16(a integer, b integer)+
+  RETURNS void                                                                +
+  LANGUAGE sql                                                                +
+ BEGIN ATOMIC                                                                 +
+  INSERT INTO functest1 (i)  SELECT (functest_s_16.a + functest_s_16.b);      +
+ END                                                                          +
+ 
+(1 row)
+
+DROP TABLE functest1 CASCADE;
+NOTICE:  drop cascades to function functest_s_16(integer,integer)
 -- test with views
 CREATE TABLE functest3 (a int);
 INSERT INTO functest3 VALUES (1), (2);
diff --git a/src/test/regress/sql/create_function_3.sql b/src/test/regress/sql/create_function_3.sql
@@ -180,6 +180,14 @@ CREATE FUNCTION functest_S_13() RETURNS boolean
         SELECT false;
     END;
 
+-- check display of function argments in sub-SELECT
+CREATE TABLE functest1 (i int);
+CREATE FUNCTION functest_S_16(a int, b int) RETURNS void
+    LANGUAGE SQL
+    BEGIN ATOMIC
+        INSERT INTO functest1 SELECT a + $2;
+    END;
+
 -- error: duplicate function body
 CREATE FUNCTION functest_S_xxx(x int) RETURNS int
     LANGUAGE SQL
@@ -217,6 +225,9 @@ SELECT pg_get_functiondef('functest_S_3a'::regproc);
 SELECT pg_get_functiondef('functest_S_10'::regproc);
 SELECT pg_get_functiondef('functest_S_13'::regproc);
 SELECT pg_get_functiondef('functest_S_15'::regproc);
+SELECT pg_get_functiondef('functest_S_16'::regproc);
+
+DROP TABLE functest1 CASCADE;
 
 -- test with views
 CREATE TABLE functest3 (a int);

Original file line number	Diff line number	Diff line change
`@@ -6566,7 +6566,7 @@ get_insert_query_def(Query query, deparse_context context)`
`6566`	`6566`	`if (select_rte)`
`6567`	`6567`	`{`
`6568`	`6568`	`/* Add the SELECT */`
`6569`		`- get_query_def(select_rte->subquery, buf, NIL, NULL,`
	`6569`	`+ get_query_def(select_rte->subquery, buf, context->namespaces, NULL,`
`6570`	`6570`	`context->prettyFlags, context->wrapColumn,`
`6571`	`6571`	`context->indentLevel);`
`6572`	`6572`	`}`
`@@ -7919,10 +7919,12 @@ get_parameter(Param param, deparse_context context)`
`7919`	`7919`	`* If it's an external parameter, see if the outermost namespace provides`
`7920`	`7920`	`* function argument names.`
`7921`	`7921`	`*/`
`7922`		`- if (param->paramkind == PARAM_EXTERN)`
	`7922`	`+ if (param->paramkind == PARAM_EXTERN && context->namespaces != NIL)`
`7923`	`7923`	`{`
`7924`		`- dpns = lfirst(list_tail(context->namespaces));`
`7925`		`- if (dpns->argnames)`
	`7924`	`+ dpns = llast(context->namespaces);`
	`7925`	`+ if (dpns->argnames &&`
	`7926`	`+ param->paramid > 0 &&`
	`7927`	`+ param->paramid <= dpns->numargs)`
`7926`	`7928`	`{`
`7927`	`7929`	`char *argname = dpns->argnames[param->paramid - 1];`
`7928`	`7930`