From a5c6d33a199041c511d2a67362ef8bf54ff1e2d9 Mon Sep 17 00:00:00 2001 From: Tom Lane 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 Reviewed-by: shihao zhong Discussion: https://postgr.es/m/2865288.1793403201@sss.pgh.pa.us --- .../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