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

On 2017-01-19 13:06:20 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2017-01-18 16:56:46 -0500, Tom Lane wrote:
> >> Andres Freund <andres(at)anarazel(dot)de> 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