Re: UNNEST with multiple args, and TABLE with multiple funcs - Mailing list pgsql-hackers

Boszormenyi Zoltan wrote:
>> This parser hackery is of course somewhat ugly. But given the objective
>> of implementing the spec's unnest syntax, it seems to be the least ugly
>> of the possible approaches. (The hard part of doing it any other way
>> would be generating the description of the result type; composite array
>> parameters expand into multiple result columns.)
>
> Harder maybe but it may still be cleaner in the long run.

I'm not so sure.

As far as I'm concerned, though, the situation is fairly simple: there
are no proposals on the table for any mechanism that would allow the
deduction of a return type structure for multi-arg unnest, I have
tried and failed to come up with a usable alternative proposal, and
there is no prospect of one resulting from any other work that I know
about. So the parser hack is the way it goes, and anyone who doesn't
like it is welcome to suggest a better idea.

>> Overall, it's my intention here to remove as many as feasible of the old
>> reasons why one might use an SRF in the select list.
>
> Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
> WITH ORDINALITY and this feature, I would vote for removing
> SRF-in-targetlist and call the release PostgreSQL 10.0.

I want to make this ABSOLUTELY clear: I am not advocating removing
SRF-in-targetlist in the near future and I will not support anyone who
does. Please do not use this code as an argument for that (at least
until a few releases have elapsed). All I'm interested in at this
point is providing an alternative with better semantics.

> The SQL spec says these:
>
> In 7.6 <table reference>
[mega-snip]
>
> As far as I can tell, these should also be allowed but isn't:

No, they're not allowed. You missed this rule (in 7.6 Syntax Rules):

2) If a <table primary> TP simply contains a <table function derived  table> TFDT, then:
  a) The <collection value expression> immediately contained in TFDT     shall be a <routine invocation>.

In other words, func(...) is the only allowed form for the collection
value expression inside TABLE( ).  (Same rule exists in 201x, but
numbered 6 rather than 2.)

Largely as a matter of consistency, the patch does presently allow
expressions that are not <routine invocation>s but which are part of
the func_expr_windowless production, so things like TABLE(USER) work.
(This is because historically these are allowed in the FROM clause as
tables.) I'm not sure this is a good idea in general; should it be
tightened up to only allow func_application?

> * If it claims to improve performance, does it?
>
> It certainly improves writing queries, as functions inside
> unnest() get processed in one scan.

I'm not making any specific performance claims, but I have tested it
against the idea of doing separate function scans with a full join on
ordinality columns, and my approach is faster (1.5x to 2x in my tests)
even with pathkey support and with elimination of extra materialize
nodes (by allowing mark/restore in FunctionScan).

------

Since the original patch was posted I have done further work on it,
including some tests. I have also come up with an additional
possibility: that of allowing multiple SRFs that return RECORD with
column definition lists, and SRFs-returning-RECORD combined with
ORDINALITY, by extending the syntax further:

select * from TABLE(func1() AS (a text, b integer),                   func2() AS (c integer, d text));

select * from TABLE(func1() AS (a text, b integer))             WITH ORDINALITY AS f(a,b,o);

-- shame to have to duplicate the column names, but avoiding that would
-- not have been easy

This removes the restriction of the previous ORDINALITY patch that
prevented its use with SRFs that needed coldef lists.

I'm open to other suggestions on the syntax for this.

(My implementation of this works by making the column definition list
a property of the function call, rather than of the RTE or the
FunctionScan node.  This eliminates a few places where TYPEFUNC_RECORD
had to be handled as a special case.)

-- 
Andrew. (irc:RhodiumToad)