On 09/02/11 04:52, Hitoshi Harada wrote:
> 2010/12/31 Jan Urbański <[email protected]>:
>> (continuing the flurry of patches)
>>
>> Here's a patch that stops PL/Python from removing the function's
>> arguments from its globals dict after calling it. It's
>> an incremental patch on top of the plpython-refactor patch sent in
>> http://archives.postgresql.org/message-id/[email protected].
>>
>> Git branch for this patch:
>> https://github.com/wulczer/postgres/tree/dont-remove-arguments
>>
>> Apart from being useless, as the whole dict is unreffed and thus freed
>> in PLy_procedure_delete, removing args actively breaks things for
>> recursive invocation of the same function. The recursive callee after
>> returning will remove the args from globals, and subsequent access to
>> the arguments in the caller will cause a NameError (see new regression
>> test in patch).
>
> I've reviewed this. The patch is old enough to be rejected by patch
> command, but I manged to apply it by hand.
> It compiles clean. Added tests pass.
> I created fibonacci function similar to recursion_test in the patch
> and confirmed the recursion raises error on 9.0 but not on 9.1.
> Doc is not with the patch since this change is to remove unnecessary
> optimization internally.
>
> "Ready for Committer"
Thanks,
patch merged with HEAD attached.
Jan
diff --git a/src/pl/plpython/expected/plpython_spi.out b/src/pl/plpython/expected/plpython_spi.out
index 7f4ae5c..cb11f60 100644
*** a/src/pl/plpython/expected/plpython_spi.out
--- b/src/pl/plpython/expected/plpython_spi.out
*************** CONTEXT: PL/Python function "result_nro
*** 133,135 ****
--- 133,163 ----
2
(1 row)
+ --
+ -- check recursion with same argument does not clobber globals
+ --
+ CREATE FUNCTION recursion_test(n integer) RETURNS integer
+ AS $$
+ if n in (0, 1):
+ return 1
+
+ return n * plpy.execute("select recursion_test(%d) as result" % (n - 1))[0]["result"]
+ $$ LANGUAGE plpythonu;
+ SELECT recursion_test(5);
+ recursion_test
+ ----------------
+ 120
+ (1 row)
+
+ SELECT recursion_test(4);
+ recursion_test
+ ----------------
+ 24
+ (1 row)
+
+ SELECT recursion_test(1);
+ recursion_test
+ ----------------
+ 1
+ (1 row)
+
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index fff7de7..61ba793 100644
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
*************** static Datum PLy_function_handler(Functi
*** 318,324 ****
static HeapTuple PLy_trigger_handler(FunctionCallInfo fcinfo, PLyProcedure *);
static PyObject *PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure *);
- static void PLy_function_delete_args(PLyProcedure *);
static PyObject *PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *,
HeapTuple *);
static HeapTuple PLy_modify_tuple(PLyProcedure *, PyObject *,
--- 318,323 ----
*************** PLy_function_handler(FunctionCallInfo fc
*** 1036,1049 ****
*/
plargs = PLy_function_build_args(fcinfo, proc);
plrv = PLy_procedure_call(proc, "args", plargs);
- if (!proc->is_setof)
- {
- /*
- * SETOF function parameters will be deleted when last row is
- * returned
- */
- PLy_function_delete_args(proc);
- }
Assert(plrv != NULL);
}
--- 1035,1040 ----
*************** PLy_function_handler(FunctionCallInfo fc
*** 1101,1108 ****
Py_XDECREF(plargs);
Py_XDECREF(plrv);
- PLy_function_delete_args(proc);
-
if (has_error)
ereport(ERROR,
(errcode(ERRCODE_DATA_EXCEPTION),
--- 1092,1097 ----
*************** PLy_function_build_args(FunctionCallInfo
*** 1310,1329 ****
return args;
}
-
- static void
- PLy_function_delete_args(PLyProcedure *proc)
- {
- int i;
-
- if (!proc->argnames)
- return;
-
- for (i = 0; i < proc->nargs; i++)
- if (proc->argnames[i])
- PyDict_DelItemString(proc->globals, proc->argnames[i]);
- }
-
/*
* Decide whether a cached PLyProcedure struct is still valid
*/
--- 1299,1304 ----
diff --git a/src/pl/plpython/sql/plpython_spi.sql b/src/pl/plpython/sql/plpython_spi.sql
index 7f8f6a3..3b65f95 100644
*** a/src/pl/plpython/sql/plpython_spi.sql
--- b/src/pl/plpython/sql/plpython_spi.sql
*************** else:
*** 105,107 ****
--- 105,123 ----
$$ LANGUAGE plpythonu;
SELECT result_nrows_test();
+
+
+ --
+ -- check recursion with same argument does not clobber globals
+ --
+ CREATE FUNCTION recursion_test(n integer) RETURNS integer
+ AS $$
+ if n in (0, 1):
+ return 1
+
+ return n * plpy.execute("select recursion_test(%d) as result" % (n - 1))[0]["result"]
+ $$ LANGUAGE plpythonu;
+
+ SELECT recursion_test(5);
+ SELECT recursion_test(4);
+ SELECT recursion_test(1);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
