shihao zhong <[email protected]> writes:
> Did a quick test, I think it worked on master
> CREATE FUNCTION r3(OUT r record)
> AS $$ return (1, 2) $$ LANGUAGE plpython3u;
> SELECT * FROM r3() AS (a int, b int);
> SELECT * FROM r3();
Ah, good point. I was thinking about the general case,
which doesn't really work:
regression=# CREATE FUNCTION multiout_record(INOUT n integer, OUT record) AS $$
return (n, (n * 2, n * 3))
$$ LANGUAGE plpython3u;
CREATE FUNCTION
regression=# SELECT * FROM multiout_record(2);
ERROR: record type has not been registered
CONTEXT: while creating return value
PL/Python function "multiout_record"
But with just one "OUT record" parameter, that's equivalent to
"RETURNS record", so it should work. This is showing us another
omission in plpython's regression tests :-(
After sleeping on it, I've decided that while moving these checks into
the validator is a good idea, it's not the most appropriate way to fix
this in v19. That is, I now think that this is really funccache.c's
bug not plpython's: it should be on funccache.c to not fail in this
situation. Yesterday I'd been dithering over whether external users
of funccache.c might want the existing behavior, but I can't see how
it'd be useful to them. Validators subject to this issue would have
to construct fake fcinfos that are much more complete than what we've
expected so far.
So here's a v2 patch series. 0001 fixes the immediate problem by
teaching funccache.c to not fail, and I envision back-patching that
to v19 or maybe even v18. Then 0002 rearranges plpython's validator,
but I now think that should go to master only. It does cause a user-
visible behavioral change (tighter checking with check_function_bodies
off), and we should refrain from unnecessary changes of that kind
post-rc1.
regards, tom lane
From 68a1f8600e0b43c8c1960362e062165efe1379cc Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
Date: Sat, 31 Oct 2026 11:26:56 -0400
Subject: [PATCH v2 1/2] Fix failure in function cache entry hashing during
validation mode.
When cached_function_compile() is called with forValidator = true,
the passed FunctionCallInfo is one that the caller set up specially,
and it's typically pretty incomplete. compute_function_hashkey()
was careless about this and would call get_call_result_type()
if asked to, which would fail in any but the simplest cases.
We'd managed not to notice in the pre-existing use-cases because
plpgsql and functions.c don't pass includeResultType and forValidator
both true at the same time; but plpython does, so it would reach this
failure in cases such as polymorphic argument types. plpython does
not yet support polymorphic arguments, and has no test cases about
them, so we did not see that the intended error message had degraded
to an internal failure in get_call_result_type().
To fix, just skip trying to fill the result type tupdesc when in
validation mode. This leads to building a hashtable entry that will
never match any real calls, but that's true in a lot of other cases
as well.
The root cause here is lack of clarity about what we expect the
passed FunctionCallInfo to contain in validation mode, so add
comments specifying that. Also add some plpython test cases showing
the expected error reports.
Backpatch to v18 where funccache.c was introduced. This problem's
not reachable in the core code in v18, but extensions could perhaps
be using funccache.c as of v18, and they might hit it.
Author: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 18
---
src/backend/utils/cache/funccache.c | 15 ++++++++++++---
src/pl/plpython/expected/plpython_error.out | 10 ++++++++++
src/pl/plpython/sql/plpython_error.sql | 10 ++++++++++
3 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/cache/funccache.c b/src/backend/utils/cache/funccache.c
index b3093ae5940..7b3422a07bf 100644
--- a/src/backend/utils/cache/funccache.c
+++ b/src/backend/utils/cache/funccache.c
@@ -242,6 +242,9 @@ cfunc_hashtable_delete(CachedFunction *function)
* The hashkey is returned into the caller-provided storage at *hashkey.
* Note however that if a callResultType is incorporated, we've not done
* anything about copying that.
+ *
+ * See comments for cached_function_compile() about how much of *fcinfo
+ * we're allowed to consider when forValidator is true.
*/
static void
compute_function_hashkey(FunctionCallInfo fcinfo,
@@ -326,9 +329,12 @@ compute_function_hashkey(FunctionCallInfo fcinfo,
* a function returning RECORD may depend on a column definition list to
* determine its output rowtype. If the caller needs the exact result
* type to be part of the hash lookup key, we must run
- * get_call_result_type() to find that out.
+ * get_call_result_type() to find that out. However, in validation mode
+ * the supplied fcinfo will likely not be complete enough for that to
+ * work, so leave callResultType unfilled (implying that the resulting
+ * hash entry will never match any actual calls for such functions).
*/
- if (includeResultType)
+ if (includeResultType && !forValidator)
{
Oid resultTypeId;
TupleDesc tupdesc;
@@ -482,7 +488,10 @@ delete_function(CachedFunction *func)
* include the actual result descriptor in the cache lookup key.
*
* If forValidator is true, we're only compiling for validation purposes,
- * and so some checks are skipped.
+ * and so some checks are skipped. In this case the passed fcinfo need
+ * only provide the function OID and indications of whether the function
+ * is a trigger or event trigger (as tested via CALLED_AS_TRIGGER or
+ * CALLED_AS_EVENT_TRIGGER).
*
* Note: it's important for this to fall through quickly if the function
* has already been compiled.
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index 96bbdfb9586..42003ffd69b 100644
--- a/src/pl/plpython/expected/plpython_error.out
+++ b/src/pl/plpython/expected/plpython_error.out
@@ -458,3 +458,13 @@ ERROR: error fetching next item from iterator
DETAIL: spiexceptions.InvalidDatetimeFormat: invalid value "xy" for "DD"
CONTEXT: Traceback (most recent call last):
PL/Python function "python_error_detail"
+/* test the presently-unsupported case of polymorphic functions
+ */
+CREATE FUNCTION python_poly_ret(x anyelement) RETURNS anyelement AS $$
+ return x
+$$ LANGUAGE plpython3u;
+ERROR: PL/Python functions cannot return type anyelement
+CREATE FUNCTION python_poly_arg(x anyelement) RETURNS int AS $$
+ return x
+$$ LANGUAGE plpython3u;
+ERROR: PL/Python functions cannot accept type anyelement
diff --git a/src/pl/plpython/sql/plpython_error.sql b/src/pl/plpython/sql/plpython_error.sql
index 9bb1c0b085a..35dd16440c4 100644
--- a/src/pl/plpython/sql/plpython_error.sql
+++ b/src/pl/plpython/sql/plpython_error.sql
@@ -355,3 +355,13 @@ CREATE FUNCTION python_error_detail() RETURNS SETOF text AS $$
$$ LANGUAGE plpython3u;
SELECT python_error_detail();
+
+/* test the presently-unsupported case of polymorphic functions
+ */
+CREATE FUNCTION python_poly_ret(x anyelement) RETURNS anyelement AS $$
+ return x
+$$ LANGUAGE plpython3u;
+
+CREATE FUNCTION python_poly_arg(x anyelement) RETURNS int AS $$
+ return x
+$$ LANGUAGE plpython3u;
--
2.52.0
From a5c6d33a199041c511d2a67362ef8bf54ff1e2d9 Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
Date: Sat, 31 Oct 2026 12:04:42 -0400
Subject: [PATCH v2 2/2] Move plpython's rejection of pseudotype
arguments/results.
Perform this work in the validator function, rather than repeating
it during every function call. The previous coding not only
performed unnecessary catalog lookups during function call setup,
but failed to detect invalid argument types at CREATE FUNCTION
time if check_function_bodies is off. That's not how that's
supposed to work.
Review of this patch exposed that we were not testing some cases
involving record-type output parameters, so add tests memorializing
the expected behavior.
In passing, fix an obsolete comment in plpython_composite.sql.
Author: Tom Lane <[email protected]>
Reviewed-by: shihao zhong <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
.../plpython/expected/plpython_composite.out | 11 +++-
src/pl/plpython/expected/plpython_record.out | 28 ++++++++++
src/pl/plpython/plpy_main.c | 56 ++++++++++++++++++-
src/pl/plpython/plpy_procedure.c | 48 ++--------------
src/pl/plpython/sql/plpython_composite.sql | 11 +++-
src/pl/plpython/sql/plpython_record.sql | 17 ++++++
6 files changed, 123 insertions(+), 48 deletions(-)
diff --git a/src/pl/plpython/expected/plpython_composite.out b/src/pl/plpython/expected/plpython_composite.out
index ffce7fc1be7..9c9f41855e6 100644
--- a/src/pl/plpython/expected/plpython_composite.out
+++ b/src/pl/plpython/expected/plpython_composite.out
@@ -302,7 +302,7 @@ SELECT * FROM multiout_composite();
3 | 4
(2 rows)
--- composite OUT parameters in functions returning RECORD not supported yet
+-- composite OUT parameters in functions returning RECORD
CREATE FUNCTION multiout_composite(INOUT n integer, OUT type_record) AS $$
return (n, (n * 2, n * 3))
$$ LANGUAGE plpython3u;
@@ -430,6 +430,15 @@ SELECT * FROM multiout_table_type_setof('dict', 't', 3);
2 |
(3 rows)
+-- But RECORD, rather than a named type, doesn't work in this context
+-- for lack of a way to identify its concrete type
+CREATE FUNCTION multiout_record(INOUT n integer, OUT record) AS $$
+return (n, (n * 2, n * 3))
+$$ LANGUAGE plpython3u;
+SELECT * FROM multiout_record(2);
+ERROR: record type has not been registered
+CONTEXT: while creating return value
+PL/Python function "multiout_record"
-- check what happens if a type changes under us
CREATE TABLE changing (
i integer,
diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out
index 31de198582b..26eca741f77 100644
--- a/src/pl/plpython/expected/plpython_record.out
+++ b/src/pl/plpython/expected/plpython_record.out
@@ -337,6 +337,34 @@ SELECT * FROM test_type_record_as('obj', 'one', 1, false);
one | 1
(1 row)
+-- check handling of record types defined by the calling query
+CREATE FUNCTION test_returns_record() RETURNS record AS $$
+ return (1, 2)
+$$ LANGUAGE plpython3u;
+SELECT * FROM test_returns_record() AS (a int, b int);
+ a | b
+---+---
+ 1 | 2
+(1 row)
+
+SELECT * FROM test_returns_record(); -- fail
+ERROR: a column definition list is required for functions returning "record"
+LINE 1: SELECT * FROM test_returns_record();
+ ^
+-- alternate spelling of identical functionality
+CREATE FUNCTION test_returns_out_record(OUT r record) AS $$
+ return (1, 2)
+$$ LANGUAGE plpython3u;
+SELECT * FROM test_returns_out_record() AS (a int, b int);
+ a | b
+---+---
+ 1 | 2
+(1 row)
+
+SELECT * FROM test_returns_out_record(); -- fail
+ERROR: a column definition list is required for functions returning "record"
+LINE 1: SELECT * FROM test_returns_out_record();
+ ^
-- errors cases
CREATE FUNCTION test_type_record_error1() RETURNS type_record AS $$
return { 'first': 'first' }
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index df08d3fbd1b..ce0e7044d9d 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -10,6 +10,7 @@
#include "commands/event_trigger.h"
#include "commands/trigger.h"
#include "executor/spi.h"
+#include "funcapi.h"
#include "miscadmin.h"
#include "plpy_elog.h"
#include "plpy_exec.h"
@@ -17,7 +18,9 @@
#include "plpy_plpymodule.h"
#include "plpy_subxactobject.h"
#include "plpy_util.h"
+#include "utils/builtins.h"
#include "utils/guc.h"
+#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/syscache.h"
@@ -111,6 +114,11 @@ plpython3_validator(PG_FUNCTION_ARGS)
Oid funcoid = PG_GETARG_OID(0);
HeapTuple tuple;
Form_pg_proc procStruct;
+ Oid rettype;
+ Oid *argtypes;
+ char **argnames,
+ *argmodes;
+ int nargs;
PLyTrigType is_trigger;
TriggerData trigdata;
EventTriggerData etrigdata;
@@ -120,19 +128,61 @@ plpython3_validator(PG_FUNCTION_ARGS)
if (!CheckFunctionValidatorAccess(fcinfo->flinfo->fn_oid, funcoid))
PG_RETURN_VOID();
- if (!check_function_bodies)
- PG_RETURN_VOID();
-
/* Get the new function's pg_proc entry */
tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcoid));
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for function %u", funcoid);
procStruct = (Form_pg_proc) GETSTRUCT(tuple);
+ /* Disallow pseudotype result, except for void or record */
+ rettype = procStruct->prorettype;
+ if (get_typtype(rettype) == TYPTYPE_PSEUDO)
+ {
+ if (rettype == VOIDOID || rettype == RECORDOID)
+ /* okay */ ;
+ else if (rettype == TRIGGEROID || rettype == EVENT_TRIGGEROID)
+ /* okay */ ;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("PL/Python functions cannot return type %s",
+ format_type_be(rettype))));
+ }
+
+ /* Disallow pseudotype arguments, except for output records */
+ nargs = get_func_arg_info(tuple, &argtypes, &argnames, &argmodes);
+ for (int i = 0; i < nargs; i++)
+ {
+ if (get_typtype(argtypes[i]) == TYPTYPE_PSEUDO)
+ {
+ if (argmodes &&
+ (argmodes[i] == PROARGMODE_OUT ||
+ argmodes[i] == PROARGMODE_TABLE))
+ {
+ if (rettype == RECORDOID)
+ /* okay */ ;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("PL/Python functions cannot return type %s",
+ format_type_be(argtypes[i]))));
+ }
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("PL/Python functions cannot accept type %s",
+ format_type_be(argtypes[i]))));
+ }
+ }
+
is_trigger = PLy_procedure_is_trigger(procStruct);
ReleaseSysCache(tuple);
+ /* The rest is not needed/wanted if check_function_bodies is off */
+ if (!check_function_bodies)
+ PG_RETURN_VOID();
+
/*
* Set up a fake flinfo/fcinfo with just enough info to satisfy
* PLy_procedure_get(). That function derives the call context (plain
diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c
index acfc0d8f0f0..ce8b1e099e9 100644
--- a/src/pl/plpython/plpy_procedure.c
+++ b/src/pl/plpython/plpy_procedure.c
@@ -217,36 +217,16 @@ PLy_procedure_create(PLyProcedure *proc,
if (is_trigger == PLPY_NOT_TRIGGER)
{
Oid rettype = procStruct->prorettype;
- HeapTuple rvTypeTup;
- Form_pg_type rvTypeStruct;
- rvTypeTup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(rettype));
- if (!HeapTupleIsValid(rvTypeTup))
- elog(ERROR, "cache lookup failed for type %u", rettype);
- rvTypeStruct = (Form_pg_type) GETSTRUCT(rvTypeTup);
-
- /* Disallow pseudotype result, except for void or record */
- if (rvTypeStruct->typtype == TYPTYPE_PSEUDO)
- {
- if (rettype == VOIDOID ||
- rettype == RECORDOID)
- /* okay */ ;
- else if (rettype == TRIGGEROID || rettype == EVENT_TRIGGEROID)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("trigger functions can only be called as triggers")));
- else
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("PL/Python functions cannot return type %s",
- format_type_be(rettype))));
- }
+ /* reject if function is in fact a trigger function */
+ if (rettype == TRIGGEROID || rettype == EVENT_TRIGGEROID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("trigger functions can only be called as triggers")));
/* set up output function for procedure result */
PLy_output_setup_func(&proc->result, proc->mcxt,
rettype, -1, proc);
-
- ReleaseSysCache(rvTypeTup);
}
else
{
@@ -296,9 +276,6 @@ PLy_procedure_create(PLyProcedure *proc,
for (i = pos = 0; i < total; i++)
{
- HeapTuple argTypeTup;
- Form_pg_type argTypeStruct;
-
if (modes &&
(modes[i] == PROARGMODE_OUT ||
modes[i] == PROARGMODE_TABLE))
@@ -306,19 +283,6 @@ PLy_procedure_create(PLyProcedure *proc,
Assert(types[i] == procStruct->proargtypes.values[pos]);
- argTypeTup = SearchSysCache1(TYPEOID,
- ObjectIdGetDatum(types[i]));
- if (!HeapTupleIsValid(argTypeTup))
- elog(ERROR, "cache lookup failed for type %u", types[i]);
- argTypeStruct = (Form_pg_type) GETSTRUCT(argTypeTup);
-
- /* disallow pseudotype arguments */
- if (argTypeStruct->typtype == TYPTYPE_PSEUDO)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("PL/Python functions cannot accept type %s",
- format_type_be(types[i]))));
-
/* set up I/O function info */
PLy_input_setup_func(&proc->args[pos], proc->mcxt,
types[i], -1, /* typmod not known */
@@ -327,8 +291,6 @@ PLy_procedure_create(PLyProcedure *proc,
/* get argument name */
proc->argnames[pos] = names ? pstrdup(names[i]) : NULL;
- ReleaseSysCache(argTypeTup);
-
pos++;
}
}
diff --git a/src/pl/plpython/sql/plpython_composite.sql b/src/pl/plpython/sql/plpython_composite.sql
index b401b3f2f6b..f70e74a2930 100644
--- a/src/pl/plpython/sql/plpython_composite.sql
+++ b/src/pl/plpython/sql/plpython_composite.sql
@@ -110,7 +110,7 @@ $$ LANGUAGE plpython3u;
SELECT * FROM singleout_composite();
SELECT * FROM multiout_composite();
--- composite OUT parameters in functions returning RECORD not supported yet
+-- composite OUT parameters in functions returning RECORD
CREATE FUNCTION multiout_composite(INOUT n integer, OUT type_record) AS $$
return (n, (n * 2, n * 3))
$$ LANGUAGE plpython3u;
@@ -147,6 +147,15 @@ SELECT * FROM multiout_table_type_setof('str', 'f', 6);
SELECT * FROM multiout_table_type_setof('str', 'f', 7);
SELECT * FROM multiout_table_type_setof('dict', 't', 3);
+-- But RECORD, rather than a named type, doesn't work in this context
+-- for lack of a way to identify its concrete type
+CREATE FUNCTION multiout_record(INOUT n integer, OUT record) AS $$
+return (n, (n * 2, n * 3))
+$$ LANGUAGE plpython3u;
+
+SELECT * FROM multiout_record(2);
+
+
-- check what happens if a type changes under us
CREATE TABLE changing (
diff --git a/src/pl/plpython/sql/plpython_record.sql b/src/pl/plpython/sql/plpython_record.sql
index 52bad8bccea..bc1e7d54c8a 100644
--- a/src/pl/plpython/sql/plpython_record.sql
+++ b/src/pl/plpython/sql/plpython_record.sql
@@ -132,6 +132,23 @@ ALTER TYPE type_record ADD ATTRIBUTE second int4;
SELECT * FROM test_type_record_as('obj', 'one', 1, false);
+-- check handling of record types defined by the calling query
+
+CREATE FUNCTION test_returns_record() RETURNS record AS $$
+ return (1, 2)
+$$ LANGUAGE plpython3u;
+
+SELECT * FROM test_returns_record() AS (a int, b int);
+SELECT * FROM test_returns_record(); -- fail
+
+-- alternate spelling of identical functionality
+CREATE FUNCTION test_returns_out_record(OUT r record) AS $$
+ return (1, 2)
+$$ LANGUAGE plpython3u;
+
+SELECT * FROM test_returns_out_record() AS (a int, b int);
+SELECT * FROM test_returns_out_record(); -- fail
+
-- errors cases
CREATE FUNCTION test_type_record_error1() RETURNS type_record AS $$
--
2.52.0
