Re: FmgrInfo allocation patterns (and PL handling as staged programming) - Mailing list pgsql-hackers

Chapman Flack <jcflack@acm.org> writes:
> On 04/06/25 20:01, Tom Lane wrote:
>> Looking more closely at ProcedureCreate(), it makes a dependency
>> if a transform *exists* for the argument or result type, whether
>> a TRANSFORM clause is present or not.  Surely this is completely
>> bogus?  We should be depending on the OIDs mentioned in protrftypes,

> I think that's it. I tested by creating a function like
> create function foo() returns text transform for type circle
> that is, with the transform type not appearing as an argument or
> return type.
> As far as I know, that's still a cromulent usage, as I could be saying
> I want the function to apply that transform to circles it uses or retrieves
> in queries it makes.

Oh, interesting interpretation.  The in-core PLs only consider the
transform list in relation to parameters and results, but I suppose
that's just because they have no use for internal conversions ...
and even then, you could argue that that's wrong and they should
apply the specified conversions when invoking, say, SQL queries
via SPI.  Something to think about another day.

Here's a draft patch to fix the bogus dependencies.  As given this'd
only be okay for HEAD, since I doubt we can get away with changing
ProcedureCreate()'s signature in stable branches.  Given the lack of
prior complaints, I'm content to fix this only in HEAD --- but maybe
somebody will think differently?  In the back branches we could make
ProcedureCreate() deconstruct the OID array it's given and then repeat
the transform lookups, but ugh.  I guess a "ProcedureCreateExt"
alternate entry point would do the trick too.

BTW, I feel a little uncomfortable with the idea that we're adding
dependencies on objects that are explicitly mentioned nowhere in the
pg_proc entry.  I suppose it's okay, and the preceding code did that
too, but still.

            regards, tom lane

diff --git a/contrib/bool_plperl/expected/bool_plperl.out b/contrib/bool_plperl/expected/bool_plperl.out
index 187df8db96f..183dc07b3fb 100644
--- a/contrib/bool_plperl/expected/bool_plperl.out
+++ b/contrib/bool_plperl/expected/bool_plperl.out
@@ -104,9 +104,9 @@ SELECT spi_test();

 DROP EXTENSION plperl CASCADE;
 NOTICE:  drop cascades to 6 other objects
-DETAIL:  drop cascades to function spi_test()
-drop cascades to extension bool_plperl
+DETAIL:  drop cascades to extension bool_plperl
 drop cascades to function perl2int(integer)
 drop cascades to function perl2text(text)
 drop cascades to function perl2undef()
 drop cascades to function bool2perl(boolean,boolean,boolean)
+drop cascades to function spi_test()
diff --git a/contrib/bool_plperl/expected/bool_plperlu.out b/contrib/bool_plperl/expected/bool_plperlu.out
index 8337d337e99..1496bbafac8 100644
--- a/contrib/bool_plperl/expected/bool_plperlu.out
+++ b/contrib/bool_plperl/expected/bool_plperlu.out
@@ -104,9 +104,9 @@ SELECT spi_test();

 DROP EXTENSION plperlu CASCADE;
 NOTICE:  drop cascades to 6 other objects
-DETAIL:  drop cascades to function spi_test()
-drop cascades to extension bool_plperlu
+DETAIL:  drop cascades to extension bool_plperlu
 drop cascades to function perl2int(integer)
 drop cascades to function perl2text(text)
 drop cascades to function perl2undef()
 drop cascades to function bool2perl(boolean,boolean,boolean)
+drop cascades to function spi_test()
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index bcf4050f5b1..a05f8a87c1f 100644
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -637,6 +637,7 @@ AggregateCreate(const char *aggName,
                              parameterNames,    /* parameterNames */
                              parameterDefaults, /* parameterDefaults */
                              PointerGetDatum(NULL), /* trftypes */
+                             NIL,    /* trfoids */
                              PointerGetDatum(NULL), /* proconfig */
                              InvalidOid,    /* no prosupport */
                              1, /* procost */
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 880b597fb3a..5fdcf24d5f8 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -26,7 +26,6 @@
 #include "catalog/pg_proc.h"
 #include "catalog/pg_transform.h"
 #include "catalog/pg_type.h"
-#include "commands/defrem.h"
 #include "executor/functions.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
@@ -61,6 +60,35 @@ static bool match_prosrc_to_literal(const char *prosrc, const char *literal,
 /* ----------------------------------------------------------------
  *        ProcedureCreate
  *
+ *    procedureName: string name of routine (proname)
+ *    procNamespace: OID of namespace (pronamespace)
+ *    replace: true to allow replacement of an existing pg_proc entry
+ *    returnsSet: returns set? (proretset)
+ *    returnType: OID of result type (prorettype)
+ *    proowner: OID of owner role (proowner)
+ *    languageObjectId: OID of function language (prolang)
+ *    languageValidator: OID of validator function to apply, if any
+ *    prosrc: string form of function definition (prosrc)
+ *    probin: string form of binary reference, or NULL (probin)
+ *    prosqlbody: Node tree of pre-parsed SQL body, or NULL (prosqlbody)
+ *    prokind: function/aggregate/procedure/etc code (prokind)
+ *    security_definer: security definer? (prosecdef)
+ *    isLeakProof: leak proof? (proleakproof)
+ *    isStrict: strict? (proisstrict)
+ *    volatility: volatility code (provolatile)
+ *    parallel: parallel safety code (proparallel)
+ *    parameterTypes: input parameter types, as an oidvector (proargtypes)
+ *    allParameterTypes: all parameter types, as an OID array (proallargtypes)
+ *    parameterModes: parameter modes, as a "char" array (proargmodes)
+ *    parameterNames: parameter names, as a text array (proargnames)
+ *    parameterDefaults: defaults, as a List of Node trees (proargdefaults)
+ *    trftypes: transformable type OIDs, as an OID array (protrftypes)
+ *    trfoids: List of transform OIDs that routine should depend on
+ *    proconfig: GUC set clauses, as a text array (proconfig)
+ *    prosupport: OID of support function, if any (prosupport)
+ *    procost: cost factor (procost)
+ *    prorows: estimated output rows for a SRF (prorows)
+ *
  * Note: allParameterTypes, parameterModes, parameterNames, trftypes, and proconfig
  * are either arrays of the proper types or NULL.  We declare them Datum,
  * not "ArrayType *", to avoid importing array.h into pg_proc.h.
@@ -90,6 +118,7 @@ ProcedureCreate(const char *procedureName,
                 Datum parameterNames,
                 List *parameterDefaults,
                 Datum trftypes,
+                List *trfoids,
                 Datum proconfig,
                 Oid prosupport,
                 float4 procost,
@@ -115,7 +144,6 @@ ProcedureCreate(const char *procedureName,
                 referenced;
     char       *detailmsg;
     int            i;
-    Oid            trfid;
     ObjectAddresses *addrs;

     /*
@@ -609,25 +637,18 @@ ProcedureCreate(const char *procedureName,
     ObjectAddressSet(referenced, TypeRelationId, returnType);
     add_exact_object_address(&referenced, addrs);

-    /* dependency on transform used by return type, if any */
-    if ((trfid = get_transform_oid(returnType, languageObjectId, true)))
-    {
-        ObjectAddressSet(referenced, TransformRelationId, trfid);
-        add_exact_object_address(&referenced, addrs);
-    }
-
     /* dependency on parameter types */
     for (i = 0; i < allParamCount; i++)
     {
         ObjectAddressSet(referenced, TypeRelationId, allParams[i]);
         add_exact_object_address(&referenced, addrs);
+    }

-        /* dependency on transform used by parameter type, if any */
-        if ((trfid = get_transform_oid(allParams[i], languageObjectId, true)))
-        {
-            ObjectAddressSet(referenced, TransformRelationId, trfid);
-            add_exact_object_address(&referenced, addrs);
-        }
+    /* dependency on transforms, if any */
+    foreach_oid(transformid, trfoids)
+    {
+        ObjectAddressSet(referenced, TransformRelationId, transformid);
+        add_exact_object_address(&referenced, addrs);
     }

     /* dependency on support function, if any */
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index b9fd7683abb..0335e982b31 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1046,6 +1046,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
     List       *parameterDefaults;
     Oid            variadicArgType;
     List       *trftypes_list = NIL;
+    List       *trfoids_list = NIL;
     ArrayType  *trftypes;
     Oid            requiredResultType;
     bool        isWindowFunc,
@@ -1157,11 +1158,12 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
             Oid            typeid = typenameTypeId(NULL,
                                                 lfirst_node(TypeName, lc));
             Oid            elt = get_base_element_type(typeid);
+            Oid            transformid;

             typeid = elt ? elt : typeid;
-
-            get_transform_oid(typeid, languageOid, false);
+            transformid = get_transform_oid(typeid, languageOid, false);
             trftypes_list = lappend_oid(trftypes_list, typeid);
+            trfoids_list = lappend_oid(trfoids_list, transformid);
         }
     }

@@ -1292,6 +1294,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
                            PointerGetDatum(parameterNames),
                            parameterDefaults,
                            PointerGetDatum(trftypes),
+                           trfoids_list,
                            PointerGetDatum(proconfig),
                            prosupport,
                            procost,
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 3cb3ca1cca1..45ae7472ab5 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1810,6 +1810,7 @@ makeRangeConstructors(const char *name, Oid namespace,
                                  PointerGetDatum(NULL), /* parameterNames */
                                  NIL,    /* parameterDefaults */
                                  PointerGetDatum(NULL), /* trftypes */
+                                 NIL,    /* trfoids */
                                  PointerGetDatum(NULL), /* proconfig */
                                  InvalidOid,    /* prosupport */
                                  1.0,    /* procost */
@@ -1875,6 +1876,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
                              PointerGetDatum(NULL), /* parameterNames */
                              NIL,    /* parameterDefaults */
                              PointerGetDatum(NULL), /* trftypes */
+                             NIL,    /* trfoids */
                              PointerGetDatum(NULL), /* proconfig */
                              InvalidOid,    /* prosupport */
                              1.0,    /* procost */
@@ -1919,6 +1921,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
                              PointerGetDatum(NULL), /* parameterNames */
                              NIL,    /* parameterDefaults */
                              PointerGetDatum(NULL), /* trftypes */
+                             NIL,    /* trfoids */
                              PointerGetDatum(NULL), /* proconfig */
                              InvalidOid,    /* prosupport */
                              1.0,    /* procost */
@@ -1957,6 +1960,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
                              PointerGetDatum(NULL), /* parameterNames */
                              NIL,    /* parameterDefaults */
                              PointerGetDatum(NULL), /* trftypes */
+                             NIL,    /* trfoids */
                              PointerGetDatum(NULL), /* proconfig */
                              InvalidOid,    /* prosupport */
                              1.0,    /* procost */
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 45f593fc958..d7353e7a088 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -211,6 +211,7 @@ extern ObjectAddress ProcedureCreate(const char *procedureName,
                                      Datum parameterNames,
                                      List *parameterDefaults,
                                      Datum trftypes,
+                                     List *trfoids,
                                      Datum proconfig,
                                      Oid prosupport,
                                      float4 procost,