Re: [HACKERS] Changed SRF in targetlist handling

Robert Haas writes: > Tom, it's been about 3.5 months since you wrote this. I think it > would be really valuable if you could get to this RSN because the > large patch set posted on the "Changed SRF in targetlist handling" > thread is backed up behind this -- and I think

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, Aug 22, 2016 at 4:20 PM, Tom Lane wrote: > Andres Freund writes: >> On 2016-08-17 17:41:28 -0700, Andres Freund wrote: >>> Tom, do you think this is roughly going in the right direction? > > I've not had time to look at this patch, I'm afraid. If

Re: [HACKERS] Changed SRF in targetlist handling

On Wed, Aug 24, 2016 at 3:55 AM, Andres Freund wrote: > Comments? This thread has no activity for some time now and it is linked to this CF entry: https://commitfest.postgresql.org/10/759/ I am marking it as returned with feedback.. -- Michael -- Sent via pgsql-hackers

Re: [HACKERS] Changed SRF in targetlist handling

On 2016-08-17 17:41:28 -0700, Andres Freund wrote: > a) Allow to avoid using a tuplestore for SRF_PERCALL SRFs in ROWS FROM - >otherwise our performance would regress noticeably in some cases. To demonstrate the problem: master: =# COPY (SELECT generate_series(1, 5000)) TO '/dev/null';

Re: [HACKERS] Changed SRF in targetlist handling

Hi, On 2016-08-22 16:20:58 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-08-17 17:41:28 -0700, Andres Freund wrote: > >> Tom, do you think this is roughly going in the right direction? > > I've not had time to look at this patch, I'm afraid. If you still > want

Re: [HACKERS] Changed SRF in targetlist handling

Andres Freund writes: > On 2016-08-17 17:41:28 -0700, Andres Freund wrote: >> Tom, do you think this is roughly going in the right direction? I've not had time to look at this patch, I'm afraid. If you still want me to, I can make time in a day or so. > I'm working on

Re: [HACKERS] Changed SRF in targetlist handling

Hi, On 2016-05-23 09:26:03 +0800, Craig Ringer wrote: > SRFs-in-tlist are a lot faster for lockstep iteration etc. They're also > much simpler to write, though if the result result rowcount differs > unexpectedly between the functions you get exciting and unexpected > behaviour. > > WITH

Re: [HACKERS] Changed SRF in targetlist handling

On 2016-08-17 17:41:28 -0700, Andres Freund wrote: > Tom, do you think this is roughly going in the right direction? My plan > here is to develop two patches, to come before this: > > a) Allow to avoid using a tuplestore for SRF_PERCALL SRFs in ROWS FROM - >otherwise our performance would

Re: [HACKERS] Changed SRF in targetlist handling

On 2016-08-03 20:22:03 -0700, Andres Freund wrote: > On 2016-08-02 16:30:55 -0700, Andres Freund wrote: > > > > Besides that I'm structurally wondering whether turning the original > > > > query into a subquery is the right thing to do. It requires some kind of > > > > ugly munching of Query->*,

Re: [HACKERS] Changed SRF in targetlist handling

On 2016-08-02 19:02:38 -0400, Tom Lane wrote: > Andres Freund writes: > > I've an implementation that > > > 1) turns all targetlist SRF (tSRF from now on) into ROWS FROM > >expressions. If there's tSRFs in the argument of a tSRF those becomes > >a separate, lateral,

Re: [HACKERS] Changed SRF in targetlist handling

Andres Freund writes: > I've an implementation that > 1) turns all targetlist SRF (tSRF from now on) into ROWS FROM >expressions. If there's tSRFs in the argument of a tSRF those becomes >a separate, lateral, ROWS FROM expression. > 2) If grouping/window functions

Re: [HACKERS] Changed SRF in targetlist handling

On 2016-05-25 16:55:23 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-05-25 15:20:03 -0400, Tom Lane wrote: > >> We could certainly make a variant behavior in nodeFunctionscan.c that > >> emulates that, if we feel that being exactly bug-compatible on the point > >>

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, Jun 6, 2016 at 3:26 PM, Tom Lane wrote: > Robert Haas writes: >> ... I guess I'd prefer #2 to #2.5, #2.5 to #3, and #3 to #1. >> I really don't like #1 much - I think I'd almost rather do nothing. > > FWIW, that's about my evaluation of the

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, Jun 6, 2016 at 3:26 PM, Tom Lane wrote: > Robert Haas writes: > > ... I guess I'd prefer #2 to #2.5, #2.5 to #3, and #3 to #1. > > I really don't like #1 much - I think I'd almost rather do nothing. > > FWIW, that's about my evaluation of the

Re: [HACKERS] Changed SRF in targetlist handling

Robert Haas writes: > ... I guess I'd prefer #2 to #2.5, #2.5 to #3, and #3 to #1. > I really don't like #1 much - I think I'd almost rather do nothing. FWIW, that's about my evaluation of the alternatives as well. I fear that #1 would get a lot of pushback. If we think

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, Jun 6, 2016 at 2:53 PM, Tom Lane wrote: > Now, if we decide to try to rewrite tlist SRFs as LATERAL, it would likely > behoove us to do that rewrite before expanding * not after, so that we can > eliminate the multiple evaluation of foo() that happens currently. (That

Re: [HACKERS] Changed SRF in targetlist handling

Alvaro Herrera writes: > Robert Haas wrote: >> On Mon, Jun 6, 2016 at 11:50 AM, Tom Lane wrote: >>> No, because then you get the cross-product of multiple SRFs, not the >>> run-in-lockstep behavior. >> Oh. I assumed that was the expected behavior.

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, Jun 6, 2016 at 2:31 PM, Alvaro Herrera wrote: > Robert Haas wrote: > > On Mon, Jun 6, 2016 at 11:50 AM, Tom Lane wrote: > > > Robert Haas writes: > > >> On Mon, May 23, 2016 at 4:15 PM, Tom Lane

Re: [HACKERS] Changed SRF in targetlist handling

Robert Haas wrote: > On Mon, Jun 6, 2016 at 11:50 AM, Tom Lane wrote: > > Robert Haas writes: > >> On Mon, May 23, 2016 at 4:15 PM, Tom Lane wrote: > >>> 2. Rewrite into LATERAL ROWS FROM (srf1(), srf2(), ...). This would > >>>

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, Jun 6, 2016 at 11:50 AM, Tom Lane wrote: > Robert Haas writes: >> On Mon, May 23, 2016 at 4:15 PM, Tom Lane wrote: >>> 2. Rewrite into LATERAL ROWS FROM (srf1(), srf2(), ...). This would >>> have the same behavior as before

Re: [HACKERS] Changed SRF in targetlist handling

"David G. Johnston" writes: > If the SRFs return a different number of rows the LCM behavior kicks in and > you get Robert's second result. Only if the periods of the SRFs are relatively prime. That is, neither of his examples demonstrate the full weirdness of the

Re: [HACKERS] Changed SRF in targetlist handling

On 06/06/16 18:30, David G. Johnston wrote: > To clarify, the present behavior is basically a combination of both of > Robert's results. > > If the SRFs return the same number of rows the first (zippered) result > is returned without an NULL padding. > > If the SRFs return a different number of

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, Jun 6, 2016 at 11:50 AM, Tom Lane wrote: > Robert Haas writes: > > On Mon, May 23, 2016 at 4:15 PM, Tom Lane wrote: > >> 2. Rewrite into LATERAL ROWS FROM (srf1(), srf2(), ...). This would > >> have the same behavior as

Re: [HACKERS] Changed SRF in targetlist handling

Robert Haas writes: > On Mon, May 23, 2016 at 4:15 PM, Tom Lane wrote: >> 2. Rewrite into LATERAL ROWS FROM (srf1(), srf2(), ...). This would >> have the same behavior as before if the SRFs all return the same number >> of rows, and otherwise would

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 4:15 PM, Tom Lane wrote: > We should consider single and multiple SRFs in a targetlist as distinct > use-cases; only the latter has got weird properties. > > There are several things we could potentially do with multiple SRFs in > the same targetlist.

Re: [HACKERS] Changed SRF in targetlist handling

"David G. Johnston" writes: > On Friday, June 3, 2016, Tom Lane > wrote: >> Merlin Moncure writes: >>> another interesting case today is: >>> create sequence s; >>> select

Re: [HACKERS] Changed SRF in targetlist handling

Merlin Moncure writes: > On Wed, May 25, 2016 at 3:55 PM, Tom Lane wrote: >> Andres Freund writes: >>> If we go with rewriting this into LATERAL, I'd vote for 2.5 (trailed by >>> option 1), that'd keep most of the functionality, and

Re: [HACKERS] Changed SRF in targetlist handling

On Wed, May 25, 2016 at 3:55 PM, Tom Lane wrote: > Andres Freund writes: >> On 2016-05-25 15:20:03 -0400, Tom Lane wrote: >>> We could certainly make a variant behavior in nodeFunctionscan.c that >>> emulates that, if we feel that being exactly

Re: [HACKERS] Changed SRF in targetlist handling

Andres Freund writes: > On 2016-05-25 15:20:03 -0400, Tom Lane wrote: >> We could certainly make a variant behavior in nodeFunctionscan.c that >> emulates that, if we feel that being exactly bug-compatible on the point >> is actually what we want. I'm dubious about that

Re: [HACKERS] Changed SRF in targetlist handling

On 2016-05-25 15:20:03 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-05-25 15:02:23 -0400, Tom Lane wrote: > >> [ shrug... ] That seems like it's morally equivalent to (but uglier than) > >> what I wanted to do, which is to teach the planner to rewrite the query

Re: [HACKERS] Changed SRF in targetlist handling

Andres Freund writes: > On 2016-05-25 15:02:23 -0400, Tom Lane wrote: >> [ shrug... ] That seems like it's morally equivalent to (but uglier than) >> what I wanted to do, which is to teach the planner to rewrite the query to >> put the SRFs into a lateral FROM item.

Re: [HACKERS] Changed SRF in targetlist handling

On 2016-05-25 15:02:23 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-05-23 13:10:29 -0400, Tom Lane wrote: > >> Would that not lead to, in effect, duplicating all of execQual.c? The new > >> executor node would still have to be prepared to process all expression

Re: [HACKERS] Changed SRF in targetlist handling

Andres Freund writes: > On 2016-05-23 13:10:29 -0400, Tom Lane wrote: >> Would that not lead to, in effect, duplicating all of execQual.c? The new >> executor node would still have to be prepared to process all expression >> node types. > I don't think it necessarily has to.

Re: [HACKERS] Changed SRF in targetlist handling

On 2016-05-23 13:10:29 -0400, Tom Lane wrote: > Andres Freund writes: > > One idea I circulated was to fix that by interjecting a special executor > > node to process SRF containing targetlists (reusing Result possibly?). > > That'd allow to remove the isDone argument from

Re: [HACKERS] Changed SRF in targetlist handling

On 05/23/2016 02:37 PM, David G. Johnston wrote: > ​But then I don't get Joe's point - if its an implementation detail why > should it matter if rewriting the SRF-in-tlist to be laterals changes > execution from a serial to an interleaved​ implementation. Plus, Joe's > claim: "the capability to

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 4:42 PM, Tom Lane wrote: > "David G. Johnston" writes: > > On Mon, May 23, 2016 at 4:24 PM, Alvaro Herrera < > alvhe...@2ndquadrant.com> > > wrote: > >> Ah, so that's what "pipeline results" mean! I hadn't gotten that. I

Re: [HACKERS] Changed SRF in targetlist handling

"David G. Johnston" writes: > On Mon, May 23, 2016 at 4:24 PM, Alvaro Herrera > wrote: >> Ah, so that's what "pipeline results" mean! I hadn't gotten that. I >> agree; Abhijit had a patch or a plan for this, a long time ago ... > ​Is

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 4:24 PM, Alvaro Herrera wrote: > Tom Lane wrote: > > Joe Conway writes: > > > > I'll also note that, unless I missed something, we also have to > consider > > > that the capability to pipeline results is still only available

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 4:15 PM, Tom Lane wrote: > Merlin Moncure writes: > > On Mon, May 23, 2016 at 2:13 PM, David Fetter wrote: > >> On Mon, May 23, 2016 at 01:28:11PM -0500, Merlin Moncure wrote: > >>> +1 on removing LCM. > > >> As

Re: [HACKERS] Changed SRF in targetlist handling

Tom Lane wrote: > Joe Conway writes: > > I'll also note that, unless I missed something, we also have to consider > > that the capability to pipeline results is still only available in the > > target list. > > Yes, we would definitely want to improve nodeFunctionscan.c to

Re: [HACKERS] Changed SRF in targetlist handling

Joe Conway writes: > I would be in favor of rewriting it to a LATERAL, but that would not be > backwards compatible entirely either IIUC. It could be made so, I think, but it may be more trouble than it's worth; see my previous message. > I'll also note that, unless I missed

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 4:05 PM, David Fetter wrote: > On Mon, May 23, 2016 at 02:39:54PM -0500, Merlin Moncure wrote: > > On Mon, May 23, 2016 at 2:13 PM, David Fetter wrote: > > > On Mon, May 23, 2016 at 01:28:11PM -0500, Merlin Moncure wrote: > > >> On

Re: [HACKERS] Changed SRF in targetlist handling

Merlin Moncure writes: > On Mon, May 23, 2016 at 2:13 PM, David Fetter wrote: >> On Mon, May 23, 2016 at 01:28:11PM -0500, Merlin Moncure wrote: >>> +1 on removing LCM. >> As a green field project, that would make total sense. As a thing >> decades in,

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 02:39:54PM -0500, Merlin Moncure wrote: > On Mon, May 23, 2016 at 2:13 PM, David Fetter wrote: > > On Mon, May 23, 2016 at 01:28:11PM -0500, Merlin Moncure wrote: > >> On Mon, May 23, 2016 at 12:10 PM, Tom Lane wrote: > >> > Andres

Re: [HACKERS] Changed SRF in targetlist handling

On 05/23/2016 12:39 PM, Merlin Moncure wrote: > On Mon, May 23, 2016 at 2:13 PM, David Fetter wrote: >> On Mon, May 23, 2016 at 01:28:11PM -0500, Merlin Moncure wrote: >>> On Mon, May 23, 2016 at 12:10 PM, Tom Lane wrote: Andres Freund

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 2:13 PM, David Fetter wrote: > On Mon, May 23, 2016 at 01:28:11PM -0500, Merlin Moncure wrote: >> On Mon, May 23, 2016 at 12:10 PM, Tom Lane wrote: >> > Andres Freund writes: >> >> discussing executor performance

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 01:28:11PM -0500, Merlin Moncure wrote: > On Mon, May 23, 2016 at 12:10 PM, Tom Lane wrote: > > Andres Freund writes: > >> discussing executor performance with a number of people at pgcon, > >> several hackers - me included -

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 1:44 PM, David Fetter wrote: > On Mon, May 23, 2016 at 01:36:57PM -0400, Tom Lane wrote: > > David Fetter writes: > > > On Mon, May 23, 2016 at 01:10:29PM -0400, Tom Lane wrote: > > >> This seems a bridge too far to me. It's just way

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 12:10 PM, Tom Lane wrote: > Andres Freund writes: >> discussing executor performance with a number of people at pgcon, >> several hackers - me included - complained about the additional >> complexity, both code and runtime, required

Re: [HACKERS] Changed SRF in targetlist handling

David Fetter writes: > On Mon, May 23, 2016 at 01:36:57PM -0400, Tom Lane wrote: >> David Fetter writes: >>> How about making "TABLE generate_series(1,n)" work? It's even >>> shorter in exchange for some cognitive load. >> No thanks --- the word after TABLE

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 01:36:57PM -0400, Tom Lane wrote: > David Fetter writes: > > On Mon, May 23, 2016 at 01:10:29PM -0400, Tom Lane wrote: > >> This seems a bridge too far to me. It's just way too common to do > >> "select generate_series(1,n)". We could tell people they

Re: [HACKERS] Changed SRF in targetlist handling

David Fetter writes: > On Mon, May 23, 2016 at 01:10:29PM -0400, Tom Lane wrote: >> This seems a bridge too far to me. It's just way too common to do >> "select generate_series(1,n)". We could tell people they have to >> rewrite to "select * from generate_series(1,n)", but it

Re: [HACKERS] Changed SRF in targetlist handling

On Mon, May 23, 2016 at 01:10:29PM -0400, Tom Lane wrote: > This seems a bridge too far to me. It's just way too common to do > "select generate_series(1,n)". We could tell people they have to > rewrite to "select * from generate_series(1,n)", but it would be far > more polite to do that for

Re: [HACKERS] Changed SRF in targetlist handling

Andres Freund writes: > discussing executor performance with a number of people at pgcon, > several hackers - me included - complained about the additional > complexity, both code and runtime, required to handle SRFs in the target > list. Yeah, this has been an annoyance for

Re: [HACKERS] Changed SRF in targetlist handling

tl;dr Semantic changes to SRF-in-target-list processing are undesirable when they are all but deprecated. I'd accept a refactoring that trades a performance gain for unaffected queries for a reasonable performance hit of those afflicted. Preamble... Most recent thread that I can recall seeing

Re: [HACKERS] Changed SRF in targetlist handling

On 23 May 2016 at 08:53, Andres Freund wrote: > Hi, > > discussing executor performance with a number of people at pgcon, > several hackers - me included - complained about the additional > complexity, both code and runtime, required to handle SRFs in the target > list. > >