Re: UNNEST with multiple args, and TABLE with multiple funcs

From: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: pgsql-hackers(at)postgresql(dot)org, Boszormenyi Zoltan <zb(at)cybertec(dot)at>, Greg Stark <stark(at)mit(dot)edu>, David Fetter <david(at)fetter(dot)org>, Josh Berkus <josh(at)agliodbs(dot)com>
Subject: Re: UNNEST with multiple args, and TABLE with multiple funcs
Date: 2013-10-01 18:38:54
Message-ID: 524B16BE.90301@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I've spent some time reviewing this patch - looks pretty good! I'm not
through yet, but I wanted to post an update. Attached is a new version,
with some modifications I made. Notably:

I added a new struct to hold the per-function executor state - tupdesc,
tuplestore, rowcount and slot - instead of the lists that need to be
kept in sync. This is more readable.

I replaced the CreateTupleDescCopyMany() function with a function called
TupleDescCopyEntry(), which initializes a single attribute like
TupleDescInitEntry(), but copies the information from another tupledesc.
It is used in a loop to construct the composite tupledec in multiple
function or ordinality case. This is more flexible, no need to create
the dummy single-attribute TupleDesc for ordinality anymore, for example.

I refactored the grammar a bit; the way func_table rule returned a one-
or two element list to essentially pass up a flag was an ugly hack.

Below are a couple of more comments:

On 13.09.2013 22:03, Andrew Gierth wrote:
> ***************
> *** 3529,3534 **** static Expr *
> --- 3539,3545 ----
> simplify_function(Oid funcid, Oid result_type, int32 result_typmod,
> Oid result_collid, Oid input_collid, List **args_p,
> bool funcvariadic, bool process_args, bool allow_non_const,
> + FuncExpr *orig_funcexpr,
> eval_const_expressions_context *context)
> {
> List *args = *args_p;

The new argument needs to be explained in the comment above.

> ! <para>
> ! The special table function <literal>UNNEST</literal> may be called with
> ! any number of array parameters, and returns a corresponding number of
> ! columns, as if <literal>UNNEST</literal>
> ! (see <xref linkend="functions-array">) had been called on each parameter
> ! separately and combined using the <literal>TABLE</literal> construct. The
> ! number of result columns is determined by the sum of the arities of the
> ! array element types; that is to say, any array of composite type is
> ! expanded into separate result columns for each field of the type.
> ! Likewise, the number of rows returned is determined by the largest array
> ! parameter, with smaller values padded with NULLs.
> ! </para>

"Arities", really? :-). Please reword that into more plain English.

I'm going to put this aside for a day or two now, but will get back to
it later to finish the review and commit (unless someone beats me to
it). Meanwhile, if you could do something about that comment and manual
paragraph above, and re-review the changes I made, that would be great.

- Heikki

Attachment Content-Type Size
tablefunc-20131001-heikki-1.patch.gz application/x-gzip 36.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Gierth 2013-10-01 19:59:50 Re: UNNEST with multiple args, and TABLE with multiple funcs
Previous Message Andres Freund 2013-10-01 17:56:33 Re: logical changeset generation v6.1