Re: [HACKERS] Missing array support - Mailing list pgsql-patches

From Tom Lane
Subject Re: [HACKERS] Missing array support
Date
Msg-id 20848.1057018512@sss.pgh.pa.us
Whole thread Raw
In response to Re: [HACKERS] Missing array support  (Joe Conway <mail@joeconway.com>)
Responses Re: [HACKERS] Missing array support  (Joe Conway <mail@joeconway.com>)
List pgsql-patches
Joe Conway <mail@joeconway.com> writes:
> Attached is a patch that implements polymorphic aggregates.

> Included in the patch, I changed SQL language functions so that they
> could be declared with and use polymorphic types.

I've committed the polymorphic-SQL-functions part of this separately.

I didn't like the way you did it --- in particular, hacking
enforce_generic_type_consistency to allow generic types to be returned
strikes me as a very dangerous thing; it's supposed to be replacing them
all, not passing them back.  In any case it didn't get the job done,
since any but the most trivial function bodies would fail type
resolution at some point.  For example, I tried

create function array_sub(anyarray, int) returns anyelement as
'select $1[$2]' language sql;

and it failed with something along the lines of
    ERROR: transformArraySubscripts: type anyarray is not an array

What I've done instead is not to weaken type checking, but simply to
postpone all checking of the body of a SQL function to runtime if it
has any polymorphic arguments.  At runtime, we know the actual types
for the arguments, and we know the actual assigned result type, and
then we can run the normal checking operations without any problem.

Applied patch attached, just FYI.  (It still needs documentation
updates, which I trust you will supply later.)

Now back to looking at polymorphic aggregates...

            regards, tom lane

*** src/backend/catalog/pg_proc.c.orig    Sun Jun 15 13:59:10 2003
--- src/backend/catalog/pg_proc.c    Mon Jun 30 19:45:01 2003
***************
*** 33,39 ****
  #include "utils/syscache.h"


- static void checkretval(Oid rettype, char fn_typtype, List *queryTreeList);
  Datum        fmgr_internal_validator(PG_FUNCTION_ARGS);
  Datum        fmgr_c_validator(PG_FUNCTION_ARGS);
  Datum        fmgr_sql_validator(PG_FUNCTION_ARGS);
--- 33,38 ----
***************
*** 317,331 ****
  }

  /*
!  * checkretval() -- check return value of a list of sql parse trees.
   *
   * The return value of a sql function is the value returned by
!  * the final query in the function.  We do some ad-hoc define-time
!  * type checking here to be sure that the user is returning the
!  * type he claims.
   */
! static void
! checkretval(Oid rettype, char fn_typtype, List *queryTreeList)
  {
      Query       *parse;
      int            cmd;
--- 316,335 ----
  }

  /*
!  * check_sql_fn_retval() -- check return value of a list of sql parse trees.
   *
   * The return value of a sql function is the value returned by
!  * the final query in the function.  We do some ad-hoc type checking here
!  * to be sure that the user is returning the type he claims.
!  *
!  * This is normally applied during function definition, but in the case
!  * of a function with polymorphic arguments, we instead apply it during
!  * function execution startup.  The rettype is then the actual resolved
!  * output type of the function, rather than the declared type.  (Therefore,
!  * we should never see ANYARRAY or ANYELEMENT as rettype.)
   */
! void
! check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList)
  {
      Query       *parse;
      int            cmd;
***************
*** 472,478 ****

          relation_close(reln, AccessShareLock);
      }
!     else if (fn_typtype == 'p' && rettype == RECORDOID)
      {
          /* Shouldn't have a typerelid */
          Assert(typerelid == InvalidOid);
--- 476,482 ----

          relation_close(reln, AccessShareLock);
      }
!     else if (rettype == RECORDOID)
      {
          /* Shouldn't have a typerelid */
          Assert(typerelid == InvalidOid);
***************
*** 482,487 ****
--- 486,499 ----
           * tuple.
           */
      }
+     else if (rettype == ANYARRAYOID || rettype == ANYELEMENTOID)
+     {
+         /*
+          * This should already have been caught ...
+          */
+         elog(ERROR, "functions returning ANYARRAY or ANYELEMENT must " \
+              "have at least one argument of either type");
+     }
      else
          elog(ERROR, "return type %s is not supported for SQL functions",
               format_type_be(rettype));
***************
*** 505,511 ****
      Datum        tmp;
      char       *prosrc;

!     tuple = SearchSysCache(PROCOID, funcoid, 0, 0, 0);
      if (!HeapTupleIsValid(tuple))
          elog(ERROR, "cache lookup of function %u failed", funcoid);
      proc = (Form_pg_proc) GETSTRUCT(tuple);
--- 517,525 ----
      Datum        tmp;
      char       *prosrc;

!     tuple = SearchSysCache(PROCOID,
!                            ObjectIdGetDatum(funcoid),
!                            0, 0, 0);
      if (!HeapTupleIsValid(tuple))
          elog(ERROR, "cache lookup of function %u failed", funcoid);
      proc = (Form_pg_proc) GETSTRUCT(tuple);
***************
*** 544,550 ****
      char       *prosrc;
      char       *probin;

!     tuple = SearchSysCache(PROCOID, funcoid, 0, 0, 0);
      if (!HeapTupleIsValid(tuple))
          elog(ERROR, "cache lookup of function %u failed", funcoid);
      proc = (Form_pg_proc) GETSTRUCT(tuple);
--- 558,566 ----
      char       *prosrc;
      char       *probin;

!     tuple = SearchSysCache(PROCOID,
!                            ObjectIdGetDatum(funcoid),
!                            0, 0, 0);
      if (!HeapTupleIsValid(tuple))
          elog(ERROR, "cache lookup of function %u failed", funcoid);
      proc = (Form_pg_proc) GETSTRUCT(tuple);
***************
*** 585,622 ****
      Datum        tmp;
      char       *prosrc;
      char        functyptype;
      int            i;

!     tuple = SearchSysCache(PROCOID, funcoid, 0, 0, 0);
      if (!HeapTupleIsValid(tuple))
          elog(ERROR, "cache lookup of function %u failed", funcoid);
      proc = (Form_pg_proc) GETSTRUCT(tuple);

      functyptype = get_typtype(proc->prorettype);

!     /* Disallow pseudotypes in arguments and result */
!     /* except that return type can be RECORD or VOID */
      if (functyptype == 'p' &&
          proc->prorettype != RECORDOID &&
!         proc->prorettype != VOIDOID)
          elog(ERROR, "SQL functions cannot return type %s",
               format_type_be(proc->prorettype));

      for (i = 0; i < proc->pronargs; i++)
      {
          if (get_typtype(proc->proargtypes[i]) == 'p')
!             elog(ERROR, "SQL functions cannot have arguments of type %s",
!                  format_type_be(proc->proargtypes[i]));
      }

!     tmp = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_prosrc, &isnull);
!     if (isnull)
!         elog(ERROR, "null prosrc");
!
!     prosrc = DatumGetCString(DirectFunctionCall1(textout, tmp));
!
!     querytree_list = pg_parse_and_rewrite(prosrc, proc->proargtypes, proc->pronargs);
!     checkretval(proc->prorettype, functyptype, querytree_list);

      ReleaseSysCache(tuple);

--- 601,662 ----
      Datum        tmp;
      char       *prosrc;
      char        functyptype;
+     bool        haspolyarg;
      int            i;

!     tuple = SearchSysCache(PROCOID,
!                            ObjectIdGetDatum(funcoid),
!                            0, 0, 0);
      if (!HeapTupleIsValid(tuple))
          elog(ERROR, "cache lookup of function %u failed", funcoid);
      proc = (Form_pg_proc) GETSTRUCT(tuple);

      functyptype = get_typtype(proc->prorettype);

!     /* Disallow pseudotype result */
!     /* except for RECORD, VOID, ANYARRAY, or ANYELEMENT */
      if (functyptype == 'p' &&
          proc->prorettype != RECORDOID &&
!         proc->prorettype != VOIDOID &&
!         proc->prorettype != ANYARRAYOID &&
!         proc->prorettype != ANYELEMENTOID)
          elog(ERROR, "SQL functions cannot return type %s",
               format_type_be(proc->prorettype));

+     /* Disallow pseudotypes in arguments */
+     /* except for ANYARRAY or ANYELEMENT */
+     haspolyarg = false;
      for (i = 0; i < proc->pronargs; i++)
      {
          if (get_typtype(proc->proargtypes[i]) == 'p')
!         {
!             if (proc->proargtypes[i] == ANYARRAYOID ||
!                 proc->proargtypes[i] == ANYELEMENTOID)
!                 haspolyarg = true;
!             else
!                 elog(ERROR, "SQL functions cannot have arguments of type %s",
!                      format_type_be(proc->proargtypes[i]));
!         }
      }

!     /*
!      * We can't precheck the function definition if there are any polymorphic
!      * input types, because actual datatypes of expression results will be
!      * unresolvable.  The check will be done at runtime instead.
!      */
!     if (!haspolyarg)
!     {
!         tmp = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_prosrc, &isnull);
!         if (isnull)
!             elog(ERROR, "null prosrc");
!
!         prosrc = DatumGetCString(DirectFunctionCall1(textout, tmp));
!
!         querytree_list = pg_parse_and_rewrite(prosrc,
!                                               proc->proargtypes,
!                                               proc->pronargs);
!         check_sql_fn_retval(proc->prorettype, functyptype, querytree_list);
!     }

      ReleaseSysCache(tuple);

*** src/backend/executor/functions.c.orig    Thu Jun 12 13:29:26 2003
--- src/backend/executor/functions.c    Mon Jun 30 19:46:17 2003
***************
*** 24,29 ****
--- 24,30 ----
  #include "tcop/tcopprot.h"
  #include "tcop/utility.h"
  #include "utils/builtins.h"
+ #include "utils/lsyscache.h"
  #include "utils/syscache.h"


***************
*** 76,82 ****

  /* non-export function prototypes */
  static execution_state *init_execution_state(char *src,
!                      Oid *argOidVect, int nargs);
  static void init_sql_fcache(FmgrInfo *finfo);
  static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache);
  static TupleTableSlot *postquel_getnext(execution_state *es);
--- 77,84 ----

  /* non-export function prototypes */
  static execution_state *init_execution_state(char *src,
!                      Oid *argOidVect, int nargs,
!                      Oid rettype, bool haspolyarg);
  static void init_sql_fcache(FmgrInfo *finfo);
  static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache);
  static TupleTableSlot *postquel_getnext(execution_state *es);
***************
*** 90,96 ****


  static execution_state *
! init_execution_state(char *src, Oid *argOidVect, int nargs)
  {
      execution_state *firstes;
      execution_state *preves;
--- 92,99 ----


  static execution_state *
! init_execution_state(char *src, Oid *argOidVect, int nargs,
!                      Oid rettype, bool haspolyarg)
  {
      execution_state *firstes;
      execution_state *preves;
***************
*** 99,104 ****
--- 102,114 ----

      queryTree_list = pg_parse_and_rewrite(src, argOidVect, nargs);

+     /*
+      * If the function has any arguments declared as polymorphic types,
+      * then it wasn't type-checked at definition time; must do so now.
+      */
+     if (haspolyarg)
+         check_sql_fn_retval(rettype, get_typtype(rettype), queryTree_list);
+
      firstes = NULL;
      preves = NULL;

***************
*** 133,149 ****
--- 143,163 ----
  init_sql_fcache(FmgrInfo *finfo)
  {
      Oid            foid = finfo->fn_oid;
+     Oid            rettype;
      HeapTuple    procedureTuple;
      HeapTuple    typeTuple;
      Form_pg_proc procedureStruct;
      Form_pg_type typeStruct;
      SQLFunctionCachePtr fcache;
      Oid           *argOidVect;
+     bool        haspolyarg;
      char       *src;
      int            nargs;
      Datum        tmp;
      bool        isNull;

+     fcache = (SQLFunctionCachePtr) palloc0(sizeof(SQLFunctionCache));
+
      /*
       * get the procedure tuple corresponding to the given function Oid
       */
***************
*** 153,182 ****
      if (!HeapTupleIsValid(procedureTuple))
          elog(ERROR, "init_sql_fcache: Cache lookup failed for procedure %u",
               foid);
-
      procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple);

      /*
!      * get the return type from the procedure tuple
       */
      typeTuple = SearchSysCache(TYPEOID,
!                            ObjectIdGetDatum(procedureStruct->prorettype),
                                 0, 0, 0);
      if (!HeapTupleIsValid(typeTuple))
          elog(ERROR, "init_sql_fcache: Cache lookup failed for type %u",
!              procedureStruct->prorettype);
!
      typeStruct = (Form_pg_type) GETSTRUCT(typeTuple);

-     fcache = (SQLFunctionCachePtr) palloc0(sizeof(SQLFunctionCache));
-
      /*
       * get the type length and by-value flag from the type tuple
       */
      fcache->typlen = typeStruct->typlen;

!     if (typeStruct->typtype != 'c' &&
!         procedureStruct->prorettype != RECORDOID)
      {
          /* The return type is not a composite type, so just use byval */
          fcache->typbyval = typeStruct->typbyval;
--- 167,203 ----
      if (!HeapTupleIsValid(procedureTuple))
          elog(ERROR, "init_sql_fcache: Cache lookup failed for procedure %u",
               foid);
      procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple);

      /*
!      * get the result type from the procedure tuple, and check for
!      * polymorphic result type; if so, find out the actual result type.
       */
+     rettype = procedureStruct->prorettype;
+
+     if (rettype == ANYARRAYOID || rettype == ANYELEMENTOID)
+     {
+         rettype = get_fn_expr_rettype(finfo);
+         if (rettype == InvalidOid)
+             elog(ERROR, "could not determine actual result type for function declared %s",
+                  format_type_be(procedureStruct->prorettype));
+     }
+
+     /* Now look up the actual result type */
      typeTuple = SearchSysCache(TYPEOID,
!                                ObjectIdGetDatum(rettype),
                                 0, 0, 0);
      if (!HeapTupleIsValid(typeTuple))
          elog(ERROR, "init_sql_fcache: Cache lookup failed for type %u",
!              rettype);
      typeStruct = (Form_pg_type) GETSTRUCT(typeTuple);

      /*
       * get the type length and by-value flag from the type tuple
       */
      fcache->typlen = typeStruct->typlen;

!     if (typeStruct->typtype != 'c' && rettype != RECORDOID)
      {
          /* The return type is not a composite type, so just use byval */
          fcache->typbyval = typeStruct->typbyval;
***************
*** 205,221 ****
          fcache->funcSlot = NULL;

      /*
!      * Parse and plan the queries.  We need the argument info to pass
       * to the parser.
       */
      nargs = procedureStruct->pronargs;

      if (nargs > 0)
      {
          argOidVect = (Oid *) palloc(nargs * sizeof(Oid));
          memcpy(argOidVect,
                 procedureStruct->proargtypes,
                 nargs * sizeof(Oid));
      }
      else
          argOidVect = (Oid *) NULL;
--- 226,260 ----
          fcache->funcSlot = NULL;

      /*
!      * Parse and plan the queries.  We need the argument type info to pass
       * to the parser.
       */
      nargs = procedureStruct->pronargs;
+     haspolyarg = false;

      if (nargs > 0)
      {
+         int        argnum;
+
          argOidVect = (Oid *) palloc(nargs * sizeof(Oid));
          memcpy(argOidVect,
                 procedureStruct->proargtypes,
                 nargs * sizeof(Oid));
+         /* Resolve any polymorphic argument types */
+         for (argnum = 0; argnum < nargs; argnum++)
+         {
+             Oid        argtype = argOidVect[argnum];
+
+             if (argtype == ANYARRAYOID || argtype == ANYELEMENTOID)
+             {
+                 argtype = get_fn_expr_argtype(finfo, argnum);
+                 if (argtype == InvalidOid)
+                     elog(ERROR, "could not determine actual type of argument declared %s",
+                          format_type_be(argOidVect[argnum]));
+                 argOidVect[argnum] = argtype;
+                 haspolyarg = true;
+             }
+         }
      }
      else
          argOidVect = (Oid *) NULL;
***************
*** 229,235 ****
               foid);
      src = DatumGetCString(DirectFunctionCall1(textout, tmp));

!     fcache->func_state = init_execution_state(src, argOidVect, nargs);

      pfree(src);

--- 268,275 ----
               foid);
      src = DatumGetCString(DirectFunctionCall1(textout, tmp));

!     fcache->func_state = init_execution_state(src, argOidVect, nargs,
!                                               rettype, haspolyarg);

      pfree(src);

*** src/backend/optimizer/util/clauses.c.orig    Sat Jun 28 20:33:43 2003
--- src/backend/optimizer/util/clauses.c    Mon Jun 30 18:47:38 2003
***************
*** 1731,1736 ****
--- 1731,1737 ----
      int           *usecounts;
      List       *arg;
      int            i;
+     int            j;

      /*
       * Forget it if the function is not SQL-language or has other
***************
*** 1742,1752 ****
          funcform->pronargs != length(args))
          return NULL;

!     /* Forget it if declared return type is tuple or void */
      result_typtype = get_typtype(funcform->prorettype);
      if (result_typtype != 'b' &&
          result_typtype != 'd')
          return NULL;

      /* Check for recursive function, and give up trying to expand if so */
      if (oidMember(funcid, active_fns))
--- 1743,1761 ----
          funcform->pronargs != length(args))
          return NULL;

!     /* Forget it if declared return type is not base or domain */
      result_typtype = get_typtype(funcform->prorettype);
      if (result_typtype != 'b' &&
          result_typtype != 'd')
          return NULL;
+
+     /* Forget it if any declared argument type is polymorphic */
+     for (j = 0; j < funcform->pronargs; j++)
+     {
+         if (funcform->proargtypes[j] == ANYARRAYOID ||
+             funcform->proargtypes[j] == ANYELEMENTOID)
+             return NULL;
+     }

      /* Check for recursive function, and give up trying to expand if so */
      if (oidMember(funcid, active_fns))
*** src/backend/utils/adt/array_userfuncs.c.orig    Thu Jun 26 20:33:25 2003
--- src/backend/utils/adt/array_userfuncs.c    Mon Jun 30 18:40:03 2003
***************
*** 37,44 ****
      int16        typlen;
      bool        typbyval;
      char        typalign;
!     Oid            arg0_typeid = get_fn_expr_argtype(fcinfo, 0);
!     Oid            arg1_typeid = get_fn_expr_argtype(fcinfo, 1);
      Oid            arg0_elemid;
      Oid            arg1_elemid;
      ArrayMetaState *my_extra;
--- 37,44 ----
      int16        typlen;
      bool        typbyval;
      char        typalign;
!     Oid            arg0_typeid = get_fn_expr_argtype(fcinfo->flinfo, 0);
!     Oid            arg1_typeid = get_fn_expr_argtype(fcinfo->flinfo, 1);
      Oid            arg0_elemid;
      Oid            arg1_elemid;
      ArrayMetaState *my_extra;
*** src/backend/utils/adt/arrayfuncs.c.orig    Thu Jun 26 20:33:25 2003
--- src/backend/utils/adt/arrayfuncs.c    Mon Jun 30 18:40:03 2003
***************
*** 2792,2798 ****

      if (my_extra->srctype != src_elem_type)
      {
!         Oid            tgt_type = get_fn_expr_rettype(fcinfo);
          Oid            tgt_elem_type;
          Oid            funcId;

--- 2792,2798 ----

      if (my_extra->srctype != src_elem_type)
      {
!         Oid            tgt_type = get_fn_expr_rettype(fmgr_info);
          Oid            tgt_elem_type;
          Oid            funcId;

*** src/backend/utils/fmgr/fmgr.c.orig    Sat Jun 28 20:33:44 2003
--- src/backend/utils/fmgr/fmgr.c    Mon Jun 30 18:39:50 2003
***************
*** 1616,1631 ****

  /*-------------------------------------------------------------------------
   *        Support routines for extracting info from fn_expr parse tree
   *-------------------------------------------------------------------------
   */

  /*
!  * Get the OID of the function return type
   *
   * Returns InvalidOid if information is not available
   */
  Oid
! get_fn_expr_rettype(FunctionCallInfo fcinfo)
  {
      Node   *expr;

--- 1616,1634 ----

  /*-------------------------------------------------------------------------
   *        Support routines for extracting info from fn_expr parse tree
+  *
+  * These are needed by polymorphic functions, which accept multiple possible
+  * input types and need help from the parser to know what they've got.
   *-------------------------------------------------------------------------
   */

  /*
!  * Get the actual type OID of the function return type
   *
   * Returns InvalidOid if information is not available
   */
  Oid
! get_fn_expr_rettype(FmgrInfo *flinfo)
  {
      Node   *expr;

***************
*** 1633,1653 ****
       * can't return anything useful if we have no FmgrInfo or if
       * its fn_expr node has not been initialized
       */
!     if (!fcinfo || !fcinfo->flinfo || !fcinfo->flinfo->fn_expr)
          return InvalidOid;

!     expr = fcinfo->flinfo->fn_expr;

      return exprType(expr);
  }

  /*
!  * Get the type OID of a specific function argument (counting from 0)
   *
   * Returns InvalidOid if information is not available
   */
  Oid
! get_fn_expr_argtype(FunctionCallInfo fcinfo, int argnum)
  {
      Node   *expr;
      List   *args;
--- 1636,1656 ----
       * can't return anything useful if we have no FmgrInfo or if
       * its fn_expr node has not been initialized
       */
!     if (!flinfo || !flinfo->fn_expr)
          return InvalidOid;

!     expr = flinfo->fn_expr;

      return exprType(expr);
  }

  /*
!  * Get the actual type OID of a specific function argument (counting from 0)
   *
   * Returns InvalidOid if information is not available
   */
  Oid
! get_fn_expr_argtype(FmgrInfo *flinfo, int argnum)
  {
      Node   *expr;
      List   *args;
***************
*** 1657,1666 ****
       * can't return anything useful if we have no FmgrInfo or if
       * its fn_expr node has not been initialized
       */
!     if (!fcinfo || !fcinfo->flinfo || !fcinfo->flinfo->fn_expr)
          return InvalidOid;

!     expr = fcinfo->flinfo->fn_expr;

      if (IsA(expr, FuncExpr))
          args = ((FuncExpr *) expr)->args;
--- 1660,1669 ----
       * can't return anything useful if we have no FmgrInfo or if
       * its fn_expr node has not been initialized
       */
!     if (!flinfo || !flinfo->fn_expr)
          return InvalidOid;

!     expr = flinfo->fn_expr;

      if (IsA(expr, FuncExpr))
          args = ((FuncExpr *) expr)->args;
*** src/include/catalog/pg_proc.h.orig    Thu Jun 26 20:33:25 2003
--- src/include/catalog/pg_proc.h    Mon Jun 30 19:42:59 2003
***************
*** 3438,3441 ****
--- 3438,3444 ----
                  int parameterCount,
                  const Oid *parameterTypes);

+ extern void check_sql_fn_retval(Oid rettype, char fn_typtype,
+                                 List *queryTreeList);
+
  #endif   /* PG_PROC_H */
*** src/include/fmgr.h.orig    Thu Jun 26 10:18:56 2003
--- src/include/fmgr.h    Mon Jun 30 18:39:37 2003
***************
*** 378,385 ****
   */
  extern Pg_finfo_record *fetch_finfo_record(void *filehandle, char *funcname);
  extern Oid    fmgr_internal_function(const char *proname);
! extern Oid    get_fn_expr_rettype(FunctionCallInfo fcinfo);
! extern Oid    get_fn_expr_argtype(FunctionCallInfo fcinfo, int argnum);

  /*
   * Routines in dfmgr.c
--- 378,385 ----
   */
  extern Pg_finfo_record *fetch_finfo_record(void *filehandle, char *funcname);
  extern Oid    fmgr_internal_function(const char *proname);
! extern Oid    get_fn_expr_rettype(FmgrInfo *flinfo);
! extern Oid    get_fn_expr_argtype(FmgrInfo *flinfo, int argnum);

  /*
   * Routines in dfmgr.c

pgsql-patches by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: svedish trans
Next
From: Tom Lane
Date:
Subject: Re: Patch for listing runtime option details through server executable (pg_guc)