Thread: SRF patch (was Re: [HACKERS] Set Returning Functions (SRF) - request for patch review and comment)

Tom Lane wrote:
> Sure.  foo.foo is valid for a column foo in a table foo, so I don't
> see a problem with it for a function.

Fixed

>
> You could try doing the text substitution on the diff file and then
> re-applying the diff to fresh sources.  Might get a couple of merge
> failures, but should be a lot less painful than doing the edit directly
> on the full sources.
>

Great idea! Turned out to be a relatively painless 10 minute exercise.

> Up to you; probably should wait to see if Iter is still in your way
> after you do the other thing.  I think removing it and instead inserting
> returnsSet booleans in Oper and Func nodes would be a pretty
> straightforward exercise, but it'll mean touching even more stuff.
> Might be best to do that as a separate patch.

I'd like to wait on this -- I'm already drinking from a firehose ;-)


>
> Fair enough.  We should try to get the bulk of the patch applied soon
> so that you don't have code drift problems.  The rescan issues should
> not involve touching nearly as much code.

I also fixed the execute permissions, switched from ExecEvalFunc to
ExecEvalExpr, and fixed a bug that I found in _outRangeTblEntry (which
was preventing creation of a VIEW using a RangeFunction). If this could
be applied it would definitely help -- it's getting hard to keep it in
sync with cvs due to its size. The patch applies cleanly to cvs tip as
of a few minutes ago, and passes all regression tests.

Thanks,

Joe

Attachment

Re: SRF patch (was Re: [HACKERS] Set Returning Functions

From
Joe Conway
Date:
Joe Conway wrote:
 > Tom Lane wrote:
 >
 >> Sure.  foo.foo is valid for a column foo in a table foo, so I
 >> don't see a problem with it for a function.
 >
 > Fixed

Sorry -- when I fixed this, I introduced a new bug which only shows for
functions returning composite types, and of course I tested one
returning a base type :(

If you do apply the last srf patch, please apply this one over it.

Thanks,

Joe

*** pgsql/src/backend/parser/parse_relation.c    Tue May  7 21:54:14 2002
--- pgsql.1/src/backend/parser/parse_relation.c    Tue May  7 21:44:13 2002
***************
*** 738,750 ****
           */
          Relation    rel;
          int            maxattrs;
-         int            numaliases;

          rel = heap_open(funcrelid, AccessShareLock);

-         eref = (Alias *) copyObject(alias);
-         numaliases = length(eref->colnames);
-
          /*
           * Since the rel is open anyway, let's check that the number of column
           * aliases is reasonable.
--- 738,746 ----
***************
*** 777,791 ****
           * Must be a base data type, i.e. scalar.
           * Just add one alias column named for the function.
           */
!         char       *attrname;
!         Func       *func;
!         Oid            funcid;
!
!         func = (Func *) (expr->oper);
!         funcid = func->funcid;
!         attrname = get_func_name(funcid);
!
!         eref->colnames = lappend(eref->colnames, makeString(attrname));
          rte->eref = eref;
      }

--- 773,779 ----
           * Must be a base data type, i.e. scalar.
           * Just add one alias column named for the function.
           */
!         eref->colnames = lappend(eref->colnames, makeString(funcname));
          rte->eref = eref;
      }