> On 9 Jan 2020, at 17:43, Daniel Verite <dan...@manitou-mail.org> wrote:
> 
> […]
> (using plain text instead of HTML messages might help with that).

Thanks. I’ll do that next time.

> […]
> * unnest-refcursor-v3.patch needs a slight rebase because this chunk
> in the Makefile fails
> -     regexp.o regproc.o ri_triggers.o rowtypes.o ruleutils.o \
> +     refcursor.o regexp.o regproc.o ri_triggers.o rowtypes.o ruleutils.o \

Likewise I’ll make that rebase in the next version.

> * I'm under the impression that refcursor_unnest() is functionally
> equivalent to a plpgsql implementation like this:
> 
> […]
> 
> but it would differ in performance, because internally a materialization step
> could be avoided, but only when the other patch "Allow FunctionScans to
> pipeline results"  gets in?

Yes. That’s at least true if unnest(x) is used in the FROM. If it’s used in the 
SELECT, actually it can get the performance benefit right away. However, in the 
SELECT case, there’s a bit of a gotcha because anonymous records can’t easily 
be manipulated because they have no type information available. So to make a 
useful performance contribution, it does need to be combined with another 
change — either to make it FROM pipeline as in my other patch, or perhaps 
enabling anonymous record types to be cast or otherwise manipulated.

> […]
> /*
> - * UNNEST
> + * UNNEST (array)
>  */
> 
> This chunk looks unnecessary?

It was for purpose of disambiguating. But indeed it is unnecessary. Choosing a 
different name would avoid need for it.

> * some user-facing doc would be needed.

Indeed. I fully intend that. I figured I’d get the concept on a firmer footing 
first.

> * Is it good to overload "unnest" rather than choosing a specific
> function name?

Yeah. I wondered about that. A couple of syntactically obvious ideas were:

SELECT … FROM TABLE (x) (which is what I think Oracle does, but is reserved)

SELECT … FROM CURSOR (x) (which seems likely to confuse, but, surprisingly, 
isn’t actually reserved)    

SELECT … FROM FETCH (x) (which I quite like, but is reserved)

SELECT … FROM ROWS_FROM (x) (is okay, but conflicts with our ROWS FROM 
construct)

So I kind of landed on UNNEST(x) out of lack of better idea. EXPAND(x) could be 
an option. Actually ROWS_IN(x) or ROWS_OF(x) might work.

Do you have any preference or suggestion?

Thanks a lot for the feedback.

denty.

Reply via email to