Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

Andres Freund <andres@anarazel.de> writes:
> 0003-Avoid-materializing-SRFs-in-the-FROM-list.patch
>   To avoid performance regressions from moving SRFM_ValuePerCall SRFs to
>   ROWS FROM, nodeFunctionscan.c needs to support not materializing
>   output.

Personally I'd put this one later, as it's a performance optimization not
part of the core patch IMO --- or is there something in the later ones
that directly depends on it?  Anyway I'm setting it aside for now.

> 0004-Allow-ROWS-FROM-to-return-functions-as-single-record.patch
>   To allow transforming SELECT record_srf(); nodeFunctionscan.c needs to
>   learn to return the result as a record.  I chose
>   ROWS FROM (record_srf() AS ()) as the syntax for that. It doesn't
>   necessarily have to be SQL exposed, but it does make testing easier.

The proposed commit message is wrong, as it claims aclexplode()
demonstrates the problem which it doesn't --- we get the column set
from the function's declared OUT parameters.

I can't say that I like the proposed syntax much.  What about leaving out
any syntax changes, and simply saying that "if the function returns record
and hasn't got OUT parameters, then return its result as an unexpanded
record"?  That might not take much more than removing the error check ;-)

A possible objection is that then you could not get the no-expansion
behavior for functions that return named composite types or have OUT
parameters that effectively give them known composite types.  From a
semantic standpoint we could fix that by saying "just cast the result to
record", ie ROWS FROM (aclexplode('whatever')::record) would give the
no-expansion behavior.  I'm not sure if there might be any implementation
glitches in the way of doing it like that.  Also there seems to be some
syntactic issue with it ... actually, the current behavior there is just
weird:

regression=# select * from rows from (aclexplode('{=r/postgres}')::record); 
ERROR:  syntax error at or near "::"
LINE 1: ...lect * from rows from (aclexplode('{=r/postgres}')::record);
          ^
 
regression=# select * from rows from (cast(aclexplode('{=r/postgres}') as record));grantor | grantee | privilege_type |
is_grantable
 
---------+---------+----------------+--------------     10 |       0 | SELECT         | f
(1 row)

I was not aware that there was *anyplace* in the grammar where :: and CAST
behaved differently, and I'm not very pleased to find this one.

I haven't looked at the code, as there probably isn't much point in
reviewing in any detail till we choose the syntax.
        regards, tom lane