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

On September 13, 2016 9:07:35 AM PDT, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>Andres Freund <andres(at)anarazel(dot)de> writes:
>> Attached is a significantly updated patch series (see the mail one up
>> for details about what this is, I don't want to quote it in its
>> entirety).
>
>I've reviewed the portions of 0005 that have to do with making the
>parser
>mark queries with hasTargetSRF. The code as you had it was wrong
>because
>it would set the flag as a consequence of SRFs in function RTEs, which
>we don't want.

I'd taken it more as the possibility that there's an srf, than guarantee so far. There might be cases where the planner removes the srf during folding or such. Makes sense to make it more accurate.

> It seemed to me that the best way to fix that was to
>rely
>on the parser's p_expr_kind mechanism to tell which part of the query
>we're in, whereupon we might as well make the parser act more like it
>does
>for aggregates and window functions and give a suitable error at parse
>time for misplaced SRFs.

That's a nice improvement. The execution time errors are ugly.

>I also renamed the flag to hasTargetSRFs, which is more parallel to
>hasAggs and hasWindowFuncs, and made some effort to use it in place
>of expression_returns_set() searches.
>
>I'd like to go ahead and push this, since it's a necessary prerequisite
>for either approach we might adopt for the rest of the patch series,
>and the improved error reporting and avoidance of expensive
>expression_returns_set searches make it a win IMO even if we were not
>planning to do anything more with SRFs.

Can't look are the code just now, on my way to the airport for pgopen, but the idea sounds good to me.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.