Re: Review: UNNEST (and other functions) WITH ORDINALITY

From: David Fetter <david(at)fetter(dot)org>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>, Greg Stark <stark(at)mit(dot)edu>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Subject: Re: Review: UNNEST (and other functions) WITH ORDINALITY
Date: 2013-07-28 05:49:49
Message-ID: 20130728054949.GA31463@fetter.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 25, 2013 at 10:33:54AM -0700, David Fetter wrote:
> On Wed, Jul 24, 2013 at 09:38:15PM +0000, Andrew Gierth wrote:
> > Tom Lane said:
> > > If we did it with a WithOrdinality expression node, the result would
> > > always be of type RECORD, and we'd have to use blessed tuple
> > > descriptors to keep track of exactly which record type a particular
> > > instance emits. This is certainly do-able (see RowExpr for
> > > precedent).
> >
> > Maybe RowExpr is a precedent for something, but it has this
> > long-standing problem that makes it very hard to use usefully:
> >
> > postgres=# select (r).* from (select row(a,b) as r from (values (1,2)) v(a,b)) s;
> > ERROR: record type has not been registered
> >
> > > It seems way too short on comments. [...]
> >
> > This can certainly be addressed.
> >
> > > but it sure looks like it flat out removed several existing
> > > regression-test cases
> >
> > Here's why, in rangefuncs.sql:
> >
> > --invokes ExecReScanFunctionScan
> > SELECT * FROM foorescan f WHERE f.fooid IN (SELECT fooid FROM foorescan(5002,5004)) ORDER BY 1,2;
> >
> > I don't think that has invoked ExecReScanFunctionScan since 7.4 or so.
> > It certainly does not do so now (confirmed by gdb as well as by the
> > query plan). By all means keep the old tests if you want a
> > never-remove-tests-for-any-reason policy, but having added tests that
> > actually _do_ invoke ExecReScanFunctionScan, I figured the old ones
> > were redundant. (Also, these kinds of tests can be done a bit better
> > now with values and lateral rather than creating and dropping tables
> > just for the one test.)
> >
> > > and a few existing comments as well.
> >
> > I've double-checked, and I don't see any existing comments removed.
> >
> > > FWIW, I concur with the gripe I remember seeing upthread that the
> > > default name of the added column ought not be "?column?".
> >
> > This seems to be a common complaint, but gives rise to two questions:
> >
> > 1) what should the name be?
> >
> > 2) should users be depending on it?
> >
> > I've yet to find another db that actually documents a specific column
> > name for the ordinality column; it's always taken for granted that the
> > user should always be supplying an alias. (Admittedly there are not
> > many dbs that support it at all; DB2 does, and I believe Teradata.)
>
> Next patch: changes by Andrew Gierth, testing vs up-to-date git master
> by Yours Truly.

Greg,

Are you still on this? Do you have questions or concerns?

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Atri Sharma 2013-07-28 06:51:55 Re: replication_reserved_connections
Previous Message Amit kapila 2013-07-28 05:41:44 Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])