Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (wasChanged SRF in targetlist handling)

On 2017-01-19 13:06:20 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2017-01-18 16:56:46 -0500, Tom Lane wrote:
> >> Andres Freund  writes:
> >> I have not actually looked at 0003 at all yet.  So yeah, please post
> >> for review after you're done rebasing.
>
> > Here's a rebased and lightly massaged version.
>
> I've read through this and made some minor improvements, mostly additional
> comment cleanup.

Thanks!


> One thing I wanted to ask about:
>
> @@ -4303,7 +4303,7 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
>
>      /*
>       * Forget it if the function is not SQL-language or has other showstopper
> -     * properties.  (The nargs check is just paranoia.)
> +     * properties.  (The nargs and retset checks are just paranoia.)
>       */
>      if (funcform->prolang != SQLlanguageId ||
>          funcform->prosecdef ||
>
> I thought this change was simply wrong, and removed it;

Hm. I made that change a while ago.  It might have been a holdover from
the old approach, where it'd indeed have been impossible to see any
tSRFs here.  Or it might have been because we check
querytree->hasTargetSRFs below (which should prevent inlining a function
that actually returns multiple rows).  I agree it's better to leave the
check there. Maybe we ought to remove the paranoia bit about retset
though - it's not paranoia if it has an effect.


> Other than that possible point, I think the attached is committable.

Will do so in a bit, after a s/and retset checks are/check is/. And then
fix that big-endian ordering issue.

- Andres