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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, 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-11-19 16:13:26
Message-ID: 10034.1384877606@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
>> Here is a new patch with the following changes on top of Heikki's
>> version (all the changes in which I've otherwise kept):

> Here is an updated version:

I've been hacking on this patch all day yesterday. What I'm on about at
the moment is reversing the decision to move range functions' funccoltypes
etc into FuncExpr. That's a bad idea on the grounds of bloating FuncExpr,
but the real problem with it is this: what happens if the planner decides
to inline or const-simplify the function expression? You just lost a
critical part of the RTE's infrastructure, that's what. So that's got to
go, and I've been fooling with different ways to represent the info for
multiple functions within RangeTblEntry. What I have at the moment is

/*
* Fields valid for a function RTE (else NIL/zero):
*
* There can be multiple function expressions in a function RTE.
* funccoldeflist is an integer list (of the same length as funcexprs)
* containing true if function had a column definition list, else false.
* funccolcounts is an integer list (of the same length as funcexprs)
* showing the number of RTE output columns produced by each function.
* The length of eref->colnames must be equal to either the sum of the
* funccolcounts entries, or one more than the sum if funcordinality is
* true. funccoltypes, funccoltypmods, and funccolcollations give type
* information about each output column (these lists must have the same
* length as eref->colnames). Remember that when a function returns a
* named composite type, any dropped columns in that type will have dummy
* corresponding entries in these lists.
*
* Note: funccoltypes etc are derived from either the functions' declared
* result types, or their column definition lists in case of functions
* returning RECORD. Storing this data in the RTE is redundant in the
* former case, but for simplicity we store it always.
*/
List *funcexprs; /* expression trees for func calls */
List *funccoldeflist; /* integer list of has-coldeflist booleans */
List *funccolcounts; /* number of output columns from each func */
List *funccoltypes; /* OID list of column type OIDs */
List *funccoltypmods; /* integer list of column typmods */
List *funccolcollations; /* OID list of column collation OIDs */
bool funcordinality; /* is this called WITH ORDINALITY? */

which has the advantage that the ordinality column is no longer such a
special case, it's right there in the lists. However, it turns out that
in most places where I thought we could just consult the entry-per-column
lists, we can't. We still have to do the get_expr_result_type() dance,
because we need up-to-date information about which columns of a
composite-returning function's output have been dropped since the RTE was
made. That means we'd have to chase the entry-per-function lists in
parallel with the entry-per-column lists, which is a PITA.

I'm thinking possibly it's worth inventing a new Node type that would just
be infrastructure for RTE_FUNCTION RTEs, so that we'd have something like
this in RangeTblEntry:

List *functions; /* List of RangeTblFunction nodes */
bool funcordinality; /* is this called WITH ORDINALITY? */

and a node type RangeTblFunction with fields

Node *funcexpr; /* executable expression for the function */
int funccolcount; /* number of columns emitted by function */
/* These lists are NIL unless function had a column definition list: */
List *funccoltypes; /* OID list of column type OIDs */
List *funccoltypmods; /* integer list of column typmods */
List *funccolcollations; /* OID list of column collation OIDs */

BTW, the reason we need to store the column count explicitly is that we
have to ignore the added columns if a composite type has had an ADD COLUMN
done to it since the RTE was made. The submitted patch fails rather
nastily in such cases, if the composite type isn't last in the function
list.

Thoughts, better ideas?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-11-19 16:14:37 Re: additional json functionality
Previous Message Andrew Dunstan 2013-11-19 16:07:53 Re: Review: pre-commit triggers