Re: Early WIP/PoC for inlining CTEs

2019-04-09 Thread Tom Lane
I wrote: > Andrew Gierth writes: >> So what I think we need to do here is to forbid inlining if (a) the >> refcount is greater than 1 and (b) the CTE in question contains, >> recursively anywhere inside its rtable or the rtables of any of its >> nested CTEs, a "self_reference" RTE. > That's kind

Re: Early WIP/PoC for inlining CTEs

2019-03-12 Thread Tatsuo Ishii
> On 2018-08-08 16:55:22 +1200, Thomas Munro wrote: >> On Fri, Jul 27, 2018 at 8:10 PM, David Fetter wrote: >> > On Fri, Jul 27, 2018 at 02:55:26PM +1200, Thomas Munro wrote: >> >> On Thu, Jul 26, 2018 at 7:14 AM, David Fetter wrote: >> >> > Please find attached the next version, which passes

Re: Early WIP/PoC for inlining CTEs

2019-02-26 Thread Andrew Gierth
> "Tom" == Tom Lane writes: >> I also thought about that. But what I thought about it on reflection >> was: if the user explicitly wrote NOT MATERIALIZED, then we should >> assume they mean it. Tom> Ah, but the example I gave also had MATERIALIZED on the inner WITH. Tom> Why should the

Re: Early WIP/PoC for inlining CTEs

2019-02-26 Thread Tom Lane
Andrew Gierth writes: > "Tom" == Tom Lane writes: > Tom> Also, I thought of a somewhat-related scenario that the code isn't > Tom> accounting for: you can break the restrictions about single > Tom> evaluation with nested WITHs, like > I also thought about that. But what I thought about it on

Re: Early WIP/PoC for inlining CTEs

2019-02-26 Thread Andrew Gierth
> "Tom" == Tom Lane writes: Tom> Also, I thought of a somewhat-related scenario that the code isn't Tom> accounting for: you can break the restrictions about single Tom> evaluation with nested WITHs, like I also thought about that. But what I thought about it on reflection was: if the

Re: Early WIP/PoC for inlining CTEs

2019-02-26 Thread Tom Lane
Andrew Gierth writes: > Here, uncommenting that NOT actually changes the result, from 22 rows to > 4 rows, because we end up generating multiple worktable scans and the > recursion logic is not set up to handle that. Ugh. > So what I think we need to do here is to forbid inlining if (a) the >

Re: Early WIP/PoC for inlining CTEs

2019-02-26 Thread Andrew Gierth
So it turns out there's another case we have to check for, as reported on IRC by Yaroslav Schekin (though I've simplified his query a little): WITH RECURSIVE x(a) AS ((VALUES ('a'),('b')) UNION ALL (WITH z AS /*NOT*/ MATERIALIZED (SELECT a FROM x)

Re: Early WIP/PoC for inlining CTEs

2019-02-16 Thread Andreas Karlsson
On 16/02/2019 22.14, Tom Lane wrote: I was expecting more controversy ... pushed that way. Thanks! And thanks to everyone else who worked on this patch! Andreas

Re: Early WIP/PoC for inlining CTEs

2019-02-16 Thread Tomas Vondra
On 2/16/19 10:14 PM, Tom Lane wrote: > Tomas Vondra writes: >> On 2/14/19 8:22 PM, Merlin Moncure wrote: >>> On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera >>> wrote: On 2019-Feb-14, Peter Eisentraut wrote: > If we're not really planning to add any more options, I'd register a >

Re: Early WIP/PoC for inlining CTEs

2019-02-16 Thread Jonathan S. Katz
On 2/16/19 4:14 PM, Tom Lane wrote: > Tomas Vondra writes: >> On 2/14/19 8:22 PM, Merlin Moncure wrote: >>> On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera >>> wrote: On 2019-Feb-14, Peter Eisentraut wrote: > If we're not really planning to add any more options, I'd register a >

Re: Early WIP/PoC for inlining CTEs

2019-02-16 Thread Tom Lane
Tomas Vondra writes: > On 2/14/19 8:22 PM, Merlin Moncure wrote: >> On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera >> wrote: >>> On 2019-Feb-14, Peter Eisentraut wrote: If we're not really planning to add any more options, I'd register a light vote for MATERIALIZED. It reads easier,

Re: Early WIP/PoC for inlining CTEs

2019-02-15 Thread Tomas Vondra
On 2/14/19 8:22 PM, Merlin Moncure wrote: > On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera > wrote: >> >> On 2019-Feb-14, Peter Eisentraut wrote: >> >>> On 14/02/2019 16:11, Tom Lane wrote: ... so, have we beaten this topic to death yet? Can we make a decision? Personally, I'd be

Re: Early WIP/PoC for inlining CTEs

2019-02-14 Thread Andreas Karlsson
On 14/02/2019 16.11, Tom Lane wrote: ... so, have we beaten this topic to death yet? Can we make a decision? Personally, I'd be happy with either of the last two patch versions I posted (that is, either AS [[NOT] MATERIALIZED] or AS [MATERIALIZE [ON|OFF]] syntax). But we gotta pick something.

Re: Early WIP/PoC for inlining CTEs

2019-02-14 Thread Bruce Momjian
On Thu, Feb 14, 2019 at 04:25:27PM +0100, Peter Eisentraut wrote: > On 06/02/2019 10:00, Bruce Momjian wrote: > > I think "materialize" is the right word since "materialized" would be > > past tense. > > It's an adjective. The sentence is, "with foo as the materialized > $query, do the

Re: Early WIP/PoC for inlining CTEs

2019-02-14 Thread Merlin Moncure
On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera wrote: > > On 2019-Feb-14, Peter Eisentraut wrote: > > > On 14/02/2019 16:11, Tom Lane wrote: > > > ... so, have we beaten this topic to death yet? Can we make a decision? > > > > > > Personally, I'd be happy with either of the last two patch

Re: Early WIP/PoC for inlining CTEs

2019-02-14 Thread Alvaro Herrera
On 2019-Feb-14, Peter Eisentraut wrote: > On 14/02/2019 16:11, Tom Lane wrote: > > ... so, have we beaten this topic to death yet? Can we make a decision? > > > > Personally, I'd be happy with either of the last two patch versions > > I posted (that is, either AS [[NOT] MATERIALIZED] or > > AS

Re: Early WIP/PoC for inlining CTEs

2019-02-14 Thread Peter Eisentraut
On 06/02/2019 10:00, Bruce Momjian wrote: > I think "materialize" is the right word since "materialized" would be > past tense. It's an adjective. The sentence is, "with foo as the materialized $query, do the $main_query". It's the same as "materialized view". -- Peter Eisentraut

Re: Early WIP/PoC for inlining CTEs

2019-02-14 Thread Tom Lane
... so, have we beaten this topic to death yet? Can we make a decision? Personally, I'd be happy with either of the last two patch versions I posted (that is, either AS [[NOT] MATERIALIZED] or AS [MATERIALIZE [ON|OFF]] syntax). But we gotta pick something. regards, tom

Re: Early WIP/PoC for inlining CTEs

2019-02-09 Thread Tom Lane
Andrew Gierth writes: > "Tom" == Tom Lane writes: > Tom> After further reflection I really don't like Andrew's suggestion > Tom> that we not document the rule that multiply-referenced CTEs won't > Tom> be inlined by default. That would be giving up the principle that > Tom> WITH calculations

Re: Early WIP/PoC for inlining CTEs

2019-02-06 Thread Bruce Momjian
On Sat, Feb 2, 2019 at 02:01:01PM -0500, Tom Lane wrote: > I wrote: > > I propose that we implement and document this as > > WITH ctename AS [ MATERIALIZE { ON | OFF } ] ( query ) > > which is maybe a bit clunky but not awful, and it would leave room > > to generalize it to "AS [ optionname

Re: Early WIP/PoC for inlining CTEs

2019-02-03 Thread Tom Lane
Vik Fearing writes: > On 28/01/2019 23:05, Tom Lane wrote: >> Peter Eisentraut writes: >>> Or put it at the end? >>> WITH ctename AS ( query ) MATERIALIZED >> Yeah, I thought about that too, but it doesn't seem like an improvement. >> If the query is very long (which isn't unlikely) I think

Re: Early WIP/PoC for inlining CTEs

2019-02-03 Thread Vik Fearing
On 28/01/2019 23:05, Tom Lane wrote: > Peter Eisentraut writes: >> On 28/01/2019 21:35, Tom Lane wrote: >>> Conceivably we could make it work without the parens: >>> ... > >> Or put it at the end? >> WITH ctename AS ( query ) MATERIALIZED > > Yeah, I thought about that too, but it doesn't

Re: Early WIP/PoC for inlining CTEs

2019-02-03 Thread Andrew Gierth
> "Tom" == Tom Lane writes: Tom> After further reflection I really don't like Andrew's suggestion Tom> that we not document the rule that multiply-referenced CTEs won't Tom> be inlined by default. That would be giving up the principle that Tom> WITH calculations are not done multiple

Re: Early WIP/PoC for inlining CTEs

2019-02-02 Thread Tom Lane
I wrote: > I propose that we implement and document this as > WITH ctename AS [ MATERIALIZE { ON | OFF } ] ( query ) > which is maybe a bit clunky but not awful, and it would leave room > to generalize it to "AS [ optionname optionvalue [ , ... ] ]" if we > ever need to. Looking at the

Re: Early WIP/PoC for inlining CTEs

2019-01-29 Thread Merlin Moncure
On Tuesday, January 29, 2019, Tom Lane wrote: > Merlin Moncure writes: > > On Tue, Jan 29, 2019 at 1:53 PM Tom Lane wrote: > >> I propose that we implement and document this as > >> WITH ctename AS [ MATERIALIZE { ON | OFF } ] ( query ) > > > I hate to bikeshed here, but I think it's better

Re: Early WIP/PoC for inlining CTEs

2019-01-29 Thread Tom Lane
David Fetter writes: > On Tue, Jan 29, 2019 at 02:52:44PM -0500, Tom Lane wrote: >> I propose that we implement and document this as >> WITH ctename AS [ MATERIALIZE { ON | OFF } ] ( query ) > I think this would be better with parentheses like this: > WITH ctename [ ( MATERIALIZE { ON | OFF

Re: Early WIP/PoC for inlining CTEs

2019-01-29 Thread David Fetter
On Tue, Jan 29, 2019 at 02:52:44PM -0500, Tom Lane wrote: > Michael Paquier writes: > > On Mon, Jan 28, 2019 at 05:05:32PM -0500, Tom Lane wrote: > >> Yeah, I thought about that too, but it doesn't seem like an improvement. > >> If the query is very long (which isn't unlikely) I think people

Re: Early WIP/PoC for inlining CTEs

2019-01-29 Thread Chapman Flack
On 1/29/19 3:36 PM, Merlin Moncure wrote: > I hate to bikeshed here, but I think it's better english using that > style of syntax to say, > WITH ctename AS [ MATERIALIZATION { ON | OFF } ] ( query ) I had been just about to also engage in bikeshedding, on grounds that (to me) the

Re: Early WIP/PoC for inlining CTEs

2019-01-29 Thread Tom Lane
Merlin Moncure writes: > On Tue, Jan 29, 2019 at 1:53 PM Tom Lane wrote: >> I propose that we implement and document this as >> WITH ctename AS [ MATERIALIZE { ON | OFF } ] ( query ) > I hate to bikeshed here, but I think it's better english using that > style of syntax to say, > WITH ctename

Re: Early WIP/PoC for inlining CTEs

2019-01-29 Thread Merlin Moncure
On Tue, Jan 29, 2019 at 1:53 PM Tom Lane wrote: > > Michael Paquier writes: > > On Mon, Jan 28, 2019 at 05:05:32PM -0500, Tom Lane wrote: > >> Yeah, I thought about that too, but it doesn't seem like an improvement. > >> If the query is very long (which isn't unlikely) I think people would > >>

Re: Early WIP/PoC for inlining CTEs

2019-01-29 Thread Tom Lane
Michael Paquier writes: > On Mon, Jan 28, 2019 at 05:05:32PM -0500, Tom Lane wrote: >> Yeah, I thought about that too, but it doesn't seem like an improvement. >> If the query is very long (which isn't unlikely) I think people would >> prefer to see the option(s) up front. > Having these options

Re: Early WIP/PoC for inlining CTEs

2019-01-28 Thread Michael Paquier
On Mon, Jan 28, 2019 at 05:05:32PM -0500, Tom Lane wrote: > Yeah, I thought about that too, but it doesn't seem like an improvement. > If the query is very long (which isn't unlikely) I think people would > prefer to see the option(s) up front. Having these options at the front of the WITH clause

Re: Early WIP/PoC for inlining CTEs

2019-01-28 Thread Andreas Karlsson
On 1/28/19 10:54 PM, Peter Eisentraut wrote: Or put it at the end? WITH ctename AS ( query ) MATERIALIZED Hm, seems like that would be easy to miss for long queries. Andreas

Re: Early WIP/PoC for inlining CTEs

2019-01-28 Thread Tom Lane
Peter Eisentraut writes: > On 28/01/2019 21:35, Tom Lane wrote: >> Conceivably we could make it work without the parens: >> ... > Or put it at the end? > WITH ctename AS ( query ) MATERIALIZED Yeah, I thought about that too, but it doesn't seem like an improvement. If the query is very long

Re: Early WIP/PoC for inlining CTEs

2019-01-28 Thread Peter Eisentraut
On 28/01/2019 21:35, Tom Lane wrote: > Conceivably we could make it work without the parens: > > WITH ctename AS [ option = value [ , ] ] ( query ) > > which for the immediate feature I'd be tempted to spell as > > WITH ctename AS [ materialize = on/off ] ( query ... ) > > I

Re: Early WIP/PoC for inlining CTEs

2019-01-28 Thread Tom Lane
Robert Haas writes: > However, generally we have not had great luck with just sticking > keywords in there (cf. VACUUM, ANALYZE, EXPLAIN, COPY) which is why I > suggested using a flexible syntax with parenthesized options. Fair, except that as you then proceed to point out, that does not work

Re: Early WIP/PoC for inlining CTEs

2019-01-28 Thread Robert Haas
On Sun, Jan 27, 2019 at 8:23 AM Andrew Gierth wrote: > I think it makes more sense to say > that we never inline if MATERIALIZED is specified, that we always inline > if NOT MATERIALIZED is specified, and that if neither is specified the > planner will choose (but perhaps note that currently it

Re: Early WIP/PoC for inlining CTEs

2019-01-28 Thread Robert Haas
On Mon, Jan 21, 2019 at 10:28 AM Tom Lane wrote: > Andreas Karlsson writes: > > I have a minor biksheddish question about the syntax. > > You proposed: > > WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query > > While Andrew proposed: > > WITH cte_name AS [[NOT] MATERIALIZED] (query)

Re: Early WIP/PoC for inlining CTEs

2019-01-27 Thread Andreas Karlsson
On 1/26/19 11:55 PM, Tom Lane wrote:> Hearing no immediate pushback on that proposal, I went ahead and made a version of the patch that does it like that, as attached. I also took a stab at documenting it fully. Thanks! This version of the patch looks solid, including the documentation. The

Re: Early WIP/PoC for inlining CTEs

2019-01-27 Thread Andreas Karlsson
On 1/27/19 4:21 PM, Tom Lane wrote: Andrew Gierth writes: I'm not sure we should nail down the rule that the absence of NOT MATERIALIZED will mean a multiply-referenced CTE is evaluated once. One would hope that in the future the planner might be taught to inline or not in that case depending

Re: Early WIP/PoC for inlining CTEs

2019-01-27 Thread Tom Lane
Andrew Gierth writes: > I'm not sure we should nail down the rule that the absence of NOT > MATERIALIZED will mean a multiply-referenced CTE is evaluated once. One > would hope that in the future the planner might be taught to inline or > not in that case depending on cost. I think it makes more

Re: Early WIP/PoC for inlining CTEs

2019-01-27 Thread Andrew Gierth
> "Tom" == Tom Lane writes: Tom> I was interested to find, while writing the docs, that it's a real Tom> struggle to invent plausible reasons to write MATERIALIZED given Tom> the above specification. You pretty much have to have lied to the Tom> planner, eg by making a volatile function

Re: Early WIP/PoC for inlining CTEs

2019-01-26 Thread Marko Tiikkaja
On Sat, Jan 26, 2019 at 12:22 AM Tom Lane wrote: > Therefore, I'm reversing my previous opinion that we should not have > an explicit NOT MATERIALIZED option. I think we should add that, and > the behavior ought to be: > > * No option given: inline if there's exactly one reference. > > * With

Re: Early WIP/PoC for inlining CTEs

2019-01-26 Thread Tom Lane
I wrote: > Therefore, I'm reversing my previous opinion that we should not have > an explicit NOT MATERIALIZED option. I think we should add that, and > the behavior ought to be: > * No option given: inline if there's exactly one reference. > * With MATERIALIZED: never inline. > * With NOT

Re: Early WIP/PoC for inlining CTEs

2019-01-25 Thread Tom Lane
Andreas Karlsson writes: > [ inlining-ctes-v8.patch ] I went ahead and pushed the stuff about QTW_EXAMINE_RTES_BEFORE/_AFTER, because that seems like an independent change with other possible uses. Attached is an updated version of the rest of the patch, with mostly cosmetic changes. I've not

Re: Early WIP/PoC for inlining CTEs

2019-01-21 Thread Gavin Flower
On 22/01/2019 02:40, Andreas Karlsson wrote: On 1/18/19 9:34 PM, Robert Haas wrote: On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson wrote: On 1/11/19 8:10 PM, Robert Haas wrote: WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query... Hm, when would one want "NOT MATERIALIZED"? I am

Re: Early WIP/PoC for inlining CTEs

2019-01-21 Thread Andreas Karlsson
On 1/10/19 2:28 AM, Andreas Karlsson wrote: Here is a new version of the patch with added tests, improved comments, some minor code cleanup and most importantly slightly changed logic for when we should inline. Add ctematerialized to the JumbleExpr() in pg_stat_statements on suggestion from

Re: Early WIP/PoC for inlining CTEs

2019-01-21 Thread Tom Lane
Andreas Karlsson writes: > I have a minor biksheddish question about the syntax. > You proposed: > WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query > While Andrew proposed: > WITH cte_name AS [[NOT] MATERIALIZED] (query) main_query > Do people have any preference between these two? FWIW,

Re: Early WIP/PoC for inlining CTEs

2019-01-21 Thread Andreas Karlsson
On 1/18/19 9:34 PM, Robert Haas wrote: On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson wrote: On 1/11/19 8:10 PM, Robert Haas wrote: WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query... Hm, when would one want "NOT MATERIALIZED"? I am not sure I see the usefulness of forcing

Re: Early WIP/PoC for inlining CTEs

2019-01-18 Thread Robert Haas
On Fri, Jan 18, 2019 at 3:42 PM Andres Freund wrote: > On 2019-01-18 15:34:46 -0500, Robert Haas wrote: > > On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson wrote: > > > On 1/11/19 8:10 PM, Robert Haas wrote: > > > > WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query... > > > > > > Hm,

Re: Early WIP/PoC for inlining CTEs

2019-01-18 Thread Andres Freund
On 2019-01-18 15:34:46 -0500, Robert Haas wrote: > On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson wrote: > > On 1/11/19 8:10 PM, Robert Haas wrote: > > > WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query... > > > > Hm, when would one want "NOT MATERIALIZED"? I am not sure I see the > >

Re: Early WIP/PoC for inlining CTEs

2019-01-18 Thread Robert Haas
On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson wrote: > On 1/11/19 8:10 PM, Robert Haas wrote: > > WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query... > > Hm, when would one want "NOT MATERIALIZED"? I am not sure I see the > usefulness of forcing inlining other than if we by default do

Re: Early WIP/PoC for inlining CTEs

2019-01-17 Thread Tom Lane
Andreas Karlsson writes: > On 1/11/19 8:10 PM, Robert Haas wrote: >> WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query... > Hm, when would one want "NOT MATERIALIZED"? I am not sure I see the > usefulness of forcing inlining other than if we by default do not inline > when a CTE is

Re: Early WIP/PoC for inlining CTEs

2019-01-17 Thread Andreas Karlsson
On 1/11/19 8:10 PM, Robert Haas wrote: WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query... Hm, when would one want "NOT MATERIALIZED"? I am not sure I see the usefulness of forcing inlining other than if we by default do not inline when a CTE is referenced multiple times. Do you

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Pavel Stehule
pá 11. 1. 2019 v 20:11 odesílatel Robert Haas napsal: > On Fri, Jan 11, 2019 at 2:04 PM Andres Freund wrote: > > > Maybe we could consider a more extensible syntax that is attached to > > > the contained SELECT rather than the containing WITH. Then CTEs would > > > be less special; there'd be

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Merlin Moncure
On Tue, Jul 24, 2018 at 6:10 PM Andres Freund wrote: > My read of the concensus (in which I am in the majority, so I might be > biased) is that we do want inlining to be the default. We were thinking > that it'd be necessary to provide a way to force inlining on the SQL > level for individual

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Mike Rylander
On Fri, Jan 11, 2019 at 2:10 PM Robert Haas wrote: > > On Fri, Jan 11, 2019 at 2:04 PM Andres Freund wrote: > > > Maybe we could consider a more extensible syntax that is attached to > > > the contained SELECT rather than the containing WITH. Then CTEs would > > > be less special; there'd be a

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Robert Haas
On Fri, Jan 11, 2019 at 2:04 PM Andres Freund wrote: > > Maybe we could consider a more extensible syntax that is attached to > > the contained SELECT rather than the containing WITH. Then CTEs would > > be less special; there'd be a place to put hints controlling top-level > > queries,

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Andres Freund
Hi, On 2019-01-12 07:58:39 +1300, Thomas Munro wrote: > On Sat, Jan 12, 2019 at 7:19 AM Andres Freund wrote: > > On 2019-01-11 11:12:25 -0500, Robert Haas wrote: > > > I actually think that we should go "all in" here and allow the user to > > > specify either that they want materialization or

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Thomas Munro
On Sat, Jan 12, 2019 at 7:19 AM Andres Freund wrote: > On 2019-01-11 11:12:25 -0500, Robert Haas wrote: > > I actually think that we should go "all in" here and allow the user to > > specify either that they want materialization or that they don't want > > materialization. If they specify

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Robert Haas
On Fri, Jan 11, 2019 at 1:49 PM David Fetter wrote: > I don't see those as the same thing even slightly. Functions are > Turing complete, generally speaking, which means that unless we send > along those descriptors, we're asking the planner to solve the Halting > Problem. So... your argument is

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread David Fetter
On Fri, Jan 11, 2019 at 11:12:25AM -0500, Robert Haas wrote: > > I know this is a thorny topic, but I have to say that I am uneasy > > about the MATERIALIZED syntax. Here's how you write that in some > > other RDBMS that loves hints: > > > > WITH foo AS (SELECT /*+ MATERIALIZE */ ...) > > > > I

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Andres Freund
On 2019-01-11 11:12:25 -0500, Robert Haas wrote: > I actually think that we should go "all in" here and allow the user to > specify either that they want materialization or that they don't want > materialization. If they specify neither, then we make some decision > which we may change in a

Re: Early WIP/PoC for inlining CTEs

2019-01-11 Thread Robert Haas
> I know this is a thorny topic, but I have to say that I am uneasy > about the MATERIALIZED syntax. Here's how you write that in some > other RDBMS that loves hints: > > WITH foo AS (SELECT /*+ MATERIALIZE */ ...) > > I understood that it was a long standing project policy that we don't > want

Re: Early WIP/PoC for inlining CTEs

2019-01-09 Thread Thomas Munro
On Thu, Jan 10, 2019 at 2:28 PM Andreas Karlsson wrote: > 2. Feedback on the new syntax. I am personally fine with the current > syntax, but it was just something I just quickly hacked together to move > the patch forward and which also solved my personal uses cases. Thanks for working on this.

Re: Early WIP/PoC for inlining CTEs

2019-01-09 Thread Andreas Karlsson
Here is a new version of the patch with added tests, improved comments, some minor code cleanup and most importantly slightly changed logic for when we should inline. The current strategy is to always inline unless the CTE is recursive or it has side effects, i.e. it is a DML query, it

Re: Early WIP/PoC for inlining CTEs

2019-01-01 Thread Andreas Karlsson
On 1/1/19 1:42 AM, Andrew Gierth wrote: "Andreas" == Andreas Karlsson writes: Andreas> I believe I have fixed these except for the comment on the Andreas> conditions for when we inline. Andreas> Andrew Gierth: Why did you chose to not inline on FOR UPDATE Andreas> but inline volatile

Re: Early WIP/PoC for inlining CTEs

2019-01-01 Thread Andreas Karlsson
On 1/1/19 3:18 AM, Andrew Gierth wrote: I had a comment around here which seems to have been lost: * Secondly, views (and explicit subqueries) currently have * different behaviour w.r.t. SELECT FOR UPDATE than CTEs do. A * FOR UPDATE clause is treated as extending into views and *

Re: Early WIP/PoC for inlining CTEs

2018-12-31 Thread Andrew Gierth
> "Andreas" == Andreas Karlsson writes: Andreas> + if (rte->rtekind == RTE_CTE && Andreas> + strcmp(rte->ctename, context->ctename) == 0 && Andreas> + rte->ctelevelsup == context->levelsup) Andreas> + { Andreas> + Query *newquery = copyObject(context->ctequery);

Re: Early WIP/PoC for inlining CTEs

2018-12-31 Thread Andrew Gierth
> "Andreas" == Andreas Karlsson writes: Andreas> I believe I have fixed these except for the comment on the Andreas> conditions for when we inline. Andreas> Andrew Gierth: Why did you chose to not inline on FOR UPDATE Andreas> but inline volatile functions? I feel that this might be

Re: Early WIP/PoC for inlining CTEs

2018-12-31 Thread Andreas Karlsson
On 11/16/18 10:15 PM, Tom Lane wrote: > I took a little bit of a look through this. Some thoughts: Thanks for the review! I have decided to pick up this patch and work on it since nothing has happened in a while. Here is a new version with most of the feedback fixed. * This is not the

Re: Early WIP/PoC for inlining CTEs

2018-11-18 Thread Craig Ringer
On Sat, 17 Nov 2018 at 10:12, Stephen Frost wrote: > Greetings, > > * Andrew Gierth (and...@tao11.riddles.org.uk) wrote: > > > "Tom" == Tom Lane writes: > > > > >> [ inlining-ctes-v5.patch ] > > > > Tom> I took a little bit of a look through this. Some thoughts: > > > > Tom> * I think

Re: Early WIP/PoC for inlining CTEs

2018-11-16 Thread Stephen Frost
Greetings, * Andrew Gierth (and...@tao11.riddles.org.uk) wrote: > > "Tom" == Tom Lane writes: > > >> [ inlining-ctes-v5.patch ] > > Tom> I took a little bit of a look through this. Some thoughts: > > Tom> * I think it'd be a good idea if we made OFFSET/LIMIT in a CTE be > Tom> an

Re: Early WIP/PoC for inlining CTEs

2018-11-16 Thread Tom Lane
Andrew Gierth writes: > "Tom" == Tom Lane writes: > Tom> * I have no faith in the idea that we can skip doing a copyObject > Tom> on the inlined subquery, except maybe in the case where we know > Tom> there's exactly one reference. > The code doesn't do a copyObject on the query if there are

Re: Early WIP/PoC for inlining CTEs

2018-11-16 Thread Andrew Gierth
> "Tom" == Tom Lane writes: >> [ inlining-ctes-v5.patch ] Tom> I took a little bit of a look through this. Some thoughts: Tom> * I think it'd be a good idea if we made OFFSET/LIMIT in a CTE be Tom> an alternate way of keeping it from being inlined. As the patch Tom> stands, if that's

Re: Early WIP/PoC for inlining CTEs

2018-11-16 Thread David Fetter
On Fri, Nov 16, 2018 at 04:15:10PM -0500, Tom Lane wrote: > Andreas Karlsson writes: > > [ inlining-ctes-v5.patch ] > > I took a little bit of a look through this. Some thoughts: > > * I think it'd be a good idea if we made OFFSET/LIMIT in a CTE be an > alternate way of keeping it from being

Re: Early WIP/PoC for inlining CTEs

2018-11-16 Thread Tom Lane
Andreas Karlsson writes: > [ inlining-ctes-v5.patch ] I took a little bit of a look through this. Some thoughts: * I think it'd be a good idea if we made OFFSET/LIMIT in a CTE be an alternate way of keeping it from being inlined. As the patch stands, if that's the behavior you want, you have

Re: Early WIP/PoC for inlining CTEs

2018-10-05 Thread Andrew Gierth
> "David" == David Fetter writes: >> Consider the difference between (in the absence of CTE inlining): >> >> -- inline subquery with no optimization barrier (qual may be pushed down) >> select * from (select x from y) s where x=1; David> ...and doesn't need to materialize all of y,

Re: Early WIP/PoC for inlining CTEs

2018-10-05 Thread David Fetter
On Fri, Oct 05, 2018 at 01:40:05AM +0100, Andrew Gierth wrote: > > "Andreas" == Andreas Karlsson writes: > > > On 10/03/2018 05:57 PM, David Fetter wrote: > >> Is there any meaningful distinction between "inlining," by which I > >> mean converting to a subquery, and predicate pushdown,

Re: Early WIP/PoC for inlining CTEs

2018-10-04 Thread Andrew Gierth
> "Andreas" == Andreas Karlsson writes: > On 10/03/2018 05:57 PM, David Fetter wrote: >> Is there any meaningful distinction between "inlining," by which I >> mean converting to a subquery, and predicate pushdown, which >> would happen at least for a first cut, at the rewrite stage?

Re: Early WIP/PoC for inlining CTEs

2018-10-04 Thread David Fetter
On Thu, Oct 04, 2018 at 11:22:32AM +0200, Andreas Karlsson wrote: > On 10/03/2018 05:57 PM, David Fetter wrote: > >Is there any meaningful distinction between "inlining," by which I > >mean converting to a subquery, and predicate pushdown, which > >would happen at least for a first cut, at the

Re: Early WIP/PoC for inlining CTEs

2018-10-04 Thread Andreas Karlsson
On 10/03/2018 05:57 PM, David Fetter wrote: Is there any meaningful distinction between "inlining," by which I mean converting to a subquery, and predicate pushdown, which would happen at least for a first cut, at the rewrite stage? Sorry, but I do not think I understand your question. The

Re: Early WIP/PoC for inlining CTEs

2018-10-03 Thread David Fetter
On Tue, Oct 02, 2018 at 12:03:06PM +0200, Andreas Karlsson wrote: > Hi, > > Here is an updated patch which adds some simple syntax for adding the > optimization barrier. For example: > > WITH x AS MATERIALIZED ( >SELECT 1 > ) > SELECT * FROM x; > > Andreas This is great! Is there any

Re: Early WIP/PoC for inlining CTEs

2018-10-02 Thread Andreas Karlsson
Hi, Here is an updated patch which adds some simple syntax for adding the optimization barrier. For example: WITH x AS MATERIALIZED ( SELECT 1 ) SELECT * FROM x; Andreas diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index

Re: Early WIP/PoC for inlining CTEs

2018-10-01 Thread Andreas Karlsson
On 07/27/2018 10:10 AM, David Fetter wrote: On Fri, Jul 27, 2018 at 02:55:26PM +1200, Thomas Munro wrote: On Thu, Jul 26, 2018 at 7:14 AM, David Fetter wrote: Please find attached the next version, which passes 'make check'. ... but not 'make check-world' (contrib/postgres_fdw's EXPLAIN is

Re: Early WIP/PoC for inlining CTEs

2018-08-07 Thread Andres Freund
Hi, On 2018-08-08 16:55:22 +1200, Thomas Munro wrote: > On Fri, Jul 27, 2018 at 8:10 PM, David Fetter wrote: > > On Fri, Jul 27, 2018 at 02:55:26PM +1200, Thomas Munro wrote: > >> On Thu, Jul 26, 2018 at 7:14 AM, David Fetter wrote: > >> > Please find attached the next version, which passes

Re: Early WIP/PoC for inlining CTEs

2018-08-07 Thread Thomas Munro
On Fri, Jul 27, 2018 at 8:10 PM, David Fetter wrote: > On Fri, Jul 27, 2018 at 02:55:26PM +1200, Thomas Munro wrote: >> On Thu, Jul 26, 2018 at 7:14 AM, David Fetter wrote: >> > Please find attached the next version, which passes 'make check'. >> >> ... but not 'make check-world'

Re: Early WIP/PoC for inlining CTEs

2018-07-27 Thread Andrew Gierth
> "Andreas" == Andreas Karlsson writes: >> WITH ctename AS [[NOT] MATERIALIZED] (query) Andreas> I think "NOT MATERIALIZED" would be a bit misleading since the Andreas> planner may choose to materialize anyway, It would certainly be possible to make an explicit NOT MATERIALIZED override

Re: Early WIP/PoC for inlining CTEs

2018-07-27 Thread David Fetter
On Fri, Jul 27, 2018 at 02:55:26PM +1200, Thomas Munro wrote: > On Thu, Jul 26, 2018 at 7:14 AM, David Fetter wrote: > > Please find attached the next version, which passes 'make check'. > > ... but not 'make check-world' (contrib/postgres_fdw's EXPLAIN is different). Please find attached a

Re: Early WIP/PoC for inlining CTEs

2018-07-26 Thread Thomas Munro
On Thu, Jul 26, 2018 at 7:14 AM, David Fetter wrote: > Please find attached the next version, which passes 'make check'. ... but not 'make check-world' (contrib/postgres_fdw's EXPLAIN is different). -- Thomas Munro http://www.enterprisedb.com

Re: Early WIP/PoC for inlining CTEs

2018-07-26 Thread David Fetter
On Thu, Jul 26, 2018 at 02:51:53PM +0200, Andreas Karlsson wrote: > On 07/25/2018 06:08 PM, Andrew Gierth wrote: > >WITH ctename AS [[NOT] MATERIALIZED] (query) > > I think "NOT MATERIALIZED" would be a bit misleading since the > planner may choose to materialize anyway, so I suggest skipping

Re: Early WIP/PoC for inlining CTEs

2018-07-26 Thread Andreas Karlsson
On 07/25/2018 06:08 PM, Andrew Gierth wrote: WITH ctename AS [[NOT] MATERIALIZED] (query) I think "NOT MATERIALIZED" would be a bit misleading since the planner may choose to materialize anyway, so I suggest skipping that part of the syntax unless there is a really strong reason for having

Re: Early WIP/PoC for inlining CTEs

2018-07-25 Thread David Fetter
On Wed, Jul 25, 2018 at 04:18:42PM +0100, Andrew Gierth wrote: > > "David" == David Fetter writes: > > David> Please find attached a version rebased atop 167075be3ab1547e18 > David> with what I believe are appropriate changes to regression test > David> output. The other changes to the

Re: Early WIP/PoC for inlining CTEs

2018-07-25 Thread Nico Williams
On Tue, Jul 24, 2018 at 07:57:49PM -0400, Tom Lane wrote: > Andres Freund writes: > > On 2018-07-24 19:49:19 -0400, Tom Lane wrote: > >> However, a singly-referenced SELECT CTE could reasonably be treated as > >> equivalent to a sub-select-in-FROM, and then you would have the same > >> mechanisms

Re: Early WIP/PoC for inlining CTEs

2018-07-25 Thread Nico Williams
On Wed, Jul 25, 2018 at 05:08:43PM +0100, Andrew Gierth wrote: > Nico> It is possible to add a keyword for this purpose in the WITH syntax: > > Nico> WITH VIEW (...) AS a_view > > The existing (and standard) syntax is WITH ctename AS (query). Oy, I flubbed that up. > Syntaxes that have

Re: Early WIP/PoC for inlining CTEs

2018-07-25 Thread Andrew Gierth
> "Nico" == Nico Williams writes: Nico> It is possible to add a keyword for this purpose in the WITH syntax: Nico> WITH VIEW (...) AS a_view The existing (and standard) syntax is WITH ctename AS (query). Syntaxes that have been suggested for explicitly controlling the

Re: Early WIP/PoC for inlining CTEs

2018-07-25 Thread Nico Williams
On Wed, Jul 25, 2018 at 07:42:37AM +0200, David Fetter wrote: > Please find attached a version rebased atop 167075be3ab1547e18 with > what I believe are appropriate changes to regression test output. The > other changes to the regression tests output are somewhat puzzling, as > they change the

Re: Early WIP/PoC for inlining CTEs

2018-07-25 Thread Andrew Gierth
> "David" == David Fetter writes: David> Please find attached a version rebased atop 167075be3ab1547e18 David> with what I believe are appropriate changes to regression test David> output. The other changes to the regression tests output are David> somewhat puzzling, as they change the

Re: Early WIP/PoC for inlining CTEs

2018-07-24 Thread Andres Freund
On 2018-07-25 01:08:44 +0100, Andrew Gierth wrote: > > "Andres" == Andres Freund writes: > > Andres> Even in queries with a non-0 OFFSET you can push down in a > Andres> number of cases, > > really? Yea. I guess it's a bit dependant on what kind of behaviour you consider as "pushing

Re: Early WIP/PoC for inlining CTEs

2018-07-24 Thread Andrew Gierth
> "Andres" == Andres Freund writes: Andres> Even in queries with a non-0 OFFSET you can push down in a Andres> number of cases, really? -- Andrew (irc:RhodiumToad)

  1   2   >