Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-08-13 Thread Robert Haas
On Tue, Aug 6, 2013 at 6:10 PM, Greg Stark st...@mit.edu wrote: The only other case I could come up with in my regression tests is pretty esoteric: CREATE COLLATION nulls (locale='C'); ALTER OPERATOR CLASS text_ops USING btree RENAME TO first; CREATE TABLE nulls_first(t text); CREATE INDEX

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-08-13 Thread Greg Stark
On Tue, Aug 13, 2013 at 8:20 PM, Robert Haas robertmh...@gmail.com wrote: Blech. Well, that's why we need to stop hacking the lexer before we shoot a hole through our foot that's too large to ignore. But it's not this patch's job to fix that problem. Hm. I thought it was. However it turns

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-08-06 Thread Greg Stark
On Mon, Aug 5, 2013 at 1:31 AM, Robert Haas robertmh...@gmail.com wrote: This looks like really nice work. It does. It's functionally equivalent to my attempt but with much better comments and cleaner code. But it doesn't seem to cover the case I was stumped on, namely nulls first appearing

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-08-06 Thread David Fetter
On Tue, Aug 06, 2013 at 11:10:11PM +0100, Greg Stark wrote: On Mon, Aug 5, 2013 at 1:31 AM, Robert Haas robertmh...@gmail.com wrote: This looks like really nice work. It does. It's functionally equivalent to my attempt but with much better comments and cleaner code. But it doesn't seem

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-08-04 Thread Robert Haas
On Mon, Jul 29, 2013 at 1:42 AM, Andrew Gierth and...@tao11.riddles.org.uk wrote: I propose the following patch (which goes on top of the current ordinality one) to implement the suggested grammar changes. I think this is the cleanest way, and I've tested that it both passes regression and

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-31 Thread Greg Stark
On Mon, Jul 29, 2013 at 8:45 PM, Josh Berkus j...@agliodbs.com wrote: Because this patch is still being discussed and tinkered with, I have moved it to 9.4CF2. Fwiw I already committed it. In the end I made only trivial changes the most significant of which was changing the column name to

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-31 Thread Alvaro Herrera
Greg Stark escribió: There are two followup changes that were discussed in this thread: 1) Changing the WITH_* and NULLS_* tokens to not eat the following token if it's not used by the grammar so that it doesn't interfere with syntax like select nulls first from t as a(nulls). I have

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-29 Thread Craig Ringer
On 07/25/2013 02:01 AM, Andres Freund wrote: And much of that can trivially/centrally be rewritten to LATERAL, not to speak of the cross-version compatibility problem. An interesting example of that can be found here: http://stackoverflow.com/q/12414750/398670 where someone's looking for a

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-29 Thread Vik Fearing
On 07/29/2013 09:56 AM, Craig Ringer wrote: Unless LATERAL provides a way to do lock-step iteration through a pair (or more) of functions I don't think we can get rid of SRFs-in-FROM just yet. I don't think anyone was arguing for that; we're talking about deprecating SRFs-in-SELECT. -- Sent

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-29 Thread Craig Ringer
On 07/29/2013 04:31 PM, Vik Fearing wrote: On 07/29/2013 09:56 AM, Craig Ringer wrote: Unless LATERAL provides a way to do lock-step iteration through a pair (or more) of functions I don't think we can get rid of SRFs-in-FROM just yet. I don't think anyone was arguing for that; we're

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-29 Thread Greg Stark
On Mon, Jul 29, 2013 at 8:56 AM, Craig Ringer cr...@2ndquadrant.com wrote: Unless LATERAL provides a way to do lock-step iteration through a pair (or more) of functions I don't think we can get rid of SRFs [in select target lists] yet You don't even need lateral. This works fine: postgres=#

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-29 Thread Craig Ringer
On 07/29/2013 08:02 PM, Greg Stark wrote: Unless LATERAL provides a way to do lock-step iteration through a pair (or more) of functions I don't think we can get rid of SRFs [in select target lists] yet You don't even need lateral. This works fine: postgres=# select * from

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-29 Thread Greg Stark
On Sun, Jul 28, 2013 at 7:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: I suspect it's likely to come out about the same either way once you account for all the uses of WITH. Might be worth trying both to see which seems less ugly. So I'm not really sure how to do it the other way. Once you're in

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-29 Thread Josh Berkus
All: Because this patch is still being discussed and tinkered with, I have moved it to 9.4CF2. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-28 Thread Greg Stark
On Sun, Jul 28, 2013 at 6:49 AM, David Fetter da...@fetter.org wrote: Are you still on this? Do you have questions or concerns? Still on this, I've just been a bit busy the past few days. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-28 Thread Greg Stark
On Wed, Jul 24, 2013 at 7:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: I don't see any workable fix that doesn't involve the funny token, though. Consider CREATE VIEW v AS SELECT ... FROM UNNEST(...) WITH ORDINALITY; CREATE VIEW v AS SELECT ... FROM UNNEST(...) WITH NO DATA; WITH ORDINALITY

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-28 Thread Tom Lane
Greg Stark st...@mit.edu writes: Instead of collapsing WITH TIME and WITH ORDINALITY into a single token why don't we just modify the WITH token to WITH_FOLLOWED_BY_TIME and WITH_FOLLOWED_BY_ORDINALITY but still keep the following token. Then we can just include those two tokens everywhere we

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-28 Thread Andrew Gierth
I propose the following patch (which goes on top of the current ordinality one) to implement the suggested grammar changes. I think this is the cleanest way, and I've tested that it both passes regression and allows constructs like WITH time AS (...) to work. -- Andrew (irc:RhodiumToad) ***

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-27 Thread David Fetter
On Thu, Jul 25, 2013 at 10:33:54AM -0700, David Fetter wrote: On Wed, Jul 24, 2013 at 09:38:15PM +, 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

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-25 Thread Robert Haas
On Wed, Jul 24, 2013 at 1:50 PM, Greg Stark st...@mit.edu wrote: On Wed, Jul 24, 2013 at 6:39 PM, Robert Haas robertmh...@gmail.com wrote: This patch will introduce, without documentation, a fifth class of keyword. ORDINALITY will need to be quoted when, and only when, it immediately follows

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Robert Haas
On Tue, Jul 23, 2013 at 9:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: If it weren't that we've been speculating for years about deprecating SRFs-in-tlists once we had LATERAL, I would personally consider this patch DOA in this form. If we do think we'll probably deprecate that feature, then not

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Jul 23, 2013 at 9:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: If it weren't that we've been speculating for years about deprecating SRFs-in-tlists once we had LATERAL, I would personally consider this patch DOA in this form. I guess I'd sort of

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Robert Haas
On Fri, Jul 19, 2013 at 1:50 PM, Greg Stark st...@mit.edu wrote: My only reservation with this patch is whether the WITH_ORDINALITY parser hack is the way we want to go. The precedent was already set with WITH TIME ZONE though and I think this was the best option. I share this reservation.

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Robert Haas
On Wed, Jul 24, 2013 at 1:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Jul 23, 2013 at 9:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: If it weren't that we've been speculating for years about deprecating SRFs-in-tlists once we had LATERAL, I would

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Greg Stark
On Wed, Jul 24, 2013 at 6:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: That seems to me to be unlikely to happen, because it would be impossible to preserve the current (admittedly bad) semantics. If we're going to change the behavior at all we might as well just drop the feature, IMO. It would

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Greg Stark
On Wed, Jul 24, 2013 at 6:39 PM, Robert Haas robertmh...@gmail.com wrote: This patch will introduce, without documentation, a fifth class of keyword. ORDINALITY will need to be quoted when, and only when, it immediately follows WITH. Without some change to our deparsing code, this is a

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Tom Lane
Greg Stark st...@mit.edu writes: On Wed, Jul 24, 2013 at 6:39 PM, Robert Haas robertmh...@gmail.com wrote: This patch will introduce, without documentation, a fifth class of keyword. ORDINALITY will need to be quoted when, and only when, it immediately follows WITH. Without some change to

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Andres Freund
On 2013-07-24 13:36:39 -0400, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Jul 23, 2013 at 9:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: If it weren't that we've been speculating for years about deprecating SRFs-in-tlists once we had LATERAL, I would personally consider

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-24 Thread Andrew Gierth
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

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Dean Rasheed
On 23 July 2013 06:10, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Gierth and...@tao11.riddles.org.uk writes: I must admit to finding all of this criticism of unread code a bit bizarre. If you yourself are still finding bugs in the code as of this afternoon, onlookers could be forgiven for

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Andrew Gierth
Tom Lane said: If you yourself are still finding bugs in the code as of this afternoon, onlookers could be forgiven for doubting that the code is quite as readable or ready to commit as all that. Right, and we all know that all code is perfect when committed. sheesh. (This is actually the

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Stephen Frost
Andrew, * Andrew Gierth (and...@tao11.riddles.org.uk) wrote: Right, and we all know that all code is perfect when committed. sheesh. That clearly wasn't what was claimed. (This is actually the first time in six months that I've had occasion to look at that part of the code; that's how long

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Greg Stark
On Tue, Jul 23, 2013 at 9:27 PM, Stephen Frost sfr...@snowman.net wrote: Fine- I'd have been as happy leaving this be and letting Greg commit it, but if you'd really like to hear my concerns, I'd start with pointing out that it's pretty horrid to have to copy every record around like this and

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Stephen Frost
* Greg Stark (st...@mit.edu) wrote: So given that WITH ORDINALITY is really only useful for UNNEST and we can generalize it to all SRFs on the basis that Postgres SRFs do produce ordered sets not unordered relations it isn't crazy for the work to be in the Function node. I agree, it isn't

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread David Fetter
On Tue, Jul 23, 2013 at 05:23:17PM -0400, Stephen Frost wrote: * Greg Stark (st...@mit.edu) wrote: So given that WITH ORDINALITY is really only useful for UNNEST and we can generalize it to all SRFs on the basis that Postgres SRFs do produce ordered sets not unordered relations it isn't

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Stephen Frost
David On Tuesday, July 23, 2013, David Fetter wrote: There are a lot of ways foreign tables don't yet act like local ones. Much as I'm a booster for fixing that problem, I'm thinking improvements in this direction are for a separate patch. This isn't about making FDWs more like local

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread David Fetter
On Tue, Jul 23, 2013 at 06:09:20PM -0400, Stephen Frost wrote: David On Tuesday, July 23, 2013, David Fetter wrote: There are a lot of ways foreign tables don't yet act like local ones. Much as I'm a booster for fixing that problem, I'm thinking improvements in this direction are for

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Stephen Frost
On Tuesday, July 23, 2013, David Fetter wrote: Are you saying that there's stuff that if I don't put it in now will impede our ability to add this to FTs later? I'm saying that it'd be a completely different implementation and that this one would get in the way and essentially have to be

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Andrew Gierth
Stephen Frost said: [stuff about foreign tables] I think the issue with foreign tables is probably moot because if you _did_ want to have some types of foreign tables with a fixed ordinality, you'd probably also want qual pushdown to work for it (i.e. so that WHERE rownum=123 doesn't have to

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-23 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Andrew Gierth (and...@tao11.riddles.org.uk) wrote: If you have legitimate technical concerns then let's hear them, now. Fine- I'd have been as happy leaving this be and letting Greg commit it, but if you'd really like to hear my concerns, I'd start

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-22 Thread Tom Lane
Greg Stark st...@mit.edu writes: I do find the logic and variable names a bit confusing so I'm tempted to add some comments based on my initial confusion. And I'm tempted to add an ordinalityAttNum field to the executor node so we don't need to make these odd scanslot means this unless we have

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-22 Thread Greg Stark
So the more I look at this patch the fewer things I want to change in it. I've several times thought I should make an improvement and then realized I was really just making unnecessary tweaks that didn't really make much difference. It seems a shame that the node has to call the function and get

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-22 Thread Greg Stark
On Mon, Jul 22, 2013 at 4:42 PM, Tom Lane t...@sss.pgh.pa.us wrote: I haven't read this patch, but that comment scares the heck out of me. Even if the patch isn't broken today, it will be tomorrow, if it's making random changes like that in data structure semantics. It's not making random

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-22 Thread Andrew Gierth
Greg Stark said: So the more I look at this patch the fewer things I want to change in it. I've several times thought I should make an improvement and then realized I was really just making unnecessary tweaks that didn't really make much difference. It's almost as though we actually thought

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-22 Thread Andrew Gierth
Tom Lane said: I haven't read this patch, but that comment scares the heck out of me. Even if the patch isn't broken today, it will be tomorrow, if it's making random changes like that in data structure semantics. Also, if you're confused, so will be everybody else who has to deal with the

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-22 Thread Tom Lane
Andrew Gierth and...@tao11.riddles.org.uk writes: I must admit to finding all of this criticism of unread code a bit bizarre. If you yourself are still finding bugs in the code as of this afternoon, onlookers could be forgiven for doubting that the code is quite as readable or ready to commit

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-19 Thread Greg Stark
On Wed, Jun 26, 2013 at 2:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Some of the rest of us would like to hear those reasons, because my immediate reaction is that the patch must be broken by design. WITH ORDINALITY should not be needing to mess with the fundamental evaluation semantics of

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-05 Thread Vik Fearing
On 07/04/2013 10:15 AM, Dean Rasheed wrote: On 4 July 2013 00:08, David Fetter da...@fetter.org wrote: Patch re-jiggered for recent changes to master. I re-validated this, and it all still looks good, so still ready for committer IMO. I tried to check this out, too and make check fails with

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-05 Thread David Fetter
On Fri, Jul 05, 2013 at 04:58:30PM +0200, Vik Fearing wrote: On 07/05/2013 04:51 PM, Vik Fearing wrote: I tried to check this out, too and make check fails with the following. I have not looked into the cause. Running ./configure again fixed it. Sorry for the noise. If I had a nickel

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-05 Thread Vik Fearing
On 07/05/2013 04:51 PM, Vik Fearing wrote: I tried to check this out, too and make check fails with the following. I have not looked into the cause. Running ./configure again fixed it. Sorry for the noise. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-07-04 Thread Dean Rasheed
On 4 July 2013 00:08, David Fetter da...@fetter.org wrote: Patch re-jiggered for recent changes to master. I re-validated this, and it all still looks good, so still ready for committer IMO. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-06-26 Thread Dean Rasheed
On 26 June 2013 01:22, Josh Berkus j...@agliodbs.com wrote: Folks, (the below was already discussed on IRC) Leaving names aside on this patch, I'm wondering about a piece of functionality I have with the current unnest() and with the unnest_ordinality()[1] extension: namely, the ability to

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-06-25 Thread Josh Berkus
Folks, (the below was already discussed on IRC) Leaving names aside on this patch, I'm wondering about a piece of functionality I have with the current unnest() and with the unnest_ordinality()[1] extension: namely, the ability to unnest several arrays in parallel by using unnest() in the target

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-06-25 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes: ... and if arr1, 2 and 3 are exactly the same length, this creates a coordinated dataset. I can even use the unnest_ordinality() extension function to get the ordinality of this combined dataset: SELECT id,

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-06-24 Thread Dean Rasheed
On 21 June 2013 08:31, Dean Rasheed dean.a.rash...@gmail.com wrote: On 21 June 2013 08:02, Dean Rasheed dean.a.rash...@gmail.com wrote: On 21 June 2013 06:54, David Fetter da...@fetter.org wrote: For example SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS file The spec is pretty specific

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-06-21 Thread Dean Rasheed
On 21 June 2013 06:54, David Fetter da...@fetter.org wrote: For example SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS file The spec is pretty specific about the all or none nature of naming in the AS clause...unless we can figure out a way of getting around it somehow. We already support

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-06-21 Thread Dean Rasheed
On 21 June 2013 08:02, Dean Rasheed dean.a.rash...@gmail.com wrote: On 21 June 2013 06:54, David Fetter da...@fetter.org wrote: For example SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS file The spec is pretty specific about the all or none nature of naming in the AS clause...unless we can

Re: [HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-06-19 Thread Dean Rasheed
On 19 June 2013 04:11, David Fetter da...@fetter.org wrote: On Tue, Jun 18, 2013 at 11:36:08AM +0100, Dean Rasheed wrote: On 17 June 2013 06:33, David Fetter da...@fetter.org wrote: Next revision of the patch, now with more stability. Thanks, Andrew! Rebased vs. git master. Here's my

[HACKERS] Review: UNNEST (and other functions) WITH ORDINALITY

2013-06-18 Thread Dean Rasheed
On 17 June 2013 06:33, David Fetter da...@fetter.org wrote: Next revision of the patch, now with more stability. Thanks, Andrew! Rebased vs. git master. Here's my review of the WITH ORDINALITY patch. Overall I think that the patch is in good shape, and I think that this will be a very