Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-19 Thread Tom Lane
Andres Freund writes: > Maybe we ought to remove the paranoia bit about retset > though - it's not paranoia if it has an effect. Exactly, and I already did that in my version. regards, tom lane -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-19 Thread Andres Freund
On 2017-01-19 13:06:20 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2017-01-18 16:56:46 -0500, Tom Lane wrote: > >> Andres Freund writes: > >> I have not actually looked at 0003 at all yet. So yeah, please post > >> for review after you're done

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-19 Thread Tom Lane
Andres Freund writes: > On 2017-01-18 16:56:46 -0500, Tom Lane wrote: >> Andres Freund writes: >> I have not actually looked at 0003 at all yet. So yeah, please post >> for review after you're done rebasing. > Here's a rebased and lightly massaged

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Tom Lane
Robert Haas writes: > So, one of the big reasons I use CASE is to avoid evaluating > expressions in cases where they might throw an ERROR. Like, you know: > CASE WHEN d != 0 THEN n / d ELSE NULL END > I guess it's not the end of the world if that only works for >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Robert Haas
On Wed, Jan 18, 2017 at 7:00 PM, Andres Freund wrote: >>So, one of the big reasons I use CASE is to avoid evaluating >>expressions in cases where they might throw an ERROR. Like, you know: >> >>CASE WHEN d != 0 THEN n / d ELSE NULL END >> >>I guess it's not the end of the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
On January 18, 2017 3:59:00 PM PST, Robert Haas wrote: >On Wed, Jan 18, 2017 at 6:19 PM, Andres Freund >wrote: >>> SELECT x, CASE WHEN x > 0 THEN generate_series(1, 5) ELSE 0 END >FROM tab; >>> >>> It might seem that this should produce five

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Robert Haas
On Wed, Jan 18, 2017 at 6:19 PM, Andres Freund wrote: >> SELECT x, CASE WHEN x > 0 THEN generate_series(1, 5) ELSE 0 END FROM tab; >> >> It might seem that this should produce five repetitions of input rows >> that have x > 0, and a single repetition of those that do

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
On 2017-01-18 16:27:53 -0700, David G. Johnston wrote: > ​I'd rather fail now and allow for the possibility of future implementation > of the "it might seem that..." behavior.​ That's very unlikely to happen. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread David G. Johnston
On Wed, Jan 18, 2017 at 4:14 PM, Tom Lane wrote: > I wrote: > > I'll try to write something about the SRF-in-CASE issue too. Seeing > > whether we can document that adequately seems like an important part > > of making the decision about whether we need to block it. > >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
On 2017-01-18 18:14:26 -0500, Tom Lane wrote: > I wrote: > > I'll try to write something about the SRF-in-CASE issue too. Seeing > > whether we can document that adequately seems like an important part > > of making the decision about whether we need to block it. > > Here's what I came up with:

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Tom Lane
I wrote: > I'll try to write something about the SRF-in-CASE issue too. Seeing > whether we can document that adequately seems like an important part > of making the decision about whether we need to block it. Here's what I came up with: This behavior also means that set-returning functions

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
On 2017-01-18 17:34:56 -0500, Tom Lane wrote: > Andres Freund writes: > > (I also noticed the previous patch should have had a catversion bump :(, > > will do after the meeting). > > Uh, why? It isn't touching any on-disk data structure. Forget what I said - I was rushing

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Tom Lane
Andres Freund writes: > (I also noticed the previous patch should have had a catversion bump :(, > will do after the meeting). Uh, why? It isn't touching any on-disk data structure. regards, tom lane -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
On 2017-01-18 16:56:46 -0500, Tom Lane wrote: > Andres Freund writes: > I have not actually looked at 0003 at all yet. So yeah, please post > for review after you're done rebasing. Here's a rebased and lightly massaged version. I'm vanishing in a meeting for a bit, thought

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Tom Lane
Andres Freund writes: > There's one sgml comment you'd added: > "Furthermore, nested set-returning functions did not work at all." > I'm not quite sure what you're referring to there - it was previously > allowed to have one set argument to an SRF: Ooops ... that was composed

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
Hi, On 2017-01-18 15:24:32 -0500, Tom Lane wrote: > Andres Freund writes: > > Yea, have something lying around. Let me push it then when I get back from > > lunch? > > Sure, no sweat. Pushed. Yay! There's one sgml comment you'd added: "Furthermore, nested set-returning

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Tom Lane
Andres Freund writes: > Yea, have something lying around. Let me push it then when I get back from > lunch? Sure, no sweat. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
Hi, On January 18, 2017 12:00:12 PM PST, Tom Lane wrote: >Andres Freund writes: >> On 2017-01-18 14:24:12 -0500, Tom Lane wrote: >>> So I think we can push this patch now and get on with the downstream >>> patches. Do you want to do the honors, or shall

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Tom Lane
Andres Freund writes: > On 2017-01-18 14:24:12 -0500, Tom Lane wrote: >> So I think we can push this patch now and get on with the downstream >> patches. Do you want to do the honors, or shall I? > Whatever you prefer - either way, I'll go on to rebasing the cleanup > patch

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
Hi, On 2017-01-18 14:24:12 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2017-01-18 08:43:24 -0500, Tom Lane wrote: > >> ... except for one thing. The more I look at it, > >> the more disturbed I am by the behavioral change shown in rangefuncs.out > >> --- that's the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Tom Lane
Andres Freund writes: > On 2017-01-18 08:43:24 -0500, Tom Lane wrote: >> ... except for one thing. The more I look at it, >> the more disturbed I am by the behavioral change shown in rangefuncs.out >> --- that's the SRF-in-one-arm-of-CASE issue. > I'm fine with leaving it as

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Andres Freund
Hi, On 2017-01-18 08:43:24 -0500, Tom Lane wrote: > I did a review pass over 0001 and 0002. I think the attached updated > version is committable Cool. > ... except for one thing. The more I look at it, > the more disturbed I am by the behavioral change shown in rangefuncs.out > --- that's

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-18 Thread Tom Lane
I did a review pass over 0001 and 0002. I think the attached updated version is committable ... except for one thing. The more I look at it, the more disturbed I am by the behavioral change shown in rangefuncs.out --- that's the SRF-in-one-arm-of-CASE issue. (The changes in tsrf.out are fine

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Tom Lane
Andres Freund writes: > On 2017-01-17 13:43:38 -0500, Tom Lane wrote: >> I'm not convinced that that optimization is worth preserving, but if we >> keep it then ProjectSet isn't le mot juste here, any more than you'd want >> to rename Result to Project without changing its

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Andres Freund
On 2017-01-17 13:43:38 -0500, Tom Lane wrote: > Although ... looking closer at Andres' patch, the new node type *is* > channeling Result, in the sense that it might or might not have any input > plan. This probably traces to what I wrote in September: > > + * XXX Possibly-temporary hack: if

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Tom Lane
Robert Haas writes: > On Tue, Jan 17, 2017 at 1:18 PM, Tom Lane wrote: >> Using Result for two completely different things is a wart though. If we >> had it to do over I think we'd define Result as a scan node that produces >> rows from no input, and

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Robert Haas
On Tue, Jan 17, 2017 at 1:18 PM, Tom Lane wrote: > Andres Freund writes: >> I'd not have gone for SetResult if we didn't already have Result. I'm >> not super happy ending up having Project in ProjectSet but not in the >> Result that end up doing the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Tom Lane
Andres Freund writes: > I'd not have gone for SetResult if we didn't already have Result. I'm > not super happy ending up having Project in ProjectSet but not in the > Result that end up doing the majority of the projection. But eh, we can > live with it. Using Result for

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Andres Freund
Hi, On 2017-01-17 12:52:20 -0500, Tom Lane wrote: > Robert Haas writes: > > On Mon, Jan 16, 2017 at 2:13 PM, Tom Lane wrote: > >> "Srf" is ugly as can be, and unintelligible. SetResult might be OK. > > > The operation we're performing here, IIUC, is

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Robert Haas
On Tue, Jan 17, 2017 at 12:52 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Jan 16, 2017 at 2:13 PM, Tom Lane wrote: >>> "Srf" is ugly as can be, and unintelligible. SetResult might be OK. > >> The operation we're performing

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Tom Lane
Robert Haas writes: > On Mon, Jan 16, 2017 at 2:13 PM, Tom Lane wrote: >> "Srf" is ugly as can be, and unintelligible. SetResult might be OK. > The operation we're performing here, IIUC, is projection. SetResult > lacks a verb, although Set could be

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-17 Thread Robert Haas
On Mon, Jan 16, 2017 at 2:13 PM, Tom Lane wrote: > Andres Freund writes: >> That worked quite well. So we have a few questions, before I clean this >> up: > >> - For now the node is named 'Srf' both internally and in explain - not >> sure if we want to

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-16 Thread Alvaro Herrera
Andres Freund wrote: > On 2017-01-16 16:04:34 -0300, Alvaro Herrera wrote: > > Andres Freund wrote: > > > On 2017-01-16 12:17:46 -0300, Alvaro Herrera wrote: > > > > Hmm. I wonder if your stuff could be used as support code for > > > > XMLTABLE[1]. > > > > > > I don't immediately see what

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-16 Thread Andres Freund
On 2017-01-16 16:04:34 -0300, Alvaro Herrera wrote: > Andres Freund wrote: > > On 2017-01-16 12:17:46 -0300, Alvaro Herrera wrote: > > > Hmm. I wonder if your stuff could be used as support code for > > > XMLTABLE[1]. > > > > I don't immediately see what functionality overlaps, could you expand

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-16 Thread Andres Freund
Hi, On 2017-01-16 14:13:18 -0500, Tom Lane wrote: > Andres Freund writes: > > That worked quite well. So we have a few questions, before I clean this > > up: > > > - For now the node is named 'Srf' both internally and in explain - not > > sure if we want to make that

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-16 Thread Tom Lane
Andres Freund writes: > That worked quite well. So we have a few questions, before I clean this > up: > - For now the node is named 'Srf' both internally and in explain - not > sure if we want to make that something longer/easier to understand for > others? Proposals?

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-16 Thread Alvaro Herrera
Andres Freund wrote: > On 2017-01-16 12:17:46 -0300, Alvaro Herrera wrote: > > Andres Freund wrote: > > > > > That worked quite well. So we have a few questions, before I clean this > > > up: > > > > > > - For now the node is named 'Srf' both internally and in explain - not > > > sure if we

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-16 Thread Andres Freund
On 2017-01-16 12:17:46 -0300, Alvaro Herrera wrote: > Andres Freund wrote: > > > That worked quite well. So we have a few questions, before I clean this > > up: > > > > - For now the node is named 'Srf' both internally and in explain - not > > sure if we want to make that something

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-16 Thread Alvaro Herrera
Andres Freund wrote: > That worked quite well. So we have a few questions, before I clean this > up: > > - For now the node is named 'Srf' both internally and in explain - not > sure if we want to make that something longer/easier to understand for > others? Proposals? TargetFunctionScan?

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-16 Thread Andres Freund
On 2017-01-15 19:29:52 -0800, Andres Freund wrote: > On 2016-10-31 09:06:39 -0700, Andres Freund wrote: > > On 2016-09-14 19:28:25 -0400, Tom Lane wrote: > > > Andres Freund writes: > > > > On 2016-09-12 19:35:22 -0400, Tom Lane wrote: > > > Here's a draft patch that is just

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2017-01-15 Thread Andres Freund
On 2016-10-31 09:06:39 -0700, Andres Freund wrote: > On 2016-09-14 19:28:25 -0400, Tom Lane wrote: > > Andres Freund writes: > > > On 2016-09-12 19:35:22 -0400, Tom Lane wrote: > > Here's a draft patch that is just meant to investigate what the planner > > changes might look

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-10-31 Thread Tom Lane
Andres Freund writes: > On 2016-09-14 19:28:25 -0400, Tom Lane wrote: >> So I think we should continue investigating this way of doing things. >> I'll try to take a look at the executor end of it tomorrow. However >> I'm leaving Friday for a week's vacation, and may not have

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-10-31 Thread Andres Freund
Hi Tom, On 2016-09-14 19:28:25 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-12 19:35:22 -0400, Tom Lane wrote: > >> Anyway I'll draft a prototype and then we can compare. > > > Ok, cool. > > Here's a draft patch that is just meant to investigate what the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-10-02 Thread Michael Paquier
On Fri, Sep 16, 2016 at 6:12 AM, Tom Lane wrote: > I'm happy to get rid of the LCM behavior, I just want to have some wiggle > room to be able to get it back if somebody really needs it. Er, actually no that's this thread for this CF entry:

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-15 Thread Tom Lane
Andres Freund writes: > On 2016-09-15 16:48:59 -0400, Tom Lane wrote: >> The patch that I posted would run both the generate_series(1, 2) and >> generate_series(2,4) calls in the same SRF node, forcing them to run in >> lockstep, after which their results would be fed to the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-15 Thread Andres Freund
On 2016-09-15 16:48:59 -0400, Tom Lane wrote: > Andres Freund writes: > > Hm. One thing I wonder about with this approach, is how we're going to > > handle something absurd like: > > SELECT generate_series(1, generate_series(1, 2)), generate_series(1, > >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-15 Thread Tom Lane
Andres Freund writes: > Hm. One thing I wonder about with this approach, is how we're going to > handle something absurd like: > SELECT generate_series(1, generate_series(1, 2)), generate_series(1, > generate_series(2,4)); The patch that I posted would run both the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-15 Thread Andres Freund
Hi, On 2016-09-14 19:28:25 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-12 19:35:22 -0400, Tom Lane wrote: > >> Anyway I'll draft a prototype and then we can compare. > > > Ok, cool. > > Here's a draft patch that is just meant to investigate what the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-15 Thread Andres Freund
On 2016-09-15 15:23:58 -0400, Tom Lane wrote: > Andres Freund writes: > > In my present patch I've *ripped out* the support for materialization > > in nodeFunctionscan.c entirely. That means that rescans referencing > > volatile functions can change their behaviour (if a

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-15 Thread Tom Lane
Andres Freund writes: > 0003-Avoid-materializing-SRFs-in-the-FROM-list.patch > To avoid performance regressions from moving SRFM_ValuePerCall SRFs to > ROWS FROM, nodeFunctionscan.c needs to support not materializing > output. I looked over this patch a bit. > In my

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-14 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 19:35:22 -0400, Tom Lane wrote: >> Anyway I'll draft a prototype and then we can compare. > Ok, cool. Here's a draft patch that is just meant to investigate what the planner changes might look like if we do it in the pipelined-result

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-13 Thread Tom Lane
Andres Freund writes: > On 2016-09-13 12:07:35 -0400, Tom Lane wrote: >> /* >> - * Don't pull up a subquery that has any set-returning functions in its >> - * targetlist. Otherwise we might well wind up inserting set-returning >> - * functions into places where

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-13 Thread Andres Freund
On 2016-09-13 12:07:35 -0400, Tom Lane wrote: > diff --git a/src/backend/optimizer/plan/analindex e28a8dc..74e4245 100644 > --- a/src/backend/optimizer/plan/analyzejoins.c > +++ b/src/backend/optimizer/plan/analyzejoins.c > @@ -650,6 +650,11 @@ rel_is_distinct_for(PlannerInfo *root, R > bool >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-13 Thread Tom Lane
Andres Freund writes: > On September 13, 2016 9:07:35 AM PDT, Tom Lane wrote: >> I'd like to go ahead and push this, since it's a necessary prerequisite >> for either approach we might adopt for the rest of the patch series, >> and the improved error

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-13 Thread Andres Freund
On September 13, 2016 9:07:35 AM PDT, Tom Lane wrote: >Andres Freund writes: >> Attached is a significantly updated patch series (see the mail one up >> for details about what this is, I don't want to quote it in its >> entirety). > >I've reviewed the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-13 Thread Tom Lane
Andres Freund writes: > Attached is a significantly updated patch series (see the mail one up > for details about what this is, I don't want to quote it in its > entirety). I've reviewed the portions of 0005 that have to do with making the parser mark queries with

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 20:15:46 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-12 16:56:32 -0700, Andres Freund wrote: > >> Attached is a noticeably expanded set of tests, with fixes for the stuff > >> you'd found. I plan to push that soon-ish. Independent of the

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 16:56:32 -0700, Andres Freund wrote: >> Attached is a noticeably expanded set of tests, with fixes for the stuff >> you'd found. I plan to push that soon-ish. Independent of the approach >> we'll be choosing, increased coverage seems

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 16:56:32 -0700, Andres Freund wrote: > On 2016-09-12 09:14:47 -0700, Andres Freund wrote: > > On 2016-09-12 10:19:14 -0400, Tom Lane wrote: > > > Andres Freund writes: > > > > > > > 0001-Add-some-more-targetlist-srf-tests.patch > > > > Add some test. > > > >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 09:14:47 -0700, Andres Freund wrote: > On 2016-09-12 10:19:14 -0400, Tom Lane wrote: > > Andres Freund writes: > > > > > 0001-Add-some-more-targetlist-srf-tests.patch > > > Add some test. > > > > I think you should go ahead and push this tests-adding patch

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 19:35:22 -0400, Tom Lane wrote: > >> You're inventing objections. > > > Heh, it's actually your own objection ;) > > http://archives.postgresql.org/message-id/32673.1464023429%40sss.pgh.pa.us > > I'm changing my opinion in the light of unfavorable evidence. Is that > wrong? Nah,

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 18:35:03 -0400, Tom Lane wrote: >> You're inventing objections. > Heh, it's actually your own objection ;) > http://archives.postgresql.org/message-id/32673.1464023429%40sss.pgh.pa.us I'm changing my opinion in the light of unfavorable

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
Hi, On 2016-09-12 18:35:03 -0400, Tom Lane wrote: > Andres Freund writes: > > I don't think it'd be all that hard to add something like the current > > LCM behaviour into nodeFunctionscan.c if we really wanted. But I think > > it'll be better to just say no here. > > "Just

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 17:36:07 -0400, Tom Lane wrote: >> Um, I dunno. You've added half a thousand lines of not-highly-readable- >> nor-extensively-commented code to the planner; that certainly reaches *my* >> threshold of pain. > Well, I certainly plan (and

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 17:36:07 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-12 13:26:20 -0400, Tom Lane wrote: > >> Stepping back a little bit ... way back at the start of this thread > >> you muttered about possibly implementing tSRFs as a special pipeline > >> node

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 13:26:20 -0400, Tom Lane wrote: >> Stepping back a little bit ... way back at the start of this thread >> you muttered about possibly implementing tSRFs as a special pipeline >> node type, a la Result. That would have the same benefits in

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 14:05:33 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-12 13:48:05 -0400, Tom Lane wrote: > >> Andres Freund writes: > >>> I kind of like ROWS FROM (... AS VALUE), that seems to confer the > >>> meaning quite well. As VALUE

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 13:48:05 -0400, Tom Lane wrote: >> Andres Freund writes: >>> I kind of like ROWS FROM (... AS VALUE), that seems to confer the >>> meaning quite well. As VALUE isn't a reserved keyword, that'd afaik only >>> really

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 13:48:05 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-12 13:26:20 -0400, Tom Lane wrote: > >> Andres Freund writes: > > On 2016-09-12 12:10:01 -0400, Tom Lane wrote: > I can't say that I like the proposed syntax much. >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 13:26:20 -0400, Tom Lane wrote: >> Andres Freund writes: > On 2016-09-12 12:10:01 -0400, Tom Lane wrote: I can't say that I like the proposed syntax much. >>> Me neither. But I haven't really found a better

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 13:26:20 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-12 12:10:01 -0400, Tom Lane wrote: > >> I can't say that I like the proposed syntax much. > > > Me neither. But I haven't really found a better approach. It seems > > kinda consistent to

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
Hi, On 2016-09-12 13:26:20 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-12 12:10:01 -0400, Tom Lane wrote: > >> I can't say that I like the proposed syntax much. > > > Me neither. But I haven't really found a better approach. It seems > > kinda consistent

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 12:10:01 -0400, Tom Lane wrote: >> I can't say that I like the proposed syntax much. > Me neither. But I haven't really found a better approach. It seems > kinda consistent to have ROWS FROM (... AS ()) change the picked out > columns to

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
Hi, On 2016-09-12 12:10:01 -0400, Tom Lane wrote: > Andres Freund writes: > > 0003-Avoid-materializing-SRFs-in-the-FROM-list.patch > > To avoid performance regressions from moving SRFM_ValuePerCall SRFs to > > ROWS FROM, nodeFunctionscan.c needs to support not

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > On 2016-09-12 11:29:37 -0400, Tom Lane wrote: >> I'm on board with disallowing SRFs in UPDATE, because it produces >> underdetermined and unspecified results; but the other restriction >> seems 100% arbitrary. There is no semantic difference between >>

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 11:29:37 -0400, Tom Lane wrote: > Andres Freund writes: > > 0002-Shore-up-some-weird-corner-cases-for-targetlist-SRFs.patch > > Forbid UPDATE ... SET foo = SRF() and ORDER BY / GROUP BY containing > > SRFs that would change the number of returned rows.

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Andres Freund
On 2016-09-12 10:19:14 -0400, Tom Lane wrote: > Andres Freund writes: > > > 0001-Add-some-more-targetlist-srf-tests.patch > > Add some test. > > I think you should go ahead and push this tests-adding patch now, as it > adds documentation of the current behavior that is a

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > 0003-Avoid-materializing-SRFs-in-the-FROM-list.patch > To avoid performance regressions from moving SRFM_ValuePerCall SRFs to > ROWS FROM, nodeFunctionscan.c needs to support not materializing > output. Personally I'd put this one later, as it's

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > 0002-Shore-up-some-weird-corner-cases-for-targetlist-SRFs.patch > Forbid UPDATE ... SET foo = SRF() and ORDER BY / GROUP BY containing > SRFs that would change the number of returned rows. Without the > latter e.g. SELECT 1 ORDER BY

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-12 Thread Tom Lane
Andres Freund writes: > Attached is a significantly updated patch series (see the mail one up > for details about what this is, I don't want to quote it in its > entirety). I've finally cleared my plate enough to start reviewing this patchset. >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Andres Freund
On 2016-09-02 14:04:24 +0530, Robert Haas wrote: > On Sun, Aug 28, 2016 at 3:18 AM, Andres Freund wrote: > > 0003-Avoid-materializing-SRFs-in-the-FROM-list.patch > > To avoid performance regressions from moving SRFM_ValuePerCall SRFs to > > ROWS FROM, nodeFunctionscan.c

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Tom Lane
Andres Freund writes: > ... What we could do is to add efficient > ROWS FROM (..) WITH ORDINALITY ORDER bY ordinality; > support. Hm? regression=# explain select * from rows from (generate_series(1,10)) with ordinality order by ordinality;

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Andres Freund
On 2016-09-02 10:58:59 -0500, Kevin Grittner wrote: > If it has no significant performance impact to maintain the > historical order, then I have no problem with doing so. It's not really a runtime issue, it's just a question of how to nicely constraint the join order. There's no additional

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Kevin Grittner
On Fri, Sep 2, 2016 at 10:10 AM, Tom Lane wrote: > Kevin Grittner writes: >> On Fri, Sep 2, 2016 at 9:51 AM, Tom Lane wrote: >>> regression=# select *, generate_series(1,3) from int8_tbl; > >> I'm sure that you realize that running a

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Kevin Grittner
On Fri, Sep 2, 2016 at 10:10 AM, Tom Lane wrote: > Also, before getting too high and mighty with users who expect > "select * from table" to produce rows in a predictable order, > you should reflect on the number of places in our regression > tests that assume exactly that

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Kevin Grittner
On Fri, Sep 2, 2016 at 10:31 AM, Andres Freund wrote: > On 2016-09-02 09:41:28 -0500, Kevin Grittner wrote: >> On Fri, Sep 2, 2016 at 9:25 AM, Tom Lane wrote: >> > Andres Freund writes: >> >> Oh, and we've previously re-added that

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Andres Freund
On 2016-09-02 09:41:28 -0500, Kevin Grittner wrote: > On Fri, Sep 2, 2016 at 9:25 AM, Tom Lane wrote: > > Andres Freund writes: > >> Oh, and we've previously re-added that based on > >> complaints. C.f. d543170f2fdd6d9845aaf91dc0f6be7a2bf0d9e7 (and others

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Andres Freund
On 2016-09-02 10:34:54 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-02 10:20:42 -0400, Tom Lane wrote: > >> ... ISTM all we > >> need is that the SRF be on the inside of the join, which is automatic > >> if it's LATERAL. > > > Right. But there's nothing to

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Tom Lane
Kevin Grittner writes: > On Fri, Sep 2, 2016 at 9:51 AM, Tom Lane wrote: >> regression=# select *, generate_series(1,3) from int8_tbl; > I'm sure that you realize that running a query of that form twice > against a table with more than one heap page could

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Kevin Grittner
On Fri, Sep 2, 2016 at 9:51 AM, Tom Lane wrote: > regression=# select *, generate_series(1,3) from int8_tbl; I'm sure that you realize that running a query of that form twice against a table with more than one heap page could result in rows in a different order, even if no

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Tom Lane
Kevin Grittner writes: > On Fri, Sep 2, 2016 at 9:25 AM, Tom Lane wrote: >> ORDER BY is not a useful suggestion when there is nothing >> you could order by to get the old behavior. > I'm apparently missing something, because I see a column with the >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Tom Lane
Andres Freund writes: > On 2016-09-02 10:20:42 -0400, Tom Lane wrote: >> ... ISTM all we >> need is that the SRF be on the inside of the join, which is automatic >> if it's LATERAL. > Right. But there's nothing to force a lateral reference to be there > intrinsically. I've

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Kevin Grittner
On Fri, Sep 2, 2016 at 9:25 AM, Tom Lane wrote: > Andres Freund writes: >> Oh, and we've previously re-added that based on >> complaints. C.f. d543170f2fdd6d9845aaf91dc0f6be7a2bf0d9e7 (and others >> IIRC). > > That one wasn't about row order per se, but I

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Tom Lane
Andres Freund writes: > On 2016-09-02 09:05:35 -0500, Kevin Grittner wrote: >>> In general, we've been skeptical about giving any guarantees about >>> result ordering. > Well, it's historically how we behaved for SRFs. I'm pretty sure that > people will be confused if >

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Andres Freund
On 2016-09-02 10:20:42 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-09-02 09:05:35 -0500, Kevin Grittner wrote: > >>> In general, we've been skeptical about giving any guarantees about > >>> result ordering. > > > Well, it's historically how we behaved for SRFs.

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Tom Lane
Andres Freund writes: > Oh, and we've previously re-added that based on > complaints. C.f. d543170f2fdd6d9845aaf91dc0f6be7a2bf0d9e7 (and others > IIRC). That one wasn't about row order per se, but I agree that people *will* bitch if we change the behavior, especially if we

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Andres Freund
On 2016-09-02 07:11:10 -0700, Andres Freund wrote: > On 2016-09-02 09:05:35 -0500, Kevin Grittner wrote: > > On Fri, Sep 2, 2016 at 3:31 AM, Robert Haas wrote: > > > On Tue, Aug 23, 2016 at 3:10 AM, Andres Freund wrote: > > > > >> =# SELECT * FROM few,

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Kevin Grittner
On Fri, Sep 2, 2016 at 9:11 AM, Andres Freund wrote: > On 2016-09-02 09:05:35 -0500, Kevin Grittner wrote: >> On Fri, Sep 2, 2016 at 3:31 AM, Robert Haas wrote: >>> On Tue, Aug 23, 2016 at 3:10 AM, Andres Freund wrote: >> =#

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Andres Freund
Hi, Thanks for looking. On 2016-09-02 14:01:32 +0530, Robert Haas wrote: > > 6) SETOF record type functions cannot directly be used in ROWS FROM() - > >as ROWS FROM "expands" records returned by functions. When converting > >something like > > CREATE OR REPLACE FUNCTION

Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

2016-09-02 Thread Andres Freund
On 2016-09-02 09:05:35 -0500, Kevin Grittner wrote: > On Fri, Sep 2, 2016 at 3:31 AM, Robert Haas wrote: > > On Tue, Aug 23, 2016 at 3:10 AM, Andres Freund wrote: > > >> =# SELECT * FROM few, ROWS FROM(generate_series(1,3)); > >>

  1   2   >