Defend against possible crash if a plpython function does not specify names

tglsfdc · tglsfdc · commit cd331e4b845f · 2009-04-03T16:59:43.000Z
for its arguments.  Also add a regression test, since someone apparently
changed every single plpython test case to use only named parameters; else
we'd have noticed this sooner.

Euler Taveira de Oliveira, per a report from Alvaro
diff --git a/src/pl/plpython/expected/plpython_function.out b/src/pl/plpython/expected/plpython_function.out
@@ -357,8 +357,11 @@ CREATE FUNCTION test_return_none() RETURNS int AS $$
 None
 $$ LANGUAGE plpythonu;
 --
--- Test named parameters
+-- Test named and nameless parameters
 --
+CREATE FUNCTION test_param_names0(integer, integer) RETURNS int AS $$
+return args[0] + args[1]
+$$ LANGUAGE plpythonu;
 CREATE FUNCTION test_param_names1(a0 integer, a1 text) RETURNS boolean AS $$
 assert a0 == args[0]
 assert a1 == args[1]
diff --git a/src/pl/plpython/expected/plpython_test.out b/src/pl/plpython/expected/plpython_test.out
@@ -197,7 +197,13 @@ SELECT test_return_none(), test_return_none() IS NULL AS "is null";
                   | t
 (1 row)
 
--- Test for functions with named parameters
+-- Test for functions with named and nameless parameters
+SELECT test_param_names0(2,7);
+ test_param_names0 
+-------------------
+                 9
+(1 row)
+
 SELECT test_param_names1(1,'text');
  test_param_names1 
 -------------------
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
@@ -1,7 +1,7 @@
 /**********************************************************************
  * plpython.c - python as a procedural language for PostgreSQL
  *
- *	$PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.119 2009/03/26 22:26:08 petere Exp $
+ *	$PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.120 2009/04/03 16:59:42 tgl Exp $
  *
  *********************************************************************
  */
@@ -1052,9 +1052,11 @@ PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure * proc)
 				arg = Py_None;
 			}
 
-			if (PyList_SetItem(args, i, arg) == -1 ||
-				(proc->argnames &&
-				 PyDict_SetItemString(proc->globals, proc->argnames[i], arg) == -1))
+			if (PyList_SetItem(args, i, arg) == -1)
+				PLy_elog(ERROR, "PyList_SetItem() failed for PL/Python function \"%s\" while setting up arguments", proc->proname);
+
+			if (proc->argnames && proc->argnames[i] &&
+				PyDict_SetItemString(proc->globals, proc->argnames[i], arg) == -1)
 				PLy_elog(ERROR, "PyDict_SetItemString() failed for PL/Python function \"%s\" while setting up arguments", proc->proname);
 			arg = NULL;
 		}
@@ -1081,7 +1083,8 @@ PLy_function_delete_args(PLyProcedure * proc)
 		return;
 
 	for (i = 0; i < proc->nargs; i++)
-		PyDict_DelItemString(proc->globals, proc->argnames[i]);
+		if (proc->argnames[i])
+			PyDict_DelItemString(proc->globals, proc->argnames[i]);
 }
 
 
diff --git a/src/pl/plpython/sql/plpython_function.sql b/src/pl/plpython/sql/plpython_function.sql
@@ -391,8 +391,12 @@ $$ LANGUAGE plpythonu;
 
 
 --
--- Test named parameters
+-- Test named and nameless parameters
 --
+CREATE FUNCTION test_param_names0(integer, integer) RETURNS int AS $$
+return args[0] + args[1]
+$$ LANGUAGE plpythonu;
+
 CREATE FUNCTION test_param_names1(a0 integer, a1 text) RETURNS boolean AS $$
 assert a0 == args[0]
 assert a1 == args[1]
diff --git a/src/pl/plpython/sql/plpython_test.sql b/src/pl/plpython/sql/plpython_test.sql
@@ -74,7 +74,8 @@ SELECT test_void_func1(), test_void_func1() IS NULL AS "is null";
 SELECT test_void_func2(); -- should fail
 SELECT test_return_none(), test_return_none() IS NULL AS "is null";
 
--- Test for functions with named parameters
+-- Test for functions with named and nameless parameters
+SELECT test_param_names0(2,7);
 SELECT test_param_names1(1,'text');
 SELECT test_param_names2(users) from users;
 SELECT test_param_names3(1);

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`/**********************************************************************`
`2`	`2`	`* plpython.c - python as a procedural language for PostgreSQL`
`3`	`3`	`*`
`4`		`- * $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.119 2009/03/26 22:26:08 petere Exp $`
	`4`	`+ * $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.120 2009/04/03 16:59:42 tgl Exp $`
`5`	`5`	`*`
`6`	`6`	`*********************************************************************`
`7`	`7`	`*/`
`@@ -1052,9 +1052,11 @@ PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure * proc)`
`1052`	`1052`	`arg = Py_None;`
`1053`	`1053`	`}`
`1054`	`1054`
`1055`		`- if (PyList_SetItem(args, i, arg) == -1 \|\|`
`1056`		`- (proc->argnames &&`
`1057`		`- PyDict_SetItemString(proc->globals, proc->argnames[i], arg) == -1))`
	`1055`	`+ if (PyList_SetItem(args, i, arg) == -1)`
	`1056`	`+ PLy_elog(ERROR, "PyList_SetItem() failed for PL/Python function \"%s\" while setting up arguments", proc->proname);`
	`1057`	`+`
	`1058`	`+ if (proc->argnames && proc->argnames[i] &&`
	`1059`	`+ PyDict_SetItemString(proc->globals, proc->argnames[i], arg) == -1)`
`1058`	`1060`	`PLy_elog(ERROR, "PyDict_SetItemString() failed for PL/Python function \"%s\" while setting up arguments", proc->proname);`
`1059`	`1061`	`arg = NULL;`
`1060`	`1062`	`}`
`@@ -1081,7 +1083,8 @@ PLy_function_delete_args(PLyProcedure * proc)`
`1081`	`1083`	`return;`
`1082`	`1084`
`1083`	`1085`	`for (i = 0; i < proc->nargs; i++)`
`1084`		`- PyDict_DelItemString(proc->globals, proc->argnames[i]);`
	`1086`	`+ if (proc->argnames[i])`
	`1087`	`+ PyDict_DelItemString(proc->globals, proc->argnames[i]);`
`1085`	`1088`	`}`
`1086`	`1089`
`1087`	`1090`