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

Hi,

Thanks for looking.

On 2016-09-02 14:01:32 +0530, Robert Haas wrote:
> > 6) SETOF record type functions cannot directly be used in ROWS FROM() -
> > as ROWS FROM "expands" records returned by functions. When converting
> > something like
> > CREATE OR REPLACE FUNCTION setof_record_sql() RETURNS SETOF record LANGUAGE sql AS $$SELECT 1 AS a, 2 AS b UNION ALL SELECT 1, 2;$$;
> > SELECT setof_record_sql();
> > we don't have that available though.
> >
> > The best way to handle that seems to be to introduce the ability for
> > ROWS FROM not to expand the record returned by a column. I'm currently
> > thinking that something like ROWS FROM(setof_record_sql() AS ()) would
> > do the trick. That'd also considerably simplify the handling of
> > functions returning known composite types - my current POC patch
> > generates a ROW(a,b,..) type expression for those.
> >
> > I'm open to better syntax suggestions.
>
> I definitely agree that having some syntax to avoid row-expansion in
> this case (and maybe in other cases) would be a good thing; I suspect
> that would get a good bit of use. I don't care much for that
> particular choice of syntax, which seems fairly magical, but I'm not
> sure what would be better.

I'm not a fan either, but until somebody ocmes up with something better
:/

> That sounds good.
> > I've implemented ([2]) a prototype of this. My basic approach is:
> >
> > I) During parse-analysis, remember whether a query has any tSRFs
> > (Query->hasTargetSRF). That avoids doing a useless pass over the
> > query, if no tSRFs are present.
> > II) At the beginning of subquery_planner(), before doing any work
> > operating on subqueries and such, implement SRFs if ->hasTargetSRF().
> > (unsrfify() in the POC)
> > III) Unconditionally move the "current" query into a subquery. For that
> > do a mutator pass over the query, replacing Vars/Aggrefs/... in the
> > original targetlist with Var references to the new subquery.
> > (unsrfify_reference_subquery_mutator() in the POC)
> > IV) Do a pass over the outer query's targetlist, and implement any tSRFs
> > using a ROWS FROM() RTE (or multiple ones in case of nested tSRFs).
> > (unsrfify_implement_srfs_mutator() in the POC)
> >
> > that seems to mostly work well.
>
> I gather that III and IV are skipped if hasTargetSRF isn't set.

Precisely.

> > d) Not a problem with the patch per-se, but I'm doubful that that's ok:
> > =# SELECT 1 ORDER BY generate_series(1, 10);
> > returns 10 rows ;) - maybe we should forbid that?
>
> OK by me. I feel like this isn't the only case where the presence of
> resjunk columns has user-visible effects, although I can't think of
> another one right at the moment. It seems like something to avoid,
> though.

An early patch in the series now errors out if ORDER BY or GROUP BY adds
a retset resjunk element.

Regards,

Andres