Thread: Early WIP/PoC for inlining CTEs

About a year ago I was briefly in discussion/collaboration with Adam Sah
regarding the topic of inlining CTEs into the query rather than treating
them as optimization barriers. We didn't take it very far (he sent me
some stuff, I wrote some stuff and sent it back, things kind of got
dropped at that point); but there's been some recent discussion of this
and some people have expressed an interest in seeing the code.

So I'm posting the parts that I wrote for the benefit of anyone wanting
to pick up the issue again. The assumption of this code is that some
form of syntax would exist to mark materialized CTEs and set the
"ctematerialized" flag.

I haven't rebased this or tested it since last year; this patch is
against b81eba6a65.

Posted for discussion, further development, criticism, whatever; feel
free to include this (with credit) in any relevant patch. Consider this
released under the PG license.

-- 
Andrew (irc:RhodiumToad)

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index c1a83ca909..393c673164 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2474,6 +2474,7 @@ _copyCommonTableExpr(const CommonTableExpr *from)
     COPY_NODE_FIELD(ctequery);
     COPY_LOCATION_FIELD(location);
     COPY_SCALAR_FIELD(cterecursive);
+    COPY_SCALAR_FIELD(ctematerialized);
     COPY_SCALAR_FIELD(cterefcount);
     COPY_NODE_FIELD(ctecolnames);
     COPY_NODE_FIELD(ctecoltypes);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 7a700018e7..2112560871 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2781,6 +2781,7 @@ _equalCommonTableExpr(const CommonTableExpr *a, const CommonTableExpr *b)
     COMPARE_NODE_FIELD(ctequery);
     COMPARE_LOCATION_FIELD(location);
     COMPARE_SCALAR_FIELD(cterecursive);
+    COMPARE_SCALAR_FIELD(ctematerialized);
     COMPARE_SCALAR_FIELD(cterefcount);
     COMPARE_NODE_FIELD(ctecolnames);
     COMPARE_NODE_FIELD(ctecoltypes);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 43d62062bc..2df4e6300a 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -3010,6 +3010,7 @@ _outCommonTableExpr(StringInfo str, const CommonTableExpr *node)
     WRITE_NODE_FIELD(ctequery);
     WRITE_LOCATION_FIELD(location);
     WRITE_BOOL_FIELD(cterecursive);
+    WRITE_BOOL_FIELD(ctematerialized);
     WRITE_INT_FIELD(cterefcount);
     WRITE_NODE_FIELD(ctecolnames);
     WRITE_NODE_FIELD(ctecoltypes);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index ccb6a1f4ac..4705ea19d7 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -404,6 +404,7 @@ _readCommonTableExpr(void)
     READ_NODE_FIELD(ctequery);
     READ_LOCATION_FIELD(location);
     READ_BOOL_FIELD(cterecursive);
+    READ_BOOL_FIELD(ctematerialized);
     READ_INT_FIELD(cterefcount);
     READ_NODE_FIELD(ctecolnames);
     READ_NODE_FIELD(ctecoltypes);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 1103984779..63e5828ef9 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -1129,6 +1129,141 @@ hash_ok_operator(OpExpr *expr)
 }
 
 
+struct inline_cte_walker_ctx
+{
+    const char *ctename;
+    int levelsup;
+    int refcount;
+    Query *ctequery;
+    CommonTableExpr *cte;
+};
+
+static bool inline_cte_walker(Node *node, void *ctxp)
+{
+    struct inline_cte_walker_ctx *ctx = ctxp;
+
+    if (!node)
+        return false;
+
+    if (IsA(node, Query))
+    {
+        /*
+         * This one is a bit tricky. It's our job to handle the recursion here,
+         * but we do some of the lifting normally handled by query_tree_walker
+         * in order to get the sequence of operations right.
+         *
+         * First, if the Query we're looking at is the one containing our CTE
+         * definition, then we don't need to recurse into our own CTE or CTEs
+         * that are earlier in the list than ours (since cteList has been
+         * sorted for us into dependency order). We could check whether a
+         * nested query is hiding ours, but that seems too much of an edge case
+         * to be worth optimizing (the levelsup check will ensure we don't
+         * replace any CTEs improperly). So we scan the cteList ourselves
+         * rather than having query_tree_walker do it.
+         *
+         * Second, we want to walk the rangetable _before_ replacing any
+         * RTE_CTE nodes, in order to avoid re-walking the subquery we just
+         * inlined. (range_table_walker, if told to visit the RTE nodes at all,
+         * visits them before their content.) So we have range_table_walker
+         * ignore the RTE nodes themselves and only walk their contents.
+         *
+         * Third, we scan the rangetable for RTE_CTE nodes to replace.
+         *
+         * Fourth, we use query_tree_walker to find and walk the rest of the
+         * query, telling it to ignore the rangetable and CTEs.
+         *
+         * Note that ctx->levelsup is -1 on entry the first time, since we need
+         * the incremented value to be 0 when scanning the content of the query
+         * containing the definition.
+         */
+        Query *query = castNode(Query, node);
+        ListCell *lc;
+        bool do_replace = ctx->levelsup >= 0;
+
+        ctx->levelsup++;
+
+        foreach (lc, query->cteList)
+        {
+            CommonTableExpr *cte = lfirst_node(CommonTableExpr, lc);
+
+            if (!do_replace && strcmp(cte->ctename, ctx->ctename) == 0)
+                do_replace = true;
+            else if (do_replace)
+                inline_cte_walker(cte->ctequery, ctxp);
+        }
+
+        range_table_walker(query->rtable, inline_cte_walker, ctxp, 0);
+
+        foreach (lc, query->rtable)
+        {
+            RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
+
+            if (rte->rtekind == RTE_CTE &&
+                strcmp(rte->ctename, ctx->ctename) == 0 &&
+                rte->ctelevelsup == ctx->levelsup)
+            {
+                Query *newquery = ctx->ctequery;
+
+                /*
+                 * We need to do some work here that view rewrite does not, and
+                 * in turn we do not do some work that view rewrite does.
+                 *
+                 * Firstly, views can't have outer references but CTEs can
+                 * (especially in the case of CTEs referencing other CTEs), so
+                 * we need to fix up all levelsup attributes inside the CTE
+                 * query.
+                 *
+                 * 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
+                 * subqueries, but not into CTEs. We preserve this distinction
+                 * by not trying to push rowmarks into the new subquery.
+                 *
+                 * We avoid copyObject if possible because subquery processing
+                 * copies the query too.
+                 */
+                if (ctx->levelsup > 0)
+                {
+                    newquery = copyObject(newquery);
+                    IncrementVarSublevelsUp((Node *) newquery, ctx->levelsup, 1);
+                }
+
+                /*
+                 * Here's where we do the actual substitution.
+                 */
+                rte->rtekind = RTE_SUBQUERY;
+                rte->subquery = newquery;
+                rte->security_barrier = false;
+
+                ctx->refcount--;
+            }
+        }
+
+        query_tree_walker(query, inline_cte_walker, ctxp,
+                          QTW_IGNORE_RANGE_TABLE | QTW_IGNORE_CTE_SUBQUERIES);
+
+        ctx->levelsup--;
+
+        return false;
+    }
+
+    return expression_tree_walker(node, inline_cte_walker, ctxp);
+}
+
+static void inline_cte(PlannerInfo *root, CommonTableExpr *cte)
+{
+    struct inline_cte_walker_ctx ctx;
+    ctx.ctequery = castNode(Query, cte->ctequery);
+    ctx.ctename = cte->ctename;
+    ctx.refcount = cte->cterefcount;
+    ctx.levelsup = -1;
+    ctx.cte = cte;
+    inline_cte_walker((Node *) root->parse, &ctx);
+    /* we must replace all references */
+    Assert(ctx.refcount == 0);
+}
+
+
 /*
  * SS_process_ctes: process a query's WITH list
  *
@@ -1167,6 +1302,23 @@ SS_process_ctes(PlannerInfo *root)
         }
 
         /*
+         * Consider inlining the CTE rather than planning it separately.
+         *
+         * XXX this likely needs some additional checks.
+         */
+        if (cmdType == CMD_SELECT &&
+            !cte->ctematerialized &&
+            !cte->cterecursive &&
+            (castNode(Query, cte->ctequery)->rowMarks == NIL))
+        {
+            inline_cte(root, cte);
+
+            /* Make a dummy entry in cte_plan_ids */
+            root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
+            continue;
+        }
+
+        /*
          * Copy the source Query node.  Probably not necessary, but let's keep
          * this similar to make_subplan.
          */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 732e5d6788..901a3a295f 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1372,6 +1372,7 @@ typedef struct CommonTableExpr
     int            location;        /* token location, or -1 if unknown */
     /* These fields are set during parse analysis: */
     bool        cterecursive;    /* is this CTE actually recursive? */
+    bool        ctematerialized;    /* is this an optimization fence? */
     int            cterefcount;    /* number of RTEs referencing this CTE
                                  * (excluding internal self-references) */
     List       *ctecolnames;    /* list of output column names */

On 2018-07-24 18:03:43 -0500, Jeremy Finzel wrote:
> On Tue, Jul 24, 2018 at 5:28 PM Andrew Gierth <andrew@tao11.riddles.org.uk>
> wrote:
> 
> > About a year ago I was briefly in discussion/collaboration with Adam Sah
> > regarding the topic of inlining CTEs into the query rather than treating
> > them as optimization barriers. We didn't take it very far (he sent me
> > some stuff, I wrote some stuff and sent it back, things kind of got
> > dropped at that point); but there's been some recent discussion of this
> > and some people have expressed an interest in seeing the code.
> >
> > So I'm posting the parts that I wrote for the benefit of anyone wanting
> > to pick up the issue again. The assumption of this code is that some
> > form of syntax would exist to mark materialized CTEs and set the
> > "ctematerialized" flag.
> >
> > I haven't rebased this or tested it since last year; this patch is
> > against b81eba6a65.
> >
> > Posted for discussion, further development, criticism, whatever; feel
> > free to include this (with credit) in any relevant patch. Consider this
> > released under the PG license.
> >
> > --
> > Andrew (irc:RhodiumToad)
> >
> > In our environment we often want this to be a fence.  For example it can
> be used to only have smaller numbers of joins in each cte and not hit the
> join collapse limit, or when we really know more about the subquery than
> the optimizer and have something really specific there .  So in general I
> would not want the default functionality to change all of the queries we
> have already written with this in mind. I do however like the idea of this
> feature being an option, but I would question whether it perhaps worked the
> other way around where you have to mark a CTE as not being a fence.

This essentially has been discussed already:
http://archives.postgresql.org/message-id/5351711493487900%40web53g.yandex.ru

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 CTEs.


> Curious what other RDBMSs do here?

They largely inline by default.

Greetings,

Andres Freund

>>>>> "Andres" == Andres Freund <andres@anarazel.de> writes:

 Andres> My read of the concensus (in which I am in the majority, so I
 Andres> might be biased) is that we do want inlining to be the default.
 Andres> We were thinking that it'd be necessary to provide a way to
 Andres> force inlining on the SQL level for individual CTEs.

For the record, here is where it came up in the original discussion
exactly 10 years ago when the feature was being added:

https://www.postgresql.org/message-id/flat/87myk1rg4z.fsf%40news-spur.riddles.org.uk#35efe8ef370518116c38ddc550632aa0

-- 
Andrew (irc:RhodiumToad)

Andres Freund <andres@anarazel.de> writes:
> On 2018-07-24 18:03:43 -0500, Jeremy Finzel wrote:
>> On Tue, Jul 24, 2018 at 5:28 PM Andrew Gierth <andrew@tao11.riddles.org.uk>
>> wrote:
>>> Posted for discussion, further development, criticism, whatever; feel
>>> free to include this (with credit) in any relevant patch. Consider this
>>> released under the PG license.

>> In our environment we often want this to be a fence.  For example it can
>> be used to only have smaller numbers of joins in each cte and not hit the
>> join collapse limit, or when we really know more about the subquery than
>> the optimizer and have something really specific there .  So in general I
>> would not want the default functionality to change all of the queries we
>> have already written with this in mind. I do however like the idea of this
>> feature being an option, but I would question whether it perhaps worked the
>> other way around where you have to mark a CTE as not being a fence.

> This essentially has been discussed already:
> http://archives.postgresql.org/message-id/5351711493487900%40web53g.yandex.ru
> 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 CTEs.

We can't inline wCTEs (those containing insert/update/delete) without
risk of semantics change.  I'd also not favor changing the semantics
for CTEs that are read more than once by the parent query.  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
for preventing inlining as you do for those cases, e.g. OFFSET 0.
And sticking in OFFSET 0 would be backwards-compatible too: your
code would still work the same in older releases, unlike if we invent
new syntax for this.

So if we're going to go in this direction, that's pretty much how
I'd envision it working.

>> Curious what other RDBMSs do here?

> They largely inline by default.

Even for multi-referenced CTEs?

            regards, tom lane

On 25/07/18 11:10, Andres Freund wrote:
> On 2018-07-24 18:03:43 -0500, Jeremy Finzel wrote:
>> On Tue, Jul 24, 2018 at 5:28 PM Andrew Gierth <andrew@tao11.riddles.org.uk>
>> wrote:
>>
[...]
>>> In our environment we often want this to be a fence.  For example it can
[...]
> This essentially has been discussed already:
> http://archives.postgresql.org/message-id/5351711493487900%40web53g.yandex.ru
>
> 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 CTEs.
>
>
>> Curious what other RDBMSs do here?
> They largely inline by default.
>
> Greetings,
>
> Andres Freund
>
If I'd not read anything about CTE's being a fence, I would have 
implicitly assumed that they were optimised together with the main part 
of the SQL statement, and I suspect that is the case for most people.

So I'm very much a favour of optimisation of CTE's being the default.


Cheers,
Gavin

Hi,

On 2018-07-24 19:49:19 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2018-07-24 18:03:43 -0500, Jeremy Finzel wrote:
> >> On Tue, Jul 24, 2018 at 5:28 PM Andrew Gierth <andrew@tao11.riddles.org.uk>
> >> wrote:
> >>> Posted for discussion, further development, criticism, whatever; feel
> >>> free to include this (with credit) in any relevant patch. Consider this
> >>> released under the PG license.
> 
> >> In our environment we often want this to be a fence.  For example it can
> >> be used to only have smaller numbers of joins in each cte and not hit the
> >> join collapse limit, or when we really know more about the subquery than
> >> the optimizer and have something really specific there .  So in general I
> >> would not want the default functionality to change all of the queries we
> >> have already written with this in mind. I do however like the idea of this
> >> feature being an option, but I would question whether it perhaps worked the
> >> other way around where you have to mark a CTE as not being a fence.
> 
> > This essentially has been discussed already:
> > http://archives.postgresql.org/message-id/5351711493487900%40web53g.yandex.ru
> > 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 CTEs.
> 
> We can't inline wCTEs (those containing insert/update/delete) without
> risk of semantics change.

Right.


> I'd also not favor changing the semantics for CTEs that are read more
> than once by the parent query.

I think medium term it'd be good to do a cost based analysis for that. I
think it's fine to not do that immediately, but we should imo keep that
in mind.


> 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 for preventing inlining as you do for those cases,
> e.g. OFFSET 0.  And sticking in OFFSET 0 would be backwards-compatible
> too: your code would still work the same in older releases, unlike if
> we invent new syntax for this.

I still think this is just doubling down on prior mistakes.


> >> Curious what other RDBMSs do here?
> 
> > They largely inline by default.
> 
> Even for multi-referenced CTEs?

I don't know.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> 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 for preventing inlining as you do for those cases,
>> e.g. OFFSET 0.  And sticking in OFFSET 0 would be backwards-compatible
>> too: your code would still work the same in older releases, unlike if
>> we invent new syntax for this.

> I still think this is just doubling down on prior mistakes.

Not following what you think a better alternative is?  I'd be the
first to agree that OFFSET 0 is a hack, but people are used to it.

Assuming that we go for inline-by-default for at least some cases,
there's a separate discussion to be had about whether it's worth
making a planner-control GUC to force the old behavior.  I'm not
very excited about that, but I bet some people will be.

            regards, tom lane

Hi,

On 2018-07-24 19:57:49 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> 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 for preventing inlining as you do for those cases,
> >> e.g. OFFSET 0.  And sticking in OFFSET 0 would be backwards-compatible
> >> too: your code would still work the same in older releases, unlike if
> >> we invent new syntax for this.
> 
> > I still think this is just doubling down on prior mistakes.
> 
> Not following what you think a better alternative is?

An explicit keyword controlling the behaviour. WITH ... foo AS
MATERIALIZED (query) or whatnot.


> I'd be the first to agree that OFFSET 0 is a hack, but people are used
> to it.

So are they to CTEs being blocking.

Consider e.g. the case of being able to push down constraints into CTEs
(which have been inlined to allow for that).  Even in queries with a
non-0 OFFSET you can push down in a number of cases, making CTE inlining
useful.  You can't tack on an OFFSET 0 controlling that, without going
for a superflous subquery that just has an OFFSET 0, which is fairly
ridiculous.  What if we learn to inline subqueries with some offsets?


> Assuming that we go for inline-by-default for at least some cases,
> there's a separate discussion to be had about whether it's worth
> making a planner-control GUC to force the old behavior.  I'm not
> very excited about that, but I bet some people will be.

Yea, I am not either. I think we shouldn't go for it.

Greetings,

Andres Freund

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 Tom> We can't inline wCTEs (those containing insert/update/delete)
 Tom> without risk of semantics change.

Clearly.

 Tom> I'd also not favor changing the semantics for CTEs that are read
 Tom> more than once by the parent query.

This one's more debatable. There will still be cases where a CTE
referenced multiple times will be better inlined.

(It's obviously trivial to make the posted code do it that way, just by
checking cterefcount.)

 Tom> However, a singly-referenced SELECT CTE could reasonably be
 Tom> treated as equivalent to a sub-select-in-FROM,

In the PoC code I also excluded SELECT FOR UPDATE from inlining.

(There's already a difference between how SELECT FOR UPDATE works for
CTEs compared to subqueries and views, the comments mention it)

There might also be some merit in checking for volatility?

-- 
Andrew (irc:RhodiumToad)

>>>>> "Andres" == Andres Freund <andres@anarazel.de> writes:

 Andres> Even in queries with a non-0 OFFSET you can push down in a
 Andres> number of cases,

really?

-- 
Andrew (irc:RhodiumToad)

On 2018-07-25 01:08:44 +0100, Andrew Gierth wrote:
> >>>>> "Andres" == Andres Freund <andres@anarazel.de> 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 down".  I'm doubtful it's worth the analytical complexity on
ensuring it's safe, however.  With knowledge from the outer query you
e.g. can: trim the target list; remove outer joins below the OFFSET 0;
push down a restriction into an outer join below the OFFSET if that's
guaranteed to only return max one row, and not needed if not matching
the restrcition. I'm sure you can come up with more?

Greetings,

Andres Freund

On Tue, Jul 24, 2018 at 11:28:21PM +0100, Andrew Gierth wrote:
> About a year ago I was briefly in discussion/collaboration with Adam Sah
> regarding the topic of inlining CTEs into the query rather than treating
> them as optimization barriers. We didn't take it very far (he sent me
> some stuff, I wrote some stuff and sent it back, things kind of got
> dropped at that point); but there's been some recent discussion of this
> and some people have expressed an interest in seeing the code.
> 
> So I'm posting the parts that I wrote for the benefit of anyone wanting
> to pick up the issue again. The assumption of this code is that some
> form of syntax would exist to mark materialized CTEs and set the
> "ctematerialized" flag.
> 
> I haven't rebased this or tested it since last year; this patch is
> against b81eba6a65.

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 actual results of queries.  I've also attached both
the "leftover" diff and the files to which it should be applied.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

>>>>> "David" == David Fetter <david@fetter.org> 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 actual results of queries.

Both of those changes are the result of volatile CTEs being inlined more
than once (in one case, as part of an explicit test to ensure that CTEs
are being materialized and not multiply evaluated).

If you look for the XXX comment in the patch, it should be easy to add a
check that skips inlining if cterefcount > 1 and
contains_volatile_functions is true.

-- 
Andrew (irc:RhodiumToad)

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 actual results of queries.  I've also attached both
> the "leftover" diff and the files to which it should be applied.

I think the SQL programmer needs some control over whether a CTE is:

 - a materialized view -- and therefore a barrier
 - a view (which can then be inlined by the optimizer)

It is possible to add a keyword for this purpose in the WITH syntax:

    WITH   VIEW (...) AS a_view
         , MATERIALIZED VIEW (...) AS a_barrier
    ...;

This would be a lot like creating TEMP views, but without the catalog
overhead.

(I wonder how hard it would be to partiion the OID namespace into
temp/persistent ranges so that temp schema elements need not be written
into the catalog.)

Nico
-- 

>>>>> "Nico" == Nico Williams <nico@cryptonector.com> 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
materialization are along the lines of:

WITH ctename AS [[NOT] MATERIALIZED] (query)

(MATERIALIZED is already an un-reserved keyword)

-- 
Andrew (irc:RhodiumToad)

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 been suggested for explicitly controlling the
> materialization are along the lines of:
> 
> WITH ctename AS [[NOT] MATERIALIZED] (query)
> 
> (MATERIALIZED is already an un-reserved keyword)

Works for me.

On Tue, Jul 24, 2018 at 07:57:49PM -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> 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 for preventing inlining as you do for those cases,
> >> e.g. OFFSET 0.  And sticking in OFFSET 0 would be backwards-compatible
> >> too: your code would still work the same in older releases, unlike if
> >> we invent new syntax for this.
> 
> > I still think this is just doubling down on prior mistakes.
> 
> Not following what you think a better alternative is?  I'd be the
> first to agree that OFFSET 0 is a hack, but people are used to it.
> 
> Assuming that we go for inline-by-default for at least some cases,
> there's a separate discussion to be had about whether it's worth
> making a planner-control GUC to force the old behavior.  I'm not
> very excited about that, but I bet some people will be.

It is widely known that CTEs in PG are optimizer barriers.

That actually is useful, and I do make use of that fact (though I'm not
proud of it).

My proposal is that PG add an extension for specifying that a CTE is to
be materialized (barrier) or not (then inlined).

Nico
-- 

On Wed, Jul 25, 2018 at 04:18:42PM +0100, Andrew Gierth wrote:
> >>>>> "David" == David Fetter <david@fetter.org> 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 actual results of queries.
> 
> Both of those changes are the result of volatile CTEs being inlined more
> than once (in one case, as part of an explicit test to ensure that CTEs
> are being materialized and not multiply evaluated).
> 
> If you look for the XXX comment in the patch, it should be easy to add a
> check that skips inlining if cterefcount > 1 and
> contains_volatile_functions is true.

Thanks for the broad hints!

Please find attached the next version, which passes 'make check'.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

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 it.

Andreas

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 that
> part of the syntax unless there is a really strong reason for having
> it.

If we're going to give people specific knobs to turn as part of
queries to affect query plans[1], we should think it through at a much
higher level than this.

If we're not going to do that right away, we just need to treat
instances where the wrong call was made as planner bugs and address
them at that level.

Best,
David.

[1] Not to use the word that starts with 'h' and sounds like lints http://pghintplan.osdn.jp/
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

On Thu, Jul 26, 2018 at 7:14 AM, David Fetter <david@fetter.org> 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

On Fri, Jul 27, 2018 at 02:55:26PM +1200, Thomas Munro wrote:
> On Thu, Jul 26, 2018 at 7:14 AM, David Fetter <david@fetter.org> 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 patch that does.

It doesn't always pass make installcheck-world, but I need to sleep
rather than investigate that at the moment.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

>>>>> "Andreas" == Andreas Karlsson <andreas@proxel.se> 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 such a choice by the planner, or produce an error if it is
actually impossible to do so (e.g. using NOT MATERIALIZED on a wCTE)

-- 
Andrew (irc:RhodiumToad)

On Fri, Jul 27, 2018 at 8:10 PM, David Fetter <david@fetter.org> 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 <david@fetter.org> 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 patch that does.
>
> It doesn't always pass make installcheck-world, but I need to sleep
> rather than investigate that at the moment.

One observation I wanted to share: CTE scans inhibit parallelism today
(something we might eventually want to fix with shared tuplestores).
This patch therefore allows parallelism in some WITH queries, which
seems like a very valuable thing.   Example:

postgres=# create table foo as select generate_series(1, 1000000) i;
SELECT 1000000
postgres=# create table bar as select generate_series(1, 1000000) i;
SELECT 1000000
postgres=# create table baz as select generate_series(1, 1000000) i;
SELECT 1000000
postgres=# analyze;
ANALYZE

=== unpatched master ===

postgres=# explain analyze with cte as (select * from foo join bar
using (i)) select count(*) from cte join baz using (i);
                                                            QUERY PLAN

-----------------------------------------------------------------------------------------------------------------------------------
 Aggregate  (cost=149531.00..149531.01 rows=1 width=8) (actual
time=4400.951..4400.951 rows=1 loops=1)
   CTE cte
     ->  Hash Join  (cost=30832.00..70728.00 rows=1000000 width=4)
(actual time=551.243..1961.319 rows=1000000 loops=1)
           Hash Cond: (foo.i = bar.i)
           ->  Seq Scan on foo  (cost=0.00..14425.00 rows=1000000
width=4) (actual time=0.048..219.238 rows=1000000 loops=1)
           ->  Hash  (cost=14425.00..14425.00 rows=1000000 width=4)
(actual time=550.477..550.478 rows=1000000 loops=1)
                 Buckets: 131072  Batches: 16  Memory Usage: 3227kB
                 ->  Seq Scan on bar  (cost=0.00..14425.00
rows=1000000 width=4) (actual time=0.031..213.238 rows=1000000
loops=1)
   ->  Hash Join  (cost=30832.00..76303.00 rows=1000000 width=0)
(actual time=1090.162..4279.945 rows=1000000 loops=1)
         Hash Cond: (cte.i = baz.i)
         ->  CTE Scan on cte  (cost=0.00..20000.00 rows=1000000
width=4) (actual time=551.247..2564.529 rows=1000000 loops=1)
         ->  Hash  (cost=14425.00..14425.00 rows=1000000 width=4)
(actual time=538.833..538.833 rows=1000000 loops=1)
               Buckets: 131072  Batches: 16  Memory Usage: 3227kB
               ->  Seq Scan on baz  (cost=0.00..14425.00 rows=1000000
width=4) (actual time=0.039..208.658 rows=1000000 loops=1)
 Planning Time: 0.291 ms
 Execution Time: 4416.732 ms
(16 rows)

=== 0001-Inlining-CTEs-v0005.patch  ===

postgres=# explain analyze with cte as (select * from foo join bar
using (i)) select count(*) from cte join baz using (i);

QUERY PLAN

--------------------------------------------------------------------------------------------------------------------------------------------------------
 Finalize Aggregate  (cost=57854.78..57854.79 rows=1 width=8) (actual
time=1441.663..1441.664 rows=1 loops=1)
   ->  Gather  (cost=57854.57..57854.78 rows=2 width=8) (actual
time=1440.506..1474.974 rows=3 loops=1)
         Workers Planned: 2
         Workers Launched: 2
         ->  Partial Aggregate  (cost=56854.57..56854.58 rows=1
width=8) (actual time=1435.017..1435.018 rows=1 loops=3)
               ->  Parallel Hash Join  (cost=30856.01..55812.90
rows=416667 width=0) (actual time=1135.164..1393.437 rows=333333
loops=3)
                     Hash Cond: (foo.i = baz.i)
                     ->  Parallel Hash Join  (cost=15428.00..32202.28
rows=416667 width=8) (actual time=457.786..753.374 rows=333333
loops=3)
                           Hash Cond: (foo.i = bar.i)
                           ->  Parallel Seq Scan on foo
(cost=0.00..8591.67 rows=416667 width=4) (actual time=0.094..87.666
rows=333333 loops=3)
                           ->  Parallel Hash  (cost=8591.67..8591.67
rows=416667 width=4) (actual time=217.222..217.222 rows=333333
loops=3)
                                 Buckets: 131072  Batches: 16  Memory
Usage: 3520kB
                                 ->  Parallel Seq Scan on bar
(cost=0.00..8591.67 rows=416667 width=4) (actual time=0.061..84.631
rows=333333 loops=3)
                     ->  Parallel Hash  (cost=8591.67..8591.67
rows=416667 width=4) (actual time=227.240..227.241 rows=333333
loops=3)
                           Buckets: 131072  Batches: 16  Memory Usage: 3520kB
                           ->  Parallel Seq Scan on baz
(cost=0.00..8591.67 rows=416667 width=4) (actual time=0.060..84.270
rows=333333 loops=3)
 Planning Time: 0.407 ms
 Execution Time: 1475.113 ms
(18 rows)

-- 
Thomas Munro
http://www.enterprisedb.com

Hi,

On 2018-08-08 16:55:22 +1200, Thomas Munro wrote:
> On Fri, Jul 27, 2018 at 8:10 PM, David Fetter <david@fetter.org> 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 <david@fetter.org> 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 patch that does.
> >
> > It doesn't always pass make installcheck-world, but I need to sleep
> > rather than investigate that at the moment.
> 
> One observation I wanted to share: CTE scans inhibit parallelism today
> (something we might eventually want to fix with shared tuplestores).
> This patch therefore allows parallelism in some WITH queries, which
> seems like a very valuable thing.

Might be interesting to see how big a difference it makes for
TPC-DS. Currently the results are bad (as in many queries don't finish
in a relevant time) because it uses CTEs so widely, and there's often
predicates outside the CTE that could be pushed down.

- Andres

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 <david@fetter.org> 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 patch that does.
> 
> It doesn't always pass make installcheck-world, but I need to sleep
> rather than investigate that at the moment.

I took a quick look at this patch and I have a couple of comments.

1) Is it really safe, for backwards compatibility reasons, to inline 
when there are volatile functions? I imagine that it is possible that 
there are people who rely on that volatile functions inside a CTE are 
always run.

Imagine this case:

WITH cte AS (
   SELECT x, volatile_f(x) FROM tab ORDER BY x
)
SELECT * FROM cte LIMIT 10;

It could change behavior if volatile_f() has side effects and we inline 
the CTE. Is backwards compatibility for cases like this worth 
preserving? People can get the old behavior by adding OFFSET 0 or 
MATERIALIZED, but existing code would break.

2) The code in inline_cte_walker() is quite finicky. It looks correct to 
me but it is hard to follow and some simple bug could easily be hiding 
in there. I wonder if this code could be rewritten in some way to make 
it easier to follow.

3) Can you recall what the failing test was because I have so far not 
managed to reproduce it?

Andreas

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

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 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?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

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 ability to 
push down predicates is just one of the potential benefits from inlining.

Andreas

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 rewrite stage?
> 
> Sorry, but I do not think I understand your question. The ability to push
> down predicates is just one of the potential benefits from inlining.

Oracle appears to have such a distinction, and it appears we don't.
https://medium.com/@hakibenita/be-careful-with-cte-in-postgresql-fca5e24d2119

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

>>>>> "Andreas" == Andreas Karlsson <andreas@proxel.se> 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?

Yes.

 Andreas> Sorry, but I do not think I understand your question. The
 Andreas> ability to push down predicates is just one of the potential
 Andreas> benefits from inlining.

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;

-- inline subquery with optimization barrier (qual not pushed down)
select * from (select x from y offset 0) s where x=1;

-- CTE with materialization
with s as (select x from y) select * from s where x=1;

-- 
Andrew (irc:RhodiumToad)

On Fri, Oct 05, 2018 at 01:40:05AM +0100, Andrew Gierth wrote:
> >>>>> "Andreas" == Andreas Karlsson <andreas@proxel.se> 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?
> 
> Yes.
> 
>  Andreas> Sorry, but I do not think I understand your question. The
>  Andreas> ability to push down predicates is just one of the potential
>  Andreas> benefits from inlining.
> 
> 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;

...and doesn't need to materialize all of y,

> -- inline subquery with optimization barrier (qual not pushed down)
> select * from (select x from y offset 0) s where x=1;
> 
> -- CTE with materialization
> with s as (select x from y) select * from s where x=1;

while both of these do.  I was interested to discover that on my
synthetic test of 10 million integers from generate_series(1,10000000)
both with and without a b-tree index on x--as expected, the index has
no effect--I consistently get stuff like this:

shackle@[local]:5432/shackle(10.5)(18539) > explain (analyze, verbose, costs on, buffers on, timing on) with s as
(selectx from y) select * from s where x=1;
 
                                                          QUERY PLAN
     
 

══════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
 CTE Scan on s  (cost=144247.77..369247.25 rows=50000 width=4) (actual time=0.213..2287.355 rows=1 loops=1)
   Output: s.x
   Filter: (s.x = 1)
   Rows Removed by Filter: 9999999
   Buffers: shared hit=16310 read=27938, temp written=17089
   CTE s
     ->  Seq Scan on public.y  (cost=0.00..144247.77 rows=9999977 width=4) (actual time=0.208..593.426 rows=10000000
loops=1)
           Output: y.x
           Buffers: shared hit=16310 read=27938
 Planning time: 0.110 ms
 Execution time: 2313.682 ms
(11 rows)

shackle@[local]:5432/shackle(10.5)(18539) > explain (analyze, verbose, costs on, buffers on, timing on) select * from
(selectx from y offset 0) s where x=1;
 
                                                         QUERY PLAN
   
 

════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
 Subquery Scan on s  (cost=0.00..269247.48 rows=1 width=4) (actual time=0.734..1069.012 rows=1 loops=1)
   Output: s.x
   Filter: (s.x = 1)
   Rows Removed by Filter: 9999999
   Buffers: shared hit=16316 read=27932
   ->  Seq Scan on public.y  (cost=0.00..144247.77 rows=9999977 width=4) (actual time=0.731..539.463 rows=10000000
loops=1)
         Output: y.x
         Buffers: shared hit=16316 read=27932
 Planning time: 0.114 ms
 Execution time: 1069.032 ms
(10 rows)

i.e. for this case, the CTE scan takes over 2.3x the time the simple
materialization does. Also, when I boost work_mem to 1GB (256MB wasn't
enough to avoid "temp written"), there's still a 1.8x penalty.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

>>>>> "David" == David Fetter <david@fetter.org> 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,

 >> -- inline subquery with optimization barrier (qual not pushed down)
 >> select * from (select x from y offset 0) s where x=1;
 >> 
 >> -- CTE with materialization
 >> with s as (select x from y) select * from s where x=1;

 David> while both of these do.

The non-CTE one has to _evaluate_ the whole of the "s" subquery, but it
doesn't have to actually store the result, whereas the CTE version needs
to put it all in a tuplestore and read it back.

-- 
Andrew (irc:RhodiumToad)

Andreas Karlsson <andreas@proxel.se> 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 no way to express it in
a query that will also work in older servers.  (I will manfully
resist suggesting that then we don't need the nonstandard syntax
at all ... oops, too late.)

* This is not the idiomatic way to declare an expression tree walker:

+static bool inline_cte_walker(Node *node, void *ctxp)
+{
+    struct inline_cte_walker_ctx *ctx = ctxp;

* I have no faith in the idea that we can skip doing a copyObject on the
inlined subquery, except maybe in the case where we know there's exactly
one reference.

* In "here's where we do the actual substitution", if we're going to
scribble on the RTE rather than make a new one, we must take pains
to zero out the RTE_CTE-specific fields so that the RTE looks the
same as if it had been a RTE_SUBQUERY all along; cf db1071d4e.

* The lack of comments about what conditions we inline under
(at subselect.c:1318) is distressing.  I'm not particularly
in love with the way that inline_cte_walker is commented, either.
And dare I mention that this falsifies the intro comment for
SS_process_ctes?

* Speaking of the comments, I'm not convinced that view rewrite is
a good comparison point; I think this is more like subquery pullup.

* I wonder whether we should make range_table_walker more friendly
to the needs of this patch.  The fact that it doesn't work for this usage
suggests that it might not work for others, too.  I could see replacing
the QTW_EXAMINE_RTES flag with QTW_EXAMINE_RTES_BEFORE and
QTW_EXAMINE_RTES_AFTER so that callers could say which order of operations
they want (ie, visit RTE itself before or after its substructure).  Then
we could get rid of the double traversal of the RTE list.

* I think a large fraction of the regression test cases that this
changes are actually broken by the patch, in the sense that we meant
to test the behavior with a CTE and now we're not getting that.
So we'd need to add MATERIALIZED in many more places than this has
done.  Somebody else (Stephen?) would need to opine on whether that's
true for the CTEs in rowsecurity.sql, but it's definitely true for
the one in rowtypes.sql, where the point is to test what happens
with a whole-row Var.

* Which will mean we need some new test cases showing that this patch does
anything.  It'd be a good idea to add a test case showing that this gets
things right for conflicting CTE names at different levels, eg

explain verbose
with x as (select 1 as y)
select * from (with x as (select 2 as y) select * from x) ss;

* ruleutils.c needs adjustments for the new syntax, if we keep that.

* And of course the documentation needs much more work than this.

            regards, tom lane

On Fri, Nov 16, 2018 at 04:15:10PM -0500, Tom Lane wrote:
> Andreas Karlsson <andreas@proxel.se> 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 no way to express it in
> a query that will also work in older servers.  (I will manfully
> resist suggesting that then we don't need the nonstandard syntax
> at all ... oops, too late.)

If we're on board with exposing pilot error, we could decide not to
implement the nonstandard WITH syntax. One type of pilot error this
would expose is a situation where:

- A UDF has side effects, but is declared IMMUTABLE
- A WITH clause calls it in order to get those side effects on the
  entire result set

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 the behavior you want, you have no way to
 Tom> express it in a query that will also work in older servers. (I
 Tom> will manfully resist suggesting that then we don't need the
 Tom> nonstandard syntax at all ... oops, too late.)

I think this is the wrong approach, because you may want the
optimization-barrier effects of OFFSET/LIMIT _without_ the actual
materialization - there is no need to force a query like

with d as (select stuff from bigtable offset 1) select * from d;

to push all the data through an (on-disk) tuplestore.

 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 no level
changes because everywhere else just does another copyObject on it as
the first processing step (cf. set_subquery_pathlist and the various
functions called from pull_up_subqueries).

 Tom> * In "here's where we do the actual substitution", if we're going
 Tom> to scribble on the RTE rather than make a new one, we must take
 Tom> pains to zero out the RTE_CTE-specific fields so that the RTE
 Tom> looks the same as if it had been a RTE_SUBQUERY all along; cf
 Tom> db1071d4e.

Yes, this needs fixing. (This code predates that commit.)

 Tom> * Speaking of the comments, I'm not convinced that view rewrite is
 Tom> a good comparison point; I think this is more like subquery
 Tom> pullup.

It's not really like subquery pullup because we actually do go on to do
subquery pullup _after_ inlining CTEs.

 Tom> * I wonder whether we should make range_table_walker more friendly
 Tom> to the needs of this patch. The fact that it doesn't work for this
 Tom> usage suggests that it might not work for others, too. I could see
 Tom> replacing the QTW_EXAMINE_RTES flag with QTW_EXAMINE_RTES_BEFORE
 Tom> and QTW_EXAMINE_RTES_AFTER so that callers could say which order
 Tom> of operations they want (ie, visit RTE itself before or after its
 Tom> substructure). Then we could get rid of the double traversal of
 Tom> the RTE list.

Sure, why not.

-- 
Andrew.

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 no level
> changes because everywhere else just does another copyObject on it as
> the first processing step (cf. set_subquery_pathlist and the various
> functions called from pull_up_subqueries).

Perhaps it's safe today, but is that likely to remain true?  We've had
enough pain in the past from multiply-linked parse subtrees that I am
not eager to introduce another case, especially not here where there's
absolutely no evidence that it'd provide a meaningful performance
improvement.

>  Tom> * Speaking of the comments, I'm not convinced that view rewrite is
>  Tom> a good comparison point; I think this is more like subquery
>  Tom> pullup.

> It's not really like subquery pullup because we actually do go on to do
> subquery pullup _after_ inlining CTEs.

Subquery pullup can happen across multiple levels, too.

            regards, tom lane

Greetings,

* Andrew Gierth (andrew@tao11.riddles.org.uk) wrote:
> >>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 the behavior you want, you have no way to
>  Tom> express it in a query that will also work in older servers. (I
>  Tom> will manfully resist suggesting that then we don't need the
>  Tom> nonstandard syntax at all ... oops, too late.)
>
> I think this is the wrong approach, because you may want the
> optimization-barrier effects of OFFSET/LIMIT _without_ the actual
> materialization - there is no need to force a query like
>
> with d as (select stuff from bigtable offset 1) select * from d;
>
> to push all the data through an (on-disk) tuplestore.

Agreed, there's going to be cases where you want the CTE to be inlined
even with OFFSET/LIMIT.  Let's please not cater to the crowd who
happened to know that they could hack around with OFFSET/LIMIT to make
something not be inlined when it comes to the question of if the CTE
should be inlined or not.  That's the same issue we were argueing around
when discussing if we should allow parallel array_agg, imv.

Particularly since, with CTEs anyway, we never inlined them, so the
whole OFFSET/LIMIT thing doesn't really make any sense- today, if you
wrote a CTE, you wouldn't bother with OFFSET/LIMIT because you knew it
wasn't going to be inlined, that entire line of thinking is for
subqueries, not CTEs.  If you're going to force people to change their
CTEs to require that they not be inlined, let's not pick a method which
makes it ambiguous and makes us have to ask "do they really want this
limit/offset, or did they just want to make the CTE not be inlined...?"

Thanks!

Stephen

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 idiomatic way to declare an expression tree walker:
> 
> +static bool inline_cte_walker(Node *node, void *ctxp)
> +{
> +    struct inline_cte_walker_ctx *ctx = ctxp;
> 
> * I have no faith in the idea that we can skip doing a copyObject on the
> inlined subquery, except maybe in the case where we know there's exactly
> one reference.
> 
> * In "here's where we do the actual substitution", if we're going to
> scribble on the RTE rather than make a new one, we must take pains
> to zero out the RTE_CTE-specific fields so that the RTE looks the
> same as if it had been a RTE_SUBQUERY all along; cf db1071d4e.
> 
> * The lack of comments about what conditions we inline under
> (at subselect.c:1318) is distressing.  I'm not particularly
> in love with the way that inline_cte_walker is commented, either.
> And dare I mention that this falsifies the intro comment for
> SS_process_ctes?
> 
> * Speaking of the comments, I'm not convinced that view rewrite is
> a good comparison point; I think this is more like subquery pullup.

I believe I have fixed these except for the comment on the conditions 
for when we inline.

Andrew Gierth: Why did you chose to not inline on FOR UPDATE but inline 
volatile functions? I feel that this might be inconsistent since in both 
cases the query in the CTE can change behavior if the planner pushes a 
WHERE clause into the subquery, but maybe I am missing something.

> * I wonder whether we should make range_table_walker more friendly
> to the needs of this patch.  The fact that it doesn't work for this usage
> suggests that it might not work for others, too.  I could see replacing
> the QTW_EXAMINE_RTES flag with QTW_EXAMINE_RTES_BEFORE and
> QTW_EXAMINE_RTES_AFTER so that callers could say which order of operations
> they want (ie, visit RTE itself before or after its substructure).  Then
> we could get rid of the double traversal of the RTE list.

I did as suggested and the code is now much cleaner, but I feel while 
RTE walking business would become cleaner if we could change from having 
the range table walker yield both RTEs and the contents of the RTEs to 
having it just yeild the RTEs and then the walker callback can call 
expression_tree_walker() with the RTE so RTEs are treated like any other 
node in the tree.

I might look into how big impact such a change would have and if it is 
worth the churn.

> * I think a large fraction of the regression test cases that this
> changes are actually broken by the patch, in the sense that we meant
> to test the behavior with a CTE and now we're not getting that.
> So we'd need to add MATERIALIZED in many more places than this has
> done.  Somebody else (Stephen?) would need to opine on whether that's
> true for the CTEs in rowsecurity.sql, but it's definitely true for
> the one in rowtypes.sql, where the point is to test what happens
> with a whole-row Var.

Agreed, fixed.

> * Which will mean we need some new test cases showing that this patch does
> anything.  It'd be a good idea to add a test case showing that this gets
> things right for conflicting CTE names at different levels, eg
> 
> explain verbose
> with x as (select 1 as y)
> select * from (with x as (select 2 as y) select * from x) ss;

Added this test case, but more are needed. Any suggestion for what file 
these tests belong (right now I just added it to subselect.sql)? Or 
should I add a new called cte.sql?

> * ruleutils.c needs adjustments for the new syntax, if we keep that.

Thanks, fixed!

> * And of course the documentation needs much more work than this.

Yeah, I was waiting for there to be more agreement on when CTEs should 
be inlined, but maybe I should start writing anyway.

Andreas

>>>>> "Andreas" == Andreas Karlsson <andreas@proxel.se> 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
 Andreas> inconsistent since in both cases the query in the CTE can
 Andreas> change behavior if the planner pushes a WHERE clause into the
 Andreas> subquery, but maybe I am missing something.

I chose not to inline FOR UPDATE because it was an obvious compatibility
break, potentially changing the set of locked rows, and it was an easy
condition to test.

I did not test for volatile functions simply because this was a very
early stage of the project (which wasn't my project, I was just
assisting someone else). I left the comment "this likely needs some
additional checks" there for a reason.

-- 
Andrew (irc:RhodiumToad)

>>>>> "Andreas" == Andreas Karlsson <andreas@proxel.se> 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);
 Andreas> +
 Andreas> +       /* Preserve outer references, for example to other CTEs */
 Andreas> +       if (context->levelsup > 0)
 Andreas> +           IncrementVarSublevelsUp((Node *) newquery, context->levelsup, 1);

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
 * subqueries, but not into CTEs. We preserve this distinction
 * by not trying to push rowmarks into the new subquery.

This comment seems to me to be worth preserving (unless this behavior is
changed). What I'm referring to is the following, which is unchanged by
the patch:

create table t1 as select 123 as a;
create view v1 as select * from t1;
select * from t1 for update;  -- locks row in t1
select * from t1 for update of t1;  -- locks row in t1
select * from v1 for update;  -- locks row in t1
select * from v1 for update of v1;  -- locks row in t1
select * from (select * from t1) s1 for update;  -- locks row in t1
select * from (select * from t1) s1 for update of s1;  -- locks row in t1
with c1 as (select * from t1)
  select * from c1 for update;  -- does NOT lock anything at all
with c1 as (select * from t1)
  select * from c1 for update of c1;  -- parse-time error

(Obviously, inlining decisions should not change what gets locked;
the behavior here should not be changed unless it is changed for both
inlined and non-inlined CTEs.)

-- 
Andrew (irc:RhodiumToad)

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
>   * subqueries, but not into CTEs. We preserve this distinction
>   * by not trying to push rowmarks into the new subquery.
> 
> This comment seems to me to be worth preserving (unless this behavior is
> changed). What I'm referring to is the following, which is unchanged by
> the patch:
> 
> create table t1 as select 123 as a;
> create view v1 as select * from t1;
> select * from t1 for update;  -- locks row in t1
> select * from t1 for update of t1;  -- locks row in t1
> select * from v1 for update;  -- locks row in t1
> select * from v1 for update of v1;  -- locks row in t1
> select * from (select * from t1) s1 for update;  -- locks row in t1
> select * from (select * from t1) s1 for update of s1;  -- locks row in t1
> with c1 as (select * from t1)
>    select * from c1 for update;  -- does NOT lock anything at all
> with c1 as (select * from t1)
>    select * from c1 for update of c1;  -- parse-time error
> 
> (Obviously, inlining decisions should not change what gets locked;
> the behavior here should not be changed unless it is changed for both
> inlined and non-inlined CTEs.)

I see, I misread the comment. I will re-add it, possibly with some word 
smithing. Thanks!

Andreas

On 1/1/19 1:42 AM, Andrew Gierth wrote:
>>>>>> "Andreas" == Andreas Karlsson <andreas@proxel.se> 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
>   Andreas> inconsistent since in both cases the query in the CTE can
>   Andreas> change behavior if the planner pushes a WHERE clause into the
>   Andreas> subquery, but maybe I am missing something.
> 
> I chose not to inline FOR UPDATE because it was an obvious compatibility
> break, potentially changing the set of locked rows, and it was an easy
> condition to test.
> 
> I did not test for volatile functions simply because this was a very
> early stage of the project (which wasn't my project, I was just
> assisting someone else). I left the comment "this likely needs some
> additional checks" there for a reason.

Thanks, that makes sense! I will need to ponder some on if the behavior 
change when predicates are pushed into a subquery with volatile 
functions is ok. I am leaning towards no, because otherwise inlining 
CTEs would affect more than query performance.

Andreas

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 contains calls it a 
volatile function, or it contains FOR SHARE/FOR UPDATE. I feel this is a 
conservative approach which prevents behavioral changes (other than 
performance).

Current open issues:

1. Currently the patch will inline CTEs even when there are multiple 
references to them. If this is a win or not depends on the specific 
query so I am not sure what we should do here. One thing worth noting is 
that our current documentation talks a lot about how CTEs are only 
evaluated once.

"A useful property of <literal>WITH</literal> queries is that they are 
evaluated
only once per execution of the parent query, even if they are referred to
more than once by the parent query or sibling <literal>WITH</literal> 
queries.
Thus, expensive calculations that are needed in multiple places can be
placed within a <literal>WITH</literal> query to avoid redundant work. 
Another
possible application is to prevent unwanted multiple evaluations of
functions with side-effects."

What do you think?

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.

3. Are we happy with how I modified query_tree_walker()? I feel the code 
would be clearer if we could change the tree walker to treat the RTE as 
the parent node of the subquery instead of a sibling, but this seems 
like potentially a quite invasive change.

4. I need to update the user documentation.

Andreas

On Thu, Jan 10, 2019 at 2:28 PM Andreas Karlsson <andreas@proxel.se> 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.  I very much want to see this feature go
in.  As mentioned by Andres up-thread, TPC-DS makes a lot of use of
CTEs... let me see, 34 queries out of 99 have a WITH clause.  These
will hopefully become candidates for parallel query.

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 planner hints, but now we have a proposal to support one with a
top-level non-standard syntax.  If we take this syntax, should we not
also accept MATERIALIZED in front of subselects?

-1

-- 
Thomas Munro
http://www.enterprisedb.com

> 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 planner hints, but now we have a proposal to support one with a
> top-level non-standard syntax.  If we take this syntax, should we not
> also accept MATERIALIZED in front of subselects?
>
> -1

I think this is unduly negative.  Do you want to also remove the
IMMUTABLE, STABLE, and VOLATILE keywords from functions because those
are hints to the planner as to how those things should be treated?
Should we remove CREATE INDEX -- which is, after all, a non-standard
extension to SQL syntax -- because it presumes that the user is in a
better position than we are to know which columns ought to be indexed?

OK, I know that's a bit of a straw man -- you're talking about hints
within a query, not DDL.  Still, I think our theory about not having
hints is that we should have the optimizer try to figure it out
instead of making the user specify the behavior that they want -- and
I think sometimes that's setting the bar at an impossible level.  In
the case of things that have side effects, like FOR UPDATE/SHARE or
volatile functions, we really can't know the user's intention unless
the user tells us.  But even in cases where there are no side effects
and it's just a question of finding the most efficient plan, it
doesn't seem crazy to me to allow the user to give us a clue about
what they have in mind.

It's not like this is a thing where we can get this right 90% of the
time and with some more planner work we can drive it up to near 100%.
We're going to be wrong a lot, even if we do expensive things like try
planning it both ways and see which one comes out cheaper on cost, and
we don't like driving up planner CPU consumption, either.  So it seems
to me that letting the user say what they want is a very pragmatic
approach.  Sometimes, like with things that have side effects, it's
the only way to know that we're even delivering the right answer; and
even when it's just an optimization problem, it's nice to give users a
way of fixing our planning failures that is better than asking them to
wait until we invent a way of improving the optimization decision in
some future release - which we may never even do.

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 future release.  If they do specify
something, we do that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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 future release.  If they do specify
> something, we do that.

+many

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 understood that it was a long standing project policy that we don't
> > want planner hints, but now we have a proposal to support one with a
> > top-level non-standard syntax.  If we take this syntax, should we not
> > also accept MATERIALIZED in front of subselects?
> >
> > -1
> 
> I think this is unduly negative.  Do you want to also remove the
> IMMUTABLE, STABLE, and VOLATILE keywords from functions because those
> are hints to the planner as to how those things should be treated?

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.

> Should we remove CREATE INDEX -- which is, after all, a non-standard
> extension to SQL syntax -- because it presumes that the user is in a
> better position than we are to know which columns ought to be
> indexed?

Not yet.  Maybe in a decade or two.

> OK, I know that's a bit of a straw man -- you're talking about hints
> within a query, not DDL.  Still, I think our theory about not having
> hints is that we should have the optimizer try to figure it out
> instead of making the user specify the behavior that they want -- and
> I think sometimes that's setting the bar at an impossible level.

There is a worked example that's open source.
https://github.com/ossc-db/pg_hint_plan

Have we looked over it seriously for inclusion in PostgreSQL?

> It's not like this is a thing where we can get this right 90% of the
> time and with some more planner work we can drive it up to near 100%.
> We're going to be wrong a lot, even if we do expensive things like try
> planning it both ways and see which one comes out cheaper on cost, and
> we don't like driving up planner CPU consumption, either.  So it seems
> to me that letting the user say what they want is a very pragmatic
> approach.  Sometimes, like with things that have side effects, it's
> the only way to know that we're even delivering the right answer; and
> even when it's just an optimization problem, it's nice to give users a
> way of fixing our planning failures that is better than asking them to
> wait until we invent a way of improving the optimization decision in
> some future release - which we may never even do.
> 
> 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 future release.  If they do specify
> something, we do that.

When they're specifying it, are they specifying it globally, or
per WITH clause, or...?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

On Fri, Jan 11, 2019 at 1:49 PM David Fetter <david@fetter.org> 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 that functions are Turing-complete, but the
queries which call those functions somehow aren't?  Actually, there's
probably a decent argument that WITH RECURSIVE is Turing-complete even
without any fancy functions.  See
https://wiki.postgresql.org/wiki/Turing_Machine_(with_recursive)

> > OK, I know that's a bit of a straw man -- you're talking about hints
> > within a query, not DDL.  Still, I think our theory about not having
> > hints is that we should have the optimizer try to figure it out
> > instead of making the user specify the behavior that they want -- and
> > I think sometimes that's setting the bar at an impossible level.
>
> There is a worked example that's open source.
> https://github.com/ossc-db/pg_hint_plan
>
> Have we looked over it seriously for inclusion in PostgreSQL?

That really has very little to do with what's under discussion here,
unless you're proposing that the right strategy for determining
whether to materialize the CTE or not is to execute the query both
ways and then use that to construct the plan we use to execute the
query.

> When they're specifying it, are they specifying it globally, or
> per WITH clause, or...?

Per WITH clause.  That's the proposal which is under discussion here,
not anything else.  Did you read the thread?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Sat, Jan 12, 2019 at 7:19 AM Andres Freund <andres@anarazel.de> 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 neither, then we make some decision
> > which we may change in a future release.  If they do specify
> > something, we do that.
>
> +many

I think the syntax as proposed is almost OK if we only want to
grandfather in our historically hintful CTEs but discourage the
development of any future kinds of hints.  Even then I don't love the
way it formalises a semi-procedural step at the same language level as
a glorious declarative relational query.

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, subselects, views etc too (perhaps eventually join hints,
parallelism hints etc, but "materialize this" would be just another
one of those things).  That'd be all-in.

-- 
Thomas Munro
http://www.enterprisedb.com

Hi,

On 2019-01-12 07:58:39 +1300, Thomas Munro wrote:
> On Sat, Jan 12, 2019 at 7:19 AM Andres Freund <andres@anarazel.de> 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 neither, then we make some decision
> > > which we may change in a future release.  If they do specify
> > > something, we do that.
> >
> > +many
> 
> I think the syntax as proposed is almost OK if we only want to
> grandfather in our historically hintful CTEs but discourage the
> development of any future kinds of hints.  Even then I don't love the
> way it formalises a semi-procedural step at the same language level as
> a glorious declarative relational query.
> 
> 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, subselects, views etc too (perhaps eventually join hints,
> parallelism hints etc, but "materialize this" would be just another
> one of those things).  That'd be all-in.

I think you have some purity arguments here, but the likelihood of us
developing a full-blown solution is not that high, and the lack of
inlinable CTEs is *really* hurting us. As long as the design doesn't
block a full solution, if we go there, I think it's a very acceptable
blemish in comparison to the benefits we'd get.

Greetings,

Andres Freund

On Fri, Jan 11, 2019 at 2:04 PM Andres Freund <andres@anarazel.de> 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, subselects, views etc too (perhaps eventually join hints,
> > parallelism hints etc, but "materialize this" would be just another
> > one of those things).  That'd be all-in.
>
> I think you have some purity arguments here, but the likelihood of us
> developing a full-blown solution is not that high, and the lack of
> inlinable CTEs is *really* hurting us. As long as the design doesn't
> block a full solution, if we go there, I think it's a very acceptable
> blemish in comparison to the benefits we'd get.

Also, it seems to me that this is properly a property of the
individual WITH clause, not the query as a whole.

I mean I suppose we could do

WITH or_with_out_you OPTIONS (materialized false) AS (SELECT 'mariah
carey') SELECT ...

That'd allow for extensibility, have the write scope, and look like
what we do elsewhere.  It looks a little less elegant than

WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query...

...but maybe elegance for extensibility is a good trade.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jan 11, 2019 at 2:10 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Jan 11, 2019 at 2:04 PM Andres Freund <andres@anarazel.de> 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, subselects, views etc too (perhaps eventually join hints,
> > > parallelism hints etc, but "materialize this" would be just another
> > > one of those things).  That'd be all-in.
> >
> > I think you have some purity arguments here, but the likelihood of us
> > developing a full-blown solution is not that high, and the lack of
> > inlinable CTEs is *really* hurting us. As long as the design doesn't
> > block a full solution, if we go there, I think it's a very acceptable
> > blemish in comparison to the benefits we'd get.
>
> Also, it seems to me that this is properly a property of the
> individual WITH clause, not the query as a whole.
>
> I mean I suppose we could do
>
> WITH or_with_out_you OPTIONS (materialized false) AS (SELECT 'mariah
> carey') SELECT ...
>
> That'd allow for extensibility, have the write scope, and look like
> what we do elsewhere.  It looks a little less elegant than
>
> WITH cte_name [[NOT] MATERIALIZED] AS (query) main_query...
>
> ...but maybe elegance for extensibility is a good trade.
>

Here, have $0.02 from the peanut gallery...

I mildly prefer the latter, elegant spelling, but if CTE inlining does
become a thing then I would /really/ want some way, any way, of
telling Postgres that I want it to materialize a particular CTE.

I use that currently-documented property of CTEs to structure large,
complicated OLAP queries on a regular basis, for performance.
Sometimes, such as when you have dozens of tables in a complex join
tree, breaking the query into logically related chunks (which I know
about, but the planner does not) via CTE is the only way to give the
planner a fighting chance of finding a good plan.  Otherwise you get
stuck in the GEQO ghetto, or planning time is some non-trivial
multiple of execution time.

Thanks,

--
Mike Rylander
 | Executive Director
 | Equinox Open Library Initiative
 | phone:  1-877-OPEN-ILS (673-6457)
 | email:  miker@equinoxinitiative.org
 | web:  http://equinoxinitiative.org

On Tue, Jul 24, 2018 at 6:10 PM Andres Freund <andres@anarazel.de> 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 CTEs.

This is correct.  Suggesting that we need syntax to disabling inlining
at the CTE level, and/or GUC to control the behavior (which I agree
should be defualted to inline).  Something like
enable_cte_inline=true; I'm not very enthusiastic about explicitly
breaking intentionally introduced optimization fences and then forcing
people to inject our OFFSET 0 hack.   This is just too unpleasant to
contemplate...what  happens if we come up with a better implemntation
of OFFSET?  yuck.

Thanks for providing this, CTE plan problems are a real bugaboo.

merlin

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 imaging it working something like the below?

1. Default

# Not inlined

- Referenced multiple times
- Includes FOR UPDATE|SHARE
- Includes volatile functions
- Recurisve
- DML

# Inlined

- Simple case (no side effects, referenced once)

2. MATERIALIZED

# Not inlined

- Everything

# Inlined

- (none)

3. NOT MATERIALIZED

# Not inlined

- Recursive
- DML

# Inlined

- Everything else

Andreas

Andreas Karlsson <andreas@proxel.se> 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 referenced multiple times.

I'm also concerned about what we do if the user says NOT MATERIALIZED
but there are semantic or implementation reasons not to inline.  Either
we throw an error or do something the user didn't expect, and neither
is very nice.  So I'm not in favor of having that option.

            regards, tom lane

On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson <andreas@proxel.se> 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 not inline
> when a CTE is referenced multiple times.

When the planner materializes it, but the performance of the resulting
plan therefore sucks, I suppose.

I don't feel super-strongly about this, and Tom is right that there
may be cases where materialization is just not practical due to
implementation restrictions.  But it's not crazy to imagine that
inlining a multiply-referenced CTE might create opportunities for
optimization at each of those places, perhaps not the same ones in
each case, whereas materializing it results in doing extra work.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2019-01-18 15:34:46 -0500, Robert Haas wrote:
> On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson <andreas@proxel.se> 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 not inline
> > when a CTE is referenced multiple times.
> 
> When the planner materializes it, but the performance of the resulting
> plan therefore sucks, I suppose.
> 
> I don't feel super-strongly about this, and Tom is right that there
> may be cases where materialization is just not practical due to
> implementation restrictions.

*not* materializing I assume?

Greetings,

Andres Freund

On Fri, Jan 18, 2019 at 3:42 PM Andres Freund <andres@anarazel.de> wrote:
> On 2019-01-18 15:34:46 -0500, Robert Haas wrote:
> > On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson <andreas@proxel.se> 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 not inline
> > > when a CTE is referenced multiple times.
> >
> > When the planner materializes it, but the performance of the resulting
> > plan therefore sucks, I suppose.
> >
> > I don't feel super-strongly about this, and Tom is right that there
> > may be cases where materialization is just not practical due to
> > implementation restrictions.
>
> *not* materializing I assume?

Right, sorry.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 1/18/19 9:34 PM, Robert Haas wrote:
> On Thu, Jan 17, 2019 at 10:48 AM Andreas Karlsson <andreas@proxel.se> 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 not inline
>> when a CTE is referenced multiple times.
> 
> When the planner materializes it, but the performance of the resulting
> plan therefore sucks, I suppose.
> 
> I don't feel super-strongly about this, and Tom is right that there
> may be cases where materialization is just not practical due to
> implementation restrictions.  But it's not crazy to imagine that
> inlining a multiply-referenced CTE might create opportunities for
> optimization at each of those places, perhaps not the same ones in
> each case, whereas materializing it results in doing extra work.

I see.

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?

Andreas

Andreas Karlsson <andreas@proxel.se> 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, I'd independently thought that the latter is more readable,
and probably less likely to have syntax problems with future
extensions (since AS is already fully reserved).  Didn't get
around to mentioning it yet, but +1 for putting AS first.

            regards, tom lane

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 Andrew Gierth. I think that is the correct thing to do 
since it can have a major impact on performance.

Andreas

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 <andreas@proxel.se> 
>> 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 not 
>>> inline
>>> when a CTE is referenced multiple times.
>>
>> When the planner materializes it, but the performance of the resulting
>> plan therefore sucks, I suppose.
>>
>> I don't feel super-strongly about this, and Tom is right that there
>> may be cases where materialization is just not practical due to
>> implementation restrictions.  But it's not crazy to imagine that
>> inlining a multiply-referenced CTE might create opportunities for
>> optimization at each of those places, perhaps not the same ones in
>> each case, whereas materializing it results in doing extra work.
>
> I see.
>
> 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?
>
> Andreas
>
+1

For putting the 'AS' earlier, 2nd option,  I think it reads better.


Cheers,
Gavin

Andreas Karlsson <andreas@proxel.se> 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 touched the documentation, but I think this
is otherwise committable if we are satisfied with the semantics.

However ... after thinking about it more, I'm not really satisfied
with that.  In particular I don't like the fact that by default this
will inline regardless of the number of references to the CTE.  I doubt
that inlining when there are multiple references is so likely to be a
win as to justify it being the default, especially given that it flies
in the face of what our documentation has said for as long as we've
had CTEs.

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 MATERIALIZED: inline regardless of the number of references.

(Obviously, we should not inline if there's RECURSIVE or the CTE
potentially has side-effects, regardless of the user option;
I don't think those cases are up for debate.)

I haven't done anything about that here, but the changes would be pretty
minor.

            regards, tom lane

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index f177eba..6d456f6 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2925,6 +2925,7 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)

                 /* we store the string name because RTE_CTE RTEs need it */
                 APP_JUMB_STRING(cte->ctename);
+                APP_JUMB(cte->ctematerialized);
                 JumbleQuery(jstate, castNode(Query, cte->ctequery));
             }
             break;
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index bb92d9d..8c26dd1 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -1888,7 +1888,7 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t

 -- join in CTE
 EXPLAIN (VERBOSE, COSTS OFF)
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
                                                              QUERY PLAN
              

-------------------------------------------------------------------------------------------------------------------------------------
  Limit
@@ -1905,7 +1905,7 @@ WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2
                Output: t.c1_1, t.c2_1, t.c1_3
 (12 rows)

-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
  c1_1 | c2_1
 ------+------
   101 |  101
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index f438165..56602a1 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -493,8 +493,8 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
 SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR SHARE;
 -- join in CTE
 EXPLAIN (VERBOSE, COSTS OFF)
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
 -- ctid with whole-row reference
 EXPLAIN (VERBOSE, COSTS OFF)
 SELECT t1.ctid, t1, t2, t1.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10;
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index 88bc189..92f180c 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -2199,6 +2199,8 @@ SELECT n FROM t LIMIT 100;
   </para>

   <para>
+   TODO: Update this for inlining and <literal>MATERIALIZED</literal>.
+
    A useful property of <literal>WITH</literal> queries is that they are evaluated
    only once per execution of the parent query, even if they are referred to
    more than once by the parent query or sibling <literal>WITH</literal> queries.
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 4db8142..92ede4f 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -72,7 +72,7 @@ SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac

 <phrase>and <replaceable class="parameter">with_query</replaceable> is:</phrase>

-    <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> |
<replaceableclass="parameter">values</replaceable> | <replaceable class="parameter">insert</replaceable> | <replaceable
class="parameter">update</replaceable>| <replaceable class="parameter">delete</replaceable> ) 
+    <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS [ MATERIALIZED ] ( <replaceable
class="parameter">select</replaceable>| <replaceable class="parameter">values</replaceable> | <replaceable
class="parameter">insert</replaceable>| <replaceable class="parameter">update</replaceable> | <replaceable
class="parameter">delete</replaceable>) 

 TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
 </synopsis>
@@ -273,6 +273,8 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
    </para>

    <para>
+    TODO: Update this for inlining and <literal>MATERIALIZED</literal>.
+
     A key property of <literal>WITH</literal> queries is that they
     are evaluated only once per execution of the primary query,
     even if the primary query refers to them more than once.
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 3eb7e95..3b6e206 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2536,6 +2536,7 @@ _copyCommonTableExpr(const CommonTableExpr *from)

     COPY_STRING_FIELD(ctename);
     COPY_NODE_FIELD(aliascolnames);
+    COPY_SCALAR_FIELD(ctematerialized);
     COPY_NODE_FIELD(ctequery);
     COPY_LOCATION_FIELD(location);
     COPY_SCALAR_FIELD(cterecursive);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 5c4fa7d..e7b0fae 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2791,6 +2791,7 @@ _equalCommonTableExpr(const CommonTableExpr *a, const CommonTableExpr *b)
 {
     COMPARE_STRING_FIELD(ctename);
     COMPARE_NODE_FIELD(aliascolnames);
+    COMPARE_SCALAR_FIELD(ctematerialized);
     COMPARE_NODE_FIELD(ctequery);
     COMPARE_LOCATION_FIELD(location);
     COMPARE_SCALAR_FIELD(cterecursive);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 0fde876..b8467cd 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2980,6 +2980,7 @@ _outCommonTableExpr(StringInfo str, const CommonTableExpr *node)

     WRITE_STRING_FIELD(ctename);
     WRITE_NODE_FIELD(aliascolnames);
+    WRITE_BOOL_FIELD(ctematerialized);
     WRITE_NODE_FIELD(ctequery);
     WRITE_LOCATION_FIELD(location);
     WRITE_BOOL_FIELD(cterecursive);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index ec6f256..c653ee3 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -418,6 +418,7 @@ _readCommonTableExpr(void)

     READ_STRING_FIELD(ctename);
     READ_NODE_FIELD(aliascolnames);
+    READ_BOOL_FIELD(ctematerialized);
     READ_NODE_FIELD(ctequery);
     READ_LOCATION_FIELD(location);
     READ_BOOL_FIELD(cterecursive);
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 4465f00..67f276a 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -645,8 +645,8 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
     root->partColsUpdated = false;

     /*
-     * If there is a WITH list, process each WITH query and build an initplan
-     * SubPlan structure for it.
+     * If there is a WITH list, process each WITH query and either convert it
+     * to RTE_SUBQUERY RTE(s) or build an initplan SubPlan structure for it.
      */
     if (parse->cteList)
         SS_process_ctes(root);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 64272dd..8dd6a20 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -57,6 +57,14 @@ typedef struct finalize_primnode_context
     Bitmapset  *paramids;        /* Non-local PARAM_EXEC paramids found */
 } finalize_primnode_context;

+typedef struct inline_cte_walker_context
+{
+    const char *ctename;        /* name and relative level of target CTE */
+    int            levelsup;
+    int            refcount;        /* number of remaining references */
+    Query       *ctequery;        /* query to substitute */
+} inline_cte_walker_context;
+

 static Node *build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot,
               List *plan_params,
@@ -75,6 +83,10 @@ static Node *convert_testexpr_mutator(Node *node,
 static bool subplan_is_hashable(Plan *plan);
 static bool testexpr_is_hashable(Node *testexpr);
 static bool hash_ok_operator(OpExpr *expr);
+static bool contain_dml(Node *node);
+static bool contain_dml_walker(Node *node, void *context);
+static void inline_cte(PlannerInfo *root, CommonTableExpr *cte);
+static bool inline_cte_walker(Node *node, inline_cte_walker_context *context);
 static bool simplify_EXISTS_query(PlannerInfo *root, Query *query);
 static Query *convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect,
                       Node **testexpr, List **paramIds);
@@ -804,10 +816,13 @@ hash_ok_operator(OpExpr *expr)
 /*
  * SS_process_ctes: process a query's WITH list
  *
- * We plan each interesting WITH item and convert it to an initplan.
+ * Consider each CTE in the WITH list and either ignore it (if it's an
+ * unreferenced SELECT), "inline" it to create a regular sub-SELECT-in-FROM,
+ * or convert it to an initplan.
+ *
  * A side effect is to fill in root->cte_plan_ids with a list that
  * parallels root->parse->cteList and provides the subplan ID for
- * each CTE's initplan.
+ * each CTE's initplan, or a dummy ID (-1) if we didn't make an initplan.
  */
 void
 SS_process_ctes(PlannerInfo *root)
@@ -839,6 +854,39 @@ SS_process_ctes(PlannerInfo *root)
         }

         /*
+         * Consider inlining the CTE (creating RTE_SUBQUERY RTE(s)) instead of
+         * implementing it as a separately-planned CTE.
+         *
+         * We cannot inline if any of these conditions hold:
+         *
+         * 1. The user said not to (the MATERIALIZED option).
+         *
+         * 2. The CTE is recursive.
+         *
+         * 3. The CTE has side-effects; this includes either not being a plain
+         * SELECT, or containing volatile functions.  Inlining might change
+         * the side-effects, which is not good.
+         *
+         * Otherwise, we always inline.  This could be a net loss if the CTE
+         * is multiply referenced, but not necessarily so: the ability to
+         * absorb restrictions from the outer query level could outweigh the
+         * need for duplicate computations.  We do not have nearly enough
+         * information at this point to tell if that's true, so we make the
+         * user say MATERIALIZED if it's not.
+         */
+        if (!cte->ctematerialized &&
+            !cte->cterecursive &&
+            cmdType == CMD_SELECT &&
+            !contain_dml(cte->ctequery) &&
+            !contain_volatile_functions(cte->ctequery))
+        {
+            inline_cte(root, cte);
+            /* Make a dummy entry in cte_plan_ids */
+            root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
+            continue;
+        }
+
+        /*
          * Copy the source Query node.  Probably not necessary, but let's keep
          * this similar to make_subplan.
          */
@@ -935,6 +983,127 @@ SS_process_ctes(PlannerInfo *root)
 }

 /*
+ * contain_dml: is any subquery not a plain SELECT?
+ *
+ * We reject SELECT FOR UPDATE/SHARE as well as INSERT etc.
+ */
+static bool
+contain_dml(Node *node)
+{
+    return contain_dml_walker(node, NULL);
+}
+
+static bool
+contain_dml_walker(Node *node, void *context)
+{
+    if (node == NULL)
+        return false;
+    if (IsA(node, Query))
+    {
+        Query       *query = (Query *) node;
+
+        if (query->commandType != CMD_SELECT ||
+            query->rowMarks != NIL)
+            return true;
+
+        return query_tree_walker(query, contain_dml_walker, context, 0);
+    }
+    return expression_tree_walker(node, contain_dml_walker, context);
+}
+
+/*
+ * inline_cte: convert RTE_CTE references to given CTE into RTE_SUBQUERYs
+ */
+static void
+inline_cte(PlannerInfo *root, CommonTableExpr *cte)
+{
+    struct inline_cte_walker_context context;
+
+    context.ctename = cte->ctename;
+    /* Start at levelsup = -1 because we'll immediately increment it */
+    context.levelsup = -1;
+    context.refcount = cte->cterefcount;
+    context.ctequery = castNode(Query, cte->ctequery);
+
+    (void) inline_cte_walker((Node *) root->parse, &context);
+
+    /* Assert we replaced all references */
+    Assert(context.refcount == 0);
+}
+
+static bool
+inline_cte_walker(Node *node, inline_cte_walker_context *context)
+{
+    if (node == NULL)
+        return false;
+    if (IsA(node, Query))
+    {
+        Query       *query = (Query *) node;
+
+        context->levelsup++;
+
+        /*
+         * Visit the query's RTE nodes after their contents; otherwise
+         * query_tree_walker would descend into the newly inlined CTE query,
+         * which we don't want.
+         */
+        (void) query_tree_walker(query, inline_cte_walker, context,
+                                 QTW_EXAMINE_RTES_AFTER);
+
+        context->levelsup--;
+
+        return false;
+    }
+    else if (IsA(node, RangeTblEntry))
+    {
+        RangeTblEntry *rte = (RangeTblEntry *) node;
+
+        if (rte->rtekind == RTE_CTE &&
+            strcmp(rte->ctename, context->ctename) == 0 &&
+            rte->ctelevelsup == context->levelsup)
+        {
+            /*
+             * Found a reference to replace.  Generate a copy of the CTE query
+             * with appropriate level adjustment for outer references (e.g.,
+             * to other CTEs).
+             */
+            Query       *newquery = copyObject(context->ctequery);
+
+            if (context->levelsup > 0)
+                IncrementVarSublevelsUp((Node *) newquery, context->levelsup, 1);
+
+            /*
+             * Convert the RTE_CTE RTE into a RTE_SUBQUERY.
+             *
+             * Historically, a FOR UPDATE clause has been treated as extending
+             * into views and subqueries, but not into CTEs.  We preserve this
+             * distinction by not trying to push rowmarks into the new
+             * subquery.
+             */
+            rte->rtekind = RTE_SUBQUERY;
+            rte->subquery = newquery;
+            rte->security_barrier = false;
+
+            /* Zero out CTE-specific fields */
+            rte->ctename = NULL;
+            rte->ctelevelsup = 0;
+            rte->self_reference = false;
+            rte->coltypes = NIL;
+            rte->coltypmods = NIL;
+            rte->colcollations = NIL;
+
+            /* Count the number of replacements we've done */
+            context->refcount--;
+        }
+
+        return false;
+    }
+
+    return expression_tree_walker(node, inline_cte_walker, context);
+}
+
+
+/*
  * convert_ANY_sublink_to_join: try to convert an ANY SubLink to a join
  *
  * The caller has found an ANY SubLink at the top level of one of the query's
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c1faf41..5ffdd8f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -518,7 +518,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
                 Bit ConstBit BitWithLength BitWithoutLength
 %type <str>        character
 %type <str>        extract_arg
-%type <boolean> opt_varying opt_timezone opt_no_inherit
+%type <boolean> opt_varying opt_timezone opt_no_inherit opt_materialized

 %type <ival>    Iconst SignedIconst
 %type <str>        Sconst comment_text notify_payload
@@ -11322,17 +11322,23 @@ cte_list:
         | cte_list ',' common_table_expr        { $$ = lappend($1, $3); }
         ;

-common_table_expr:  name opt_name_list AS '(' PreparableStmt ')'
+common_table_expr:  name opt_name_list AS opt_materialized '(' PreparableStmt ')'
             {
                 CommonTableExpr *n = makeNode(CommonTableExpr);
                 n->ctename = $1;
                 n->aliascolnames = $2;
-                n->ctequery = $5;
+                n->ctematerialized = $4;
+                n->ctequery = $6;
                 n->location = @1;
                 $$ = (Node *) n;
             }
         ;

+opt_materialized:
+        MATERIALIZED                            { $$ = true; }
+        | /*EMPTY*/                                { $$ = false; }
+        ;
+
 opt_with_clause:
         with_clause                                { $$ = $1; }
         | /*EMPTY*/                                { $$ = NULL; }
@@ -16214,6 +16220,7 @@ makeRecursiveViewSelect(char *relname, List *aliases, Node *query)
     /* create common table expression */
     cte->ctename = relname;
     cte->aliascolnames = aliases;
+    cte->ctematerialized = false;
     cte->ctequery = query;
     cte->location = -1;

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 302df16..eb0010d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -5166,7 +5166,10 @@ get_with_clause(Query *query, deparse_context *context)
             }
             appendStringInfoChar(buf, ')');
         }
-        appendStringInfoString(buf, " AS (");
+        appendStringInfoString(buf, " AS ");
+        if (cte->ctematerialized)
+            appendStringInfoString(buf, "MATERIALIZED ");
+        appendStringInfoChar(buf, '(');
         if (PRETTY_INDENT(context))
             appendContextKeyword(context, "", 0, 0, 0);
         get_query_def((Query *) cte->ctequery, buf, context->namespaces, NULL,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index addc2c2..136f72c 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1404,6 +1404,7 @@ typedef struct CommonTableExpr
     NodeTag        type;
     char       *ctename;        /* query name (never qualified) */
     List       *aliascolnames;    /* optional list of column names */
+    bool        ctematerialized;    /* is this an optimization fence? */
     /* SelectStmt/InsertStmt/etc before parse analysis, Query afterwards: */
     Node       *ctequery;        /* the CTE's subquery */
     int            location;        /* token location, or -1 if unknown */
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 1d12b01..2e17049 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -2179,7 +2179,7 @@ EXPLAIN (COSTS OFF) EXECUTE plancache_test;
    Filter: (((a % 2) = 0) AND f_leak(b))
 (2 rows)

-PREPARE plancache_test2 AS WITH q AS (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
+PREPARE plancache_test2 AS WITH q AS MATERIALIZED (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
 EXPLAIN (COSTS OFF) EXECUTE plancache_test2;
                    QUERY PLAN
 -------------------------------------------------
@@ -2192,7 +2192,7 @@ EXPLAIN (COSTS OFF) EXECUTE plancache_test2;
          ->  Seq Scan on z2
 (7 rows)

-PREPARE plancache_test3 AS WITH q AS (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
+PREPARE plancache_test3 AS WITH q AS MATERIALIZED (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test3;
                      QUERY PLAN
 -----------------------------------------------------
@@ -2826,7 +2826,7 @@ ALTER TABLE t1 ENABLE ROW LEVEL SECURITY;
 GRANT ALL ON t1 TO regress_rls_bob;
 INSERT INTO t1 (SELECT x, md5(x::text) FROM generate_series(0,20) x);
 SET SESSION AUTHORIZATION regress_rls_bob;
-WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
 NOTICE:  f_leak => cfcd208495d565ef66e7dff9f98764da
 NOTICE:  f_leak => c81e728d9d4c2f636f067f89cc14862c
 NOTICE:  f_leak => a87ff679a2f3e71d9181a67b7542122c
@@ -2853,7 +2853,8 @@ NOTICE:  f_leak => 98f13708210194c475687be6106a3b84
  20 | 98f13708210194c475687be6106a3b84
 (11 rows)

-EXPLAIN (COSTS OFF) WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+EXPLAIN (COSTS OFF)
+WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
                    QUERY PLAN
 -------------------------------------------------
  CTE Scan on cte1
diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out
index d6a1a33..14757b4 100644
--- a/src/test/regress/expected/rowtypes.out
+++ b/src/test/regress/expected/rowtypes.out
@@ -1037,7 +1037,7 @@ from (values (1,row(1,2)), (1,row(null,null)), (1,null),
 (6 rows)

 explain (verbose, costs off)
-with r(a,b) as
+with r(a,b) as materialized
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
@@ -1050,7 +1050,7 @@ select r, r is null as isnull, r is not null as isnotnull from r;
            Output: "*VALUES*".column1, "*VALUES*".column2
 (5 rows)

-with r(a,b) as
+with r(a,b) as materialized
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index e384cd2..bb10e27 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3108,7 +3108,7 @@ explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
 (5 rows)

 -- ensure upserting into a rule, with a CTE (different offsets!) works
-WITH data(hat_name, hat_color) AS (
+WITH data(hat_name, hat_color) AS MATERIALIZED (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
@@ -3122,7 +3122,8 @@ RETURNING *;
  h9         | blue
 (2 rows)

-EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
+EXPLAIN (costs off)
+WITH data(hat_name, hat_color) AS MATERIALIZED (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index 588d069..e2afd6f 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -1154,3 +1154,117 @@ fetch backward all in c1;
 (2 rows)

 commit;
+--
+-- Tests for CTE inlining behavior
+--
+-- Basic subquery that can be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+            QUERY PLAN
+----------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1
+   Filter: (subselect_tbl.f1 = 1)
+(3 rows)
+
+-- Explicitly request materialization
+explain (verbose, costs off)
+with x as materialized (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+                QUERY PLAN
+------------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Seq Scan on public.subselect_tbl
+           Output: subselect_tbl.f1
+(6 rows)
+
+-- Stable functions are safe to inline
+explain (verbose, costs off)
+with x as (select * from (select f1, now() from subselect_tbl) ss)
+select * from x where f1 = 1;
+            QUERY PLAN
+-----------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1, now()
+   Filter: (subselect_tbl.f1 = 1)
+(3 rows)
+
+-- Volatile functions prevent inlining
+explain (verbose, costs off)
+with x as (select * from (select f1, random() from subselect_tbl) ss)
+select * from x where f1 = 1;
+                  QUERY PLAN
+----------------------------------------------
+ CTE Scan on x
+   Output: x.f1, x.random
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Seq Scan on public.subselect_tbl
+           Output: subselect_tbl.f1, random()
+(6 rows)
+
+-- SELECT FOR UPDATE cannot be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl for update) ss)
+select * from x where f1 = 1;
+                             QUERY PLAN
+--------------------------------------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Subquery Scan on ss
+           Output: ss.f1
+           ->  LockRows
+                 Output: subselect_tbl.f1, subselect_tbl.ctid
+                 ->  Seq Scan on public.subselect_tbl
+                       Output: subselect_tbl.f1, subselect_tbl.ctid
+(10 rows)
+
+-- Check handling of outer references
+explain (verbose, costs off)
+with x as (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+         QUERY PLAN
+-----------------------------
+ Seq Scan on public.int4_tbl
+   Output: int4_tbl.f1
+(2 rows)
+
+explain (verbose, costs off)
+with x as materialized (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+             QUERY PLAN
+-------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   CTE x
+     ->  Seq Scan on public.int4_tbl
+           Output: int4_tbl.f1
+(5 rows)
+
+-- Ensure that we inline the currect CTE when there are
+-- multiple CTEs with the same name
+explain (verbose, costs off)
+with x as (select 1 as y)
+select * from (with x as (select 2 as y) select * from x) ss;
+ QUERY PLAN
+-------------
+ Result
+   Output: 2
+(2 rows)
+
+-- Row marks are not pushed into CTEs
+explain (verbose, costs off)
+with x as (select * from subselect_tbl)
+select * from x for update;
+                           QUERY PLAN
+----------------------------------------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1, subselect_tbl.f2, subselect_tbl.f3
+(2 rows)
+
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 38e9b38..52da276 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -840,10 +840,10 @@ EXPLAIN (COSTS OFF) SELECT * FROM z1 WHERE f_leak(b);
 PREPARE plancache_test AS SELECT * FROM z1 WHERE f_leak(b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test;

-PREPARE plancache_test2 AS WITH q AS (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
+PREPARE plancache_test2 AS WITH q AS MATERIALIZED (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
 EXPLAIN (COSTS OFF) EXECUTE plancache_test2;

-PREPARE plancache_test3 AS WITH q AS (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
+PREPARE plancache_test3 AS WITH q AS MATERIALIZED (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test3;

 SET ROLE regress_rls_group1;
@@ -1071,8 +1071,9 @@ INSERT INTO t1 (SELECT x, md5(x::text) FROM generate_series(0,20) x);

 SET SESSION AUTHORIZATION regress_rls_bob;

-WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
-EXPLAIN (COSTS OFF) WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+EXPLAIN (COSTS OFF)
+WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;

 WITH cte1 AS (UPDATE t1 SET a = a + 1 RETURNING *) SELECT * FROM cte1; --fail
 WITH cte1 AS (UPDATE t1 SET a = a RETURNING *) SELECT * FROM cte1; --ok
diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql
index e6d3898..b3b0774 100644
--- a/src/test/regress/sql/rowtypes.sql
+++ b/src/test/regress/sql/rowtypes.sql
@@ -428,12 +428,12 @@ from (values (1,row(1,2)), (1,row(null,null)), (1,null),
              (null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);

 explain (verbose, costs off)
-with r(a,b) as
+with r(a,b) as materialized
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;

-with r(a,b) as
+with r(a,b) as materialized
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index f4ee30e..6666951 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1132,7 +1132,7 @@ SELECT tablename, rulename, definition FROM pg_rules
 explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;

 -- ensure upserting into a rule, with a CTE (different offsets!) works
-WITH data(hat_name, hat_color) AS (
+WITH data(hat_name, hat_color) AS MATERIALIZED (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
@@ -1140,7 +1140,8 @@ WITH data(hat_name, hat_color) AS (
 INSERT INTO hats
     SELECT * FROM data
 RETURNING *;
-EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
+EXPLAIN (costs off)
+WITH data(hat_name, hat_color) AS MATERIALIZED (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 843f511..3dba35e 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -625,3 +625,52 @@ move forward all in c1;
 fetch backward all in c1;

 commit;
+
+--
+-- Tests for CTE inlining behavior
+--
+
+-- Basic subquery that can be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Explicitly request materialization
+explain (verbose, costs off)
+with x as materialized (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Stable functions are safe to inline
+explain (verbose, costs off)
+with x as (select * from (select f1, now() from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Volatile functions prevent inlining
+explain (verbose, costs off)
+with x as (select * from (select f1, random() from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- SELECT FOR UPDATE cannot be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl for update) ss)
+select * from x where f1 = 1;
+
+-- Check handling of outer references
+explain (verbose, costs off)
+with x as (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+
+explain (verbose, costs off)
+with x as materialized (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+
+-- Ensure that we inline the currect CTE when there are
+-- multiple CTEs with the same name
+explain (verbose, costs off)
+with x as (select 1 as y)
+select * from (with x as (select 2 as y) select * from x) ss;
+
+-- Row marks are not pushed into CTEs
+explain (verbose, costs off)
+with x as (select * from subselect_tbl)
+select * from x for update;

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 MATERIALIZED: inline regardless of the number of references.
> (Obviously, we should not inline if there's RECURSIVE or the CTE
> potentially has side-effects, regardless of the user option;
> I don't think those cases are up for debate.)

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.

I was interested to find, while writing the docs, that it's a real
struggle to invent plausible reasons to write MATERIALIZED given the
above specification.  You pretty much have to have lied to the planner,
eg by making a volatile function that's not marked volatile, before
there's a real need for that.  Am I missing something?  If I'm not,
then we're in a really good place backwards-compatibility-wise,
because the new default behavior shouldn't break any cases where people
weren't cheating.

            regards, tom lane

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index f177eba..6d456f6 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2925,6 +2925,7 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)

                 /* we store the string name because RTE_CTE RTEs need it */
                 APP_JUMB_STRING(cte->ctename);
+                APP_JUMB(cte->ctematerialized);
                 JumbleQuery(jstate, castNode(Query, cte->ctequery));
             }
             break;
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index bb92d9d..8c26dd1 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -1888,7 +1888,7 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t

 -- join in CTE
 EXPLAIN (VERBOSE, COSTS OFF)
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
                                                              QUERY PLAN
              

-------------------------------------------------------------------------------------------------------------------------------------
  Limit
@@ -1905,7 +1905,7 @@ WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2
                Output: t.c1_1, t.c2_1, t.c1_3
 (12 rows)

-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
  c1_1 | c2_1
 ------+------
   101 |  101
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index f438165..56602a1 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -493,8 +493,8 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
 SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR SHARE;
 -- join in CTE
 EXPLAIN (VERBOSE, COSTS OFF)
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
 -- ctid with whole-row reference
 EXPLAIN (VERBOSE, COSTS OFF)
 SELECT t1.ctid, t1, t2, t1.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10;
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index 88bc189..2225255 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -2199,16 +2199,19 @@ SELECT n FROM t LIMIT 100;
   </para>

   <para>
-   A useful property of <literal>WITH</literal> queries is that they are evaluated
-   only once per execution of the parent query, even if they are referred to
-   more than once by the parent query or sibling <literal>WITH</literal> queries.
+   A useful property of <literal>WITH</literal> queries is that they are
+   normally evaluated only once per execution of the parent query, even if
+   they are referred to more than once by the parent query or
+   sibling <literal>WITH</literal> queries.
    Thus, expensive calculations that are needed in multiple places can be
    placed within a <literal>WITH</literal> query to avoid redundant work.  Another
    possible application is to prevent unwanted multiple evaluations of
    functions with side-effects.
-   However, the other side of this coin is that the optimizer is less able to
-   push restrictions from the parent query down into a <literal>WITH</literal> query
-   than an ordinary subquery.  The <literal>WITH</literal> query will generally be
+   However, the other side of this coin is that the optimizer is not able to
+   push restrictions from the parent query down into a multiply-referenced
+   <literal>WITH</literal> query, since that might affect all uses of the
+   <literal>WITH</literal> query's output when it should affect only one.
+   The multiply-referenced <literal>WITH</literal> query will be
    evaluated as written, without suppression of rows that the parent query
    might discard afterwards.  (But, as mentioned above, evaluation might stop
    early if the reference(s) to the query demand only a limited number of
@@ -2216,6 +2219,75 @@ SELECT n FROM t LIMIT 100;
   </para>

   <para>
+   However, if a <literal>WITH</literal> query is non-recursive and
+   side-effect-free (that is, it is a <literal>SELECT</literal> containing
+   no volatile functions) then it can be folded into the parent query,
+   allowing joint optimization of the two query levels.  By default, this
+   happens if the parent query references the <literal>WITH</literal> query
+   just once, but not if it references the <literal>WITH</literal> query
+   more than once.  You can override that decision by
+   specifying <literal>MATERIALIZED</literal> to force separate calculation
+   of the <literal>WITH</literal> query, or by specifying <literal>NOT
+   MATERIALIZED</literal> to force it to be merged into the parent query.
+   The latter choice risks duplicate computation of
+   the <literal>WITH</literal> query, but it can still give a net savings if
+   each usage of the <literal>WITH</literal> query needs only a small part
+   of the <literal>WITH</literal> query's full output.
+  </para>
+
+  <para>
+   A simple example of these rules is
+<programlisting>
+WITH w AS (
+    SELECT * FROM big_table
+)
+SELECT * FROM w WHERE key = 123;
+</programlisting>
+   This <literal>WITH</literal> query will be folded, producing the same
+   execution plan as
+<programlisting>
+SELECT * FROM big_table WHERE key = 123;
+</programlisting>
+   In particular, if there's an index on <structfield>key</structfield>,
+   it will probably be used to fetch just the rows having <literal>key =
+   123</literal>.  On the other hand, in
+<programlisting>
+WITH w AS (
+    SELECT * FROM big_table
+)
+SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
+WHERE w2.key = 123;
+</programlisting>
+   the <literal>WITH</literal> query will be materialized, producing a
+   temporary copy of <structname>big_table</structname> that is then
+   joined with itself — without benefit of any index.  This query
+   will be executed much more efficiently if written as
+<programlisting>
+WITH w AS NOT MATERIALIZED (
+    SELECT * FROM big_table
+)
+SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
+WHERE w2.key = 123;
+</programlisting>
+   so that the parent query's restrictions can be applied directly
+   to scans of <structname>big_table</structname>.
+  </para>
+
+  <para>
+   An example where <literal>NOT MATERIALIZED</literal> could be
+   undesirable is
+<programlisting>
+WITH w AS (
+    SELECT key, very_expensive_function(val) as f FROM some_table
+)
+SELECT * FROM w AS w1 JOIN w AS w2 ON w1.f = w2.f;
+</programlisting>
+   Here, materialization of the <literal>WITH</literal> query ensures
+   that <function>very_expensive_function</function> is evaluated only
+   once per table row, not twice.
+  </para>
+
+  <para>
    The examples above only show <literal>WITH</literal> being used with
    <command>SELECT</command>, but it can be attached in the same way to
    <command>INSERT</command>, <command>UPDATE</command>, or <command>DELETE</command>.
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 4db8142..7035c95 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -72,7 +72,7 @@ SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac

 <phrase>and <replaceable class="parameter">with_query</replaceable> is:</phrase>

-    <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> |
<replaceableclass="parameter">values</replaceable> | <replaceable class="parameter">insert</replaceable> | <replaceable
class="parameter">update</replaceable>| <replaceable class="parameter">delete</replaceable> ) 
+    <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS [ [ NOT ] MATERIALIZED ] ( <replaceable
class="parameter">select</replaceable>| <replaceable class="parameter">values</replaceable> | <replaceable
class="parameter">insert</replaceable>| <replaceable class="parameter">update</replaceable> | <replaceable
class="parameter">delete</replaceable>) 

 TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
 </synopsis>
@@ -93,7 +93,8 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
       These effectively serve as temporary tables that can be referenced
       in the <literal>FROM</literal> list.  A <literal>WITH</literal> query
       that is referenced more than once in <literal>FROM</literal> is
-      computed only once.
+      computed only once,
+      unless specified otherwise with <literal>NOT MATERIALIZED</literal>.
       (See <xref linkend="sql-with" endterm="sql-with-title"/> below.)
      </para>
     </listitem>
@@ -273,8 +274,17 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
    </para>

    <para>
+    The primary query and the <literal>WITH</literal> queries are all
+    (notionally) executed at the same time.  This implies that the effects of
+    a data-modifying statement in <literal>WITH</literal> cannot be seen from
+    other parts of the query, other than by reading its <literal>RETURNING</literal>
+    output.  If two such data-modifying statements attempt to modify the same
+    row, the results are unspecified.
+   </para>
+
+   <para>
     A key property of <literal>WITH</literal> queries is that they
-    are evaluated only once per execution of the primary query,
+    are normally evaluated only once per execution of the primary query,
     even if the primary query refers to them more than once.
     In particular, data-modifying statements are guaranteed to be
     executed once and only once, regardless of whether the primary query
@@ -282,12 +292,35 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
    </para>

    <para>
-    The primary query and the <literal>WITH</literal> queries are all
-    (notionally) executed at the same time.  This implies that the effects of
-    a data-modifying statement in <literal>WITH</literal> cannot be seen from
-    other parts of the query, other than by reading its <literal>RETURNING</literal>
-    output.  If two such data-modifying statements attempt to modify the same
-    row, the results are unspecified.
+    However, a <literal>WITH</literal> query can be marked
+    <literal>NOT MATERIALIZED</literal> to remove this guarantee.  In that
+    case, the <literal>WITH</literal> query can be folded into the primary
+    query much as though it were a simple sub-<literal>SELECT</literal> in
+    the primary query's <literal>FROM</literal> clause.  This results in
+    duplicate computations if the primary query refers to
+    that <literal>WITH</literal> query more than once; but if each such use
+    requires only a few rows of the <literal>WITH</literal> query's total
+    output, <literal>NOT MATERIALIZED</literal> can provide a net savings by
+    allowing the queries to be optimized jointly.
+    <literal>NOT MATERIALIZED</literal> is ignored if it is attached to
+    a <literal>WITH</literal> query that is recursive or is not
+    side-effect-free (i.e., is not a plain <literal>SELECT</literal>
+    containing no volatile functions).
+   </para>
+
+   <para>
+    By default, a side-effect-free <literal>WITH</literal> query is folded
+    into the primary query if it is used exactly once in the primary
+    query's <literal>FROM</literal> clause.  This allows joint optimization
+    of the two query levels in situations where that should be semantically
+    invisible.  However, such folding can be prevented by marking the
+    <literal>WITH</literal> query as <literal>MATERIALIZED</literal>.
+    That might be useful, for example, if the <literal>WITH</literal> query
+    has side-effects that are hidden from the planner.
+    <productname>PostgreSQL</productname> versions before v12 never did
+    such folding, so another possible reason for
+    using <literal>MATERIALIZED</literal> is to duplicate the pre-v12
+    behavior.
    </para>

    <para>
@@ -2087,6 +2120,12 @@ SELECT distributors.* WHERE distributors.name = 'Westward';
    <para>
     <literal>ROWS FROM( ... )</literal> is an extension of the SQL standard.
    </para>
+
+   <para>
+    The <literal>MATERIALIZED</literal> and <literal>NOT
+    MATERIALIZED</literal> options of <literal>WITH</literal> are extensions
+    of the SQL standard.
+   </para>
   </refsect2>

  </refsect1>
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 3eb7e95..3b6e206 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2536,6 +2536,7 @@ _copyCommonTableExpr(const CommonTableExpr *from)

     COPY_STRING_FIELD(ctename);
     COPY_NODE_FIELD(aliascolnames);
+    COPY_SCALAR_FIELD(ctematerialized);
     COPY_NODE_FIELD(ctequery);
     COPY_LOCATION_FIELD(location);
     COPY_SCALAR_FIELD(cterecursive);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 5c4fa7d..e7b0fae 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2791,6 +2791,7 @@ _equalCommonTableExpr(const CommonTableExpr *a, const CommonTableExpr *b)
 {
     COMPARE_STRING_FIELD(ctename);
     COMPARE_NODE_FIELD(aliascolnames);
+    COMPARE_SCALAR_FIELD(ctematerialized);
     COMPARE_NODE_FIELD(ctequery);
     COMPARE_LOCATION_FIELD(location);
     COMPARE_SCALAR_FIELD(cterecursive);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 0fde876..9035575 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2980,6 +2980,7 @@ _outCommonTableExpr(StringInfo str, const CommonTableExpr *node)

     WRITE_STRING_FIELD(ctename);
     WRITE_NODE_FIELD(aliascolnames);
+    WRITE_ENUM_FIELD(ctematerialized, CTEMaterialize);
     WRITE_NODE_FIELD(ctequery);
     WRITE_LOCATION_FIELD(location);
     WRITE_BOOL_FIELD(cterecursive);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index ec6f256..be30894 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -418,6 +418,7 @@ _readCommonTableExpr(void)

     READ_STRING_FIELD(ctename);
     READ_NODE_FIELD(aliascolnames);
+    READ_ENUM_FIELD(ctematerialized, CTEMaterialize);
     READ_NODE_FIELD(ctequery);
     READ_LOCATION_FIELD(location);
     READ_BOOL_FIELD(cterecursive);
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 4465f00..67f276a 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -645,8 +645,8 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
     root->partColsUpdated = false;

     /*
-     * If there is a WITH list, process each WITH query and build an initplan
-     * SubPlan structure for it.
+     * If there is a WITH list, process each WITH query and either convert it
+     * to RTE_SUBQUERY RTE(s) or build an initplan SubPlan structure for it.
      */
     if (parse->cteList)
         SS_process_ctes(root);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 64272dd..dde3715 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -57,6 +57,14 @@ typedef struct finalize_primnode_context
     Bitmapset  *paramids;        /* Non-local PARAM_EXEC paramids found */
 } finalize_primnode_context;

+typedef struct inline_cte_walker_context
+{
+    const char *ctename;        /* name and relative level of target CTE */
+    int            levelsup;
+    int            refcount;        /* number of remaining references */
+    Query       *ctequery;        /* query to substitute */
+} inline_cte_walker_context;
+

 static Node *build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot,
               List *plan_params,
@@ -75,6 +83,10 @@ static Node *convert_testexpr_mutator(Node *node,
 static bool subplan_is_hashable(Plan *plan);
 static bool testexpr_is_hashable(Node *testexpr);
 static bool hash_ok_operator(OpExpr *expr);
+static bool contain_dml(Node *node);
+static bool contain_dml_walker(Node *node, void *context);
+static void inline_cte(PlannerInfo *root, CommonTableExpr *cte);
+static bool inline_cte_walker(Node *node, inline_cte_walker_context *context);
 static bool simplify_EXISTS_query(PlannerInfo *root, Query *query);
 static Query *convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect,
                       Node **testexpr, List **paramIds);
@@ -804,10 +816,13 @@ hash_ok_operator(OpExpr *expr)
 /*
  * SS_process_ctes: process a query's WITH list
  *
- * We plan each interesting WITH item and convert it to an initplan.
+ * Consider each CTE in the WITH list and either ignore it (if it's an
+ * unreferenced SELECT), "inline" it to create a regular sub-SELECT-in-FROM,
+ * or convert it to an initplan.
+ *
  * A side effect is to fill in root->cte_plan_ids with a list that
  * parallels root->parse->cteList and provides the subplan ID for
- * each CTE's initplan.
+ * each CTE's initplan, or a dummy ID (-1) if we didn't make an initplan.
  */
 void
 SS_process_ctes(PlannerInfo *root)
@@ -839,6 +854,44 @@ SS_process_ctes(PlannerInfo *root)
         }

         /*
+         * Consider inlining the CTE (creating RTE_SUBQUERY RTE(s)) instead of
+         * implementing it as a separately-planned CTE.
+         *
+         * We cannot inline if any of these conditions hold:
+         *
+         * 1. The user said not to (the CTEMaterializeAlways option).
+         *
+         * 2. The CTE is recursive.
+         *
+         * 3. The CTE has side-effects; this includes either not being a plain
+         * SELECT, or containing volatile functions.  Inlining might change
+         * the side-effects, which would be bad.
+         *
+         * Otherwise, we have an option whether to inline or not.  That should
+         * always be a win if there's just a single reference, but if the CTE
+         * is multiply-referenced then it's unclear: inlining adds duplicate
+         * computations, but the ability to absorb restrictions from the outer
+         * query level could outweigh that.  We do not have nearly enough
+         * information at this point to tell whether that's true, so we let
+         * the user express a preference.  Our default behavior is to inline
+         * only singly-referenced CTEs, but a CTE marked CTEMaterializeNever
+         * will be inlined even if multiply referenced.
+         */
+        if ((cte->ctematerialized == CTEMaterializeNever ||
+             (cte->ctematerialized == CTEMaterializeDefault &&
+              cte->cterefcount == 1)) &&
+            !cte->cterecursive &&
+            cmdType == CMD_SELECT &&
+            !contain_dml(cte->ctequery) &&
+            !contain_volatile_functions(cte->ctequery))
+        {
+            inline_cte(root, cte);
+            /* Make a dummy entry in cte_plan_ids */
+            root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
+            continue;
+        }
+
+        /*
          * Copy the source Query node.  Probably not necessary, but let's keep
          * this similar to make_subplan.
          */
@@ -935,6 +988,127 @@ SS_process_ctes(PlannerInfo *root)
 }

 /*
+ * contain_dml: is any subquery not a plain SELECT?
+ *
+ * We reject SELECT FOR UPDATE/SHARE as well as INSERT etc.
+ */
+static bool
+contain_dml(Node *node)
+{
+    return contain_dml_walker(node, NULL);
+}
+
+static bool
+contain_dml_walker(Node *node, void *context)
+{
+    if (node == NULL)
+        return false;
+    if (IsA(node, Query))
+    {
+        Query       *query = (Query *) node;
+
+        if (query->commandType != CMD_SELECT ||
+            query->rowMarks != NIL)
+            return true;
+
+        return query_tree_walker(query, contain_dml_walker, context, 0);
+    }
+    return expression_tree_walker(node, contain_dml_walker, context);
+}
+
+/*
+ * inline_cte: convert RTE_CTE references to given CTE into RTE_SUBQUERYs
+ */
+static void
+inline_cte(PlannerInfo *root, CommonTableExpr *cte)
+{
+    struct inline_cte_walker_context context;
+
+    context.ctename = cte->ctename;
+    /* Start at levelsup = -1 because we'll immediately increment it */
+    context.levelsup = -1;
+    context.refcount = cte->cterefcount;
+    context.ctequery = castNode(Query, cte->ctequery);
+
+    (void) inline_cte_walker((Node *) root->parse, &context);
+
+    /* Assert we replaced all references */
+    Assert(context.refcount == 0);
+}
+
+static bool
+inline_cte_walker(Node *node, inline_cte_walker_context *context)
+{
+    if (node == NULL)
+        return false;
+    if (IsA(node, Query))
+    {
+        Query       *query = (Query *) node;
+
+        context->levelsup++;
+
+        /*
+         * Visit the query's RTE nodes after their contents; otherwise
+         * query_tree_walker would descend into the newly inlined CTE query,
+         * which we don't want.
+         */
+        (void) query_tree_walker(query, inline_cte_walker, context,
+                                 QTW_EXAMINE_RTES_AFTER);
+
+        context->levelsup--;
+
+        return false;
+    }
+    else if (IsA(node, RangeTblEntry))
+    {
+        RangeTblEntry *rte = (RangeTblEntry *) node;
+
+        if (rte->rtekind == RTE_CTE &&
+            strcmp(rte->ctename, context->ctename) == 0 &&
+            rte->ctelevelsup == context->levelsup)
+        {
+            /*
+             * Found a reference to replace.  Generate a copy of the CTE query
+             * with appropriate level adjustment for outer references (e.g.,
+             * to other CTEs).
+             */
+            Query       *newquery = copyObject(context->ctequery);
+
+            if (context->levelsup > 0)
+                IncrementVarSublevelsUp((Node *) newquery, context->levelsup, 1);
+
+            /*
+             * Convert the RTE_CTE RTE into a RTE_SUBQUERY.
+             *
+             * Historically, a FOR UPDATE clause has been treated as extending
+             * into views and subqueries, but not into CTEs.  We preserve this
+             * distinction by not trying to push rowmarks into the new
+             * subquery.
+             */
+            rte->rtekind = RTE_SUBQUERY;
+            rte->subquery = newquery;
+            rte->security_barrier = false;
+
+            /* Zero out CTE-specific fields */
+            rte->ctename = NULL;
+            rte->ctelevelsup = 0;
+            rte->self_reference = false;
+            rte->coltypes = NIL;
+            rte->coltypmods = NIL;
+            rte->colcollations = NIL;
+
+            /* Count the number of replacements we've done */
+            context->refcount--;
+        }
+
+        return false;
+    }
+
+    return expression_tree_walker(node, inline_cte_walker, context);
+}
+
+
+/*
  * convert_ANY_sublink_to_join: try to convert an ANY SubLink to a join
  *
  * The caller has found an ANY SubLink at the top level of one of the query's
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c1faf41..56c31d9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -479,7 +479,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>    row explicit_row implicit_row type_list array_expr_list
 %type <node>    case_expr case_arg when_clause case_default
 %type <list>    when_clause_list
-%type <ival>    sub_type
+%type <ival>    sub_type opt_materialized
 %type <value>    NumericOnly
 %type <list>    NumericOnly_list
 %type <alias>    alias_clause opt_alias_clause
@@ -11322,17 +11322,24 @@ cte_list:
         | cte_list ',' common_table_expr        { $$ = lappend($1, $3); }
         ;

-common_table_expr:  name opt_name_list AS '(' PreparableStmt ')'
+common_table_expr:  name opt_name_list AS opt_materialized '(' PreparableStmt ')'
             {
                 CommonTableExpr *n = makeNode(CommonTableExpr);
                 n->ctename = $1;
                 n->aliascolnames = $2;
-                n->ctequery = $5;
+                n->ctematerialized = $4;
+                n->ctequery = $6;
                 n->location = @1;
                 $$ = (Node *) n;
             }
         ;

+opt_materialized:
+        MATERIALIZED                            { $$ = CTEMaterializeAlways; }
+        | NOT MATERIALIZED                        { $$ = CTEMaterializeNever; }
+        | /*EMPTY*/                                { $$ = CTEMaterializeDefault; }
+        ;
+
 opt_with_clause:
         with_clause                                { $$ = $1; }
         | /*EMPTY*/                                { $$ = NULL; }
@@ -16214,6 +16221,7 @@ makeRecursiveViewSelect(char *relname, List *aliases, Node *query)
     /* create common table expression */
     cte->ctename = relname;
     cte->aliascolnames = aliases;
+    cte->ctematerialized = CTEMaterializeDefault;
     cte->ctequery = query;
     cte->location = -1;

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 302df16..1369602 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -5166,7 +5166,19 @@ get_with_clause(Query *query, deparse_context *context)
             }
             appendStringInfoChar(buf, ')');
         }
-        appendStringInfoString(buf, " AS (");
+        appendStringInfoString(buf, " AS ");
+        switch (cte->ctematerialized)
+        {
+            case CTEMaterializeDefault:
+                break;
+            case CTEMaterializeAlways:
+                appendStringInfoString(buf, "MATERIALIZED ");
+                break;
+            case CTEMaterializeNever:
+                appendStringInfoString(buf, "NOT MATERIALIZED ");
+                break;
+        }
+        appendStringInfoChar(buf, '(');
         if (PRETTY_INDENT(context))
             appendContextKeyword(context, "", 0, 0, 0);
         get_query_def((Query *) cte->ctequery, buf, context->namespaces, NULL,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index addc2c2..ce8161d 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1399,11 +1399,19 @@ typedef struct OnConflictClause
  *
  * We don't currently support the SEARCH or CYCLE clause.
  */
+typedef enum CTEMaterialize
+{
+    CTEMaterializeDefault,        /* no option specified */
+    CTEMaterializeAlways,        /* MATERIALIZED */
+    CTEMaterializeNever            /* NOT MATERIALIZED */
+} CTEMaterialize;
+
 typedef struct CommonTableExpr
 {
     NodeTag        type;
     char       *ctename;        /* query name (never qualified) */
     List       *aliascolnames;    /* optional list of column names */
+    CTEMaterialize ctematerialized; /* is this an optimization fence? */
     /* SelectStmt/InsertStmt/etc before parse analysis, Query afterwards: */
     Node       *ctequery;        /* the CTE's subquery */
     int            location;        /* token location, or -1 if unknown */
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 1d12b01..2e17049 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -2179,7 +2179,7 @@ EXPLAIN (COSTS OFF) EXECUTE plancache_test;
    Filter: (((a % 2) = 0) AND f_leak(b))
 (2 rows)

-PREPARE plancache_test2 AS WITH q AS (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
+PREPARE plancache_test2 AS WITH q AS MATERIALIZED (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
 EXPLAIN (COSTS OFF) EXECUTE plancache_test2;
                    QUERY PLAN
 -------------------------------------------------
@@ -2192,7 +2192,7 @@ EXPLAIN (COSTS OFF) EXECUTE plancache_test2;
          ->  Seq Scan on z2
 (7 rows)

-PREPARE plancache_test3 AS WITH q AS (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
+PREPARE plancache_test3 AS WITH q AS MATERIALIZED (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test3;
                      QUERY PLAN
 -----------------------------------------------------
@@ -2826,7 +2826,7 @@ ALTER TABLE t1 ENABLE ROW LEVEL SECURITY;
 GRANT ALL ON t1 TO regress_rls_bob;
 INSERT INTO t1 (SELECT x, md5(x::text) FROM generate_series(0,20) x);
 SET SESSION AUTHORIZATION regress_rls_bob;
-WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
 NOTICE:  f_leak => cfcd208495d565ef66e7dff9f98764da
 NOTICE:  f_leak => c81e728d9d4c2f636f067f89cc14862c
 NOTICE:  f_leak => a87ff679a2f3e71d9181a67b7542122c
@@ -2853,7 +2853,8 @@ NOTICE:  f_leak => 98f13708210194c475687be6106a3b84
  20 | 98f13708210194c475687be6106a3b84
 (11 rows)

-EXPLAIN (COSTS OFF) WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+EXPLAIN (COSTS OFF)
+WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
                    QUERY PLAN
 -------------------------------------------------
  CTE Scan on cte1
diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out
index d6a1a33..14757b4 100644
--- a/src/test/regress/expected/rowtypes.out
+++ b/src/test/regress/expected/rowtypes.out
@@ -1037,7 +1037,7 @@ from (values (1,row(1,2)), (1,row(null,null)), (1,null),
 (6 rows)

 explain (verbose, costs off)
-with r(a,b) as
+with r(a,b) as materialized
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
@@ -1050,7 +1050,7 @@ select r, r is null as isnull, r is not null as isnotnull from r;
            Output: "*VALUES*".column1, "*VALUES*".column2
 (5 rows)

-with r(a,b) as
+with r(a,b) as materialized
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index e384cd2..bb10e27 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3108,7 +3108,7 @@ explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
 (5 rows)

 -- ensure upserting into a rule, with a CTE (different offsets!) works
-WITH data(hat_name, hat_color) AS (
+WITH data(hat_name, hat_color) AS MATERIALIZED (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
@@ -3122,7 +3122,8 @@ RETURNING *;
  h9         | blue
 (2 rows)

-EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
+EXPLAIN (costs off)
+WITH data(hat_name, hat_color) AS MATERIALIZED (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index 588d069..98feb41 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -1154,3 +1154,159 @@ fetch backward all in c1;
 (2 rows)

 commit;
+--
+-- Tests for CTE inlining behavior
+--
+-- Basic subquery that can be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+            QUERY PLAN
+----------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1
+   Filter: (subselect_tbl.f1 = 1)
+(3 rows)
+
+-- Explicitly request materialization
+explain (verbose, costs off)
+with x as materialized (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+                QUERY PLAN
+------------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Seq Scan on public.subselect_tbl
+           Output: subselect_tbl.f1
+(6 rows)
+
+-- Stable functions are safe to inline
+explain (verbose, costs off)
+with x as (select * from (select f1, now() from subselect_tbl) ss)
+select * from x where f1 = 1;
+            QUERY PLAN
+-----------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1, now()
+   Filter: (subselect_tbl.f1 = 1)
+(3 rows)
+
+-- Volatile functions prevent inlining
+explain (verbose, costs off)
+with x as (select * from (select f1, random() from subselect_tbl) ss)
+select * from x where f1 = 1;
+                  QUERY PLAN
+----------------------------------------------
+ CTE Scan on x
+   Output: x.f1, x.random
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Seq Scan on public.subselect_tbl
+           Output: subselect_tbl.f1, random()
+(6 rows)
+
+-- SELECT FOR UPDATE cannot be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl for update) ss)
+select * from x where f1 = 1;
+                             QUERY PLAN
+--------------------------------------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Subquery Scan on ss
+           Output: ss.f1
+           ->  LockRows
+                 Output: subselect_tbl.f1, subselect_tbl.ctid
+                 ->  Seq Scan on public.subselect_tbl
+                       Output: subselect_tbl.f1, subselect_tbl.ctid
+(10 rows)
+
+-- Multiply-referenced CTEs are inlined only when requested
+explain (verbose, costs off)
+with x as (select * from (select f1, now() as n from subselect_tbl) ss)
+select * from x, x x2 where x.n = x2.n;
+                QUERY PLAN
+-------------------------------------------
+ Merge Join
+   Output: x.f1, x.n, x2.f1, x2.n
+   Merge Cond: (x.n = x2.n)
+   CTE x
+     ->  Seq Scan on public.subselect_tbl
+           Output: subselect_tbl.f1, now()
+   ->  Sort
+         Output: x.f1, x.n
+         Sort Key: x.n
+         ->  CTE Scan on x
+               Output: x.f1, x.n
+   ->  Sort
+         Output: x2.f1, x2.n
+         Sort Key: x2.n
+         ->  CTE Scan on x x2
+               Output: x2.f1, x2.n
+(16 rows)
+
+explain (verbose, costs off)
+with x as not materialized (select * from (select f1, now() as n from subselect_tbl) ss)
+select * from x, x x2 where x.n = x2.n;
+                                 QUERY PLAN
+----------------------------------------------------------------------------
+ Result
+   Output: subselect_tbl.f1, now(), subselect_tbl_1.f1, now()
+   One-Time Filter: (now() = now())
+   ->  Nested Loop
+         Output: subselect_tbl.f1, subselect_tbl_1.f1
+         ->  Seq Scan on public.subselect_tbl
+               Output: subselect_tbl.f1, subselect_tbl.f2, subselect_tbl.f3
+         ->  Materialize
+               Output: subselect_tbl_1.f1
+               ->  Seq Scan on public.subselect_tbl subselect_tbl_1
+                     Output: subselect_tbl_1.f1
+(11 rows)
+
+-- Check handling of outer references
+explain (verbose, costs off)
+with x as (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+         QUERY PLAN
+-----------------------------
+ Seq Scan on public.int4_tbl
+   Output: int4_tbl.f1
+(2 rows)
+
+explain (verbose, costs off)
+with x as materialized (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+             QUERY PLAN
+-------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   CTE x
+     ->  Seq Scan on public.int4_tbl
+           Output: int4_tbl.f1
+(5 rows)
+
+-- Ensure that we inline the currect CTE when there are
+-- multiple CTEs with the same name
+explain (verbose, costs off)
+with x as (select 1 as y)
+select * from (with x as (select 2 as y) select * from x) ss;
+ QUERY PLAN
+-------------
+ Result
+   Output: 2
+(2 rows)
+
+-- Row marks are not pushed into CTEs
+explain (verbose, costs off)
+with x as (select * from subselect_tbl)
+select * from x for update;
+                           QUERY PLAN
+----------------------------------------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1, subselect_tbl.f2, subselect_tbl.f3
+(2 rows)
+
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 38e9b38..52da276 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -840,10 +840,10 @@ EXPLAIN (COSTS OFF) SELECT * FROM z1 WHERE f_leak(b);
 PREPARE plancache_test AS SELECT * FROM z1 WHERE f_leak(b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test;

-PREPARE plancache_test2 AS WITH q AS (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
+PREPARE plancache_test2 AS WITH q AS MATERIALIZED (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
 EXPLAIN (COSTS OFF) EXECUTE plancache_test2;

-PREPARE plancache_test3 AS WITH q AS (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
+PREPARE plancache_test3 AS WITH q AS MATERIALIZED (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test3;

 SET ROLE regress_rls_group1;
@@ -1071,8 +1071,9 @@ INSERT INTO t1 (SELECT x, md5(x::text) FROM generate_series(0,20) x);

 SET SESSION AUTHORIZATION regress_rls_bob;

-WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
-EXPLAIN (COSTS OFF) WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+EXPLAIN (COSTS OFF)
+WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;

 WITH cte1 AS (UPDATE t1 SET a = a + 1 RETURNING *) SELECT * FROM cte1; --fail
 WITH cte1 AS (UPDATE t1 SET a = a RETURNING *) SELECT * FROM cte1; --ok
diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql
index e6d3898..b3b0774 100644
--- a/src/test/regress/sql/rowtypes.sql
+++ b/src/test/regress/sql/rowtypes.sql
@@ -428,12 +428,12 @@ from (values (1,row(1,2)), (1,row(null,null)), (1,null),
              (null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);

 explain (verbose, costs off)
-with r(a,b) as
+with r(a,b) as materialized
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;

-with r(a,b) as
+with r(a,b) as materialized
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index f4ee30e..6666951 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1132,7 +1132,7 @@ SELECT tablename, rulename, definition FROM pg_rules
 explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;

 -- ensure upserting into a rule, with a CTE (different offsets!) works
-WITH data(hat_name, hat_color) AS (
+WITH data(hat_name, hat_color) AS MATERIALIZED (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
@@ -1140,7 +1140,8 @@ WITH data(hat_name, hat_color) AS (
 INSERT INTO hats
     SELECT * FROM data
 RETURNING *;
-EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
+EXPLAIN (costs off)
+WITH data(hat_name, hat_color) AS MATERIALIZED (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 843f511..f4093e1 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -625,3 +625,61 @@ move forward all in c1;
 fetch backward all in c1;

 commit;
+
+--
+-- Tests for CTE inlining behavior
+--
+
+-- Basic subquery that can be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Explicitly request materialization
+explain (verbose, costs off)
+with x as materialized (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Stable functions are safe to inline
+explain (verbose, costs off)
+with x as (select * from (select f1, now() from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Volatile functions prevent inlining
+explain (verbose, costs off)
+with x as (select * from (select f1, random() from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- SELECT FOR UPDATE cannot be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl for update) ss)
+select * from x where f1 = 1;
+
+-- Multiply-referenced CTEs are inlined only when requested
+explain (verbose, costs off)
+with x as (select * from (select f1, now() as n from subselect_tbl) ss)
+select * from x, x x2 where x.n = x2.n;
+
+explain (verbose, costs off)
+with x as not materialized (select * from (select f1, now() as n from subselect_tbl) ss)
+select * from x, x x2 where x.n = x2.n;
+
+-- Check handling of outer references
+explain (verbose, costs off)
+with x as (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+
+explain (verbose, costs off)
+with x as materialized (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+
+-- Ensure that we inline the currect CTE when there are
+-- multiple CTEs with the same name
+explain (verbose, costs off)
+with x as (select 1 as y)
+select * from (with x as (select 2 as y) select * from x) ss;
+
+-- Row marks are not pushed into CTEs
+explain (verbose, costs off)
+with x as (select * from subselect_tbl)
+select * from x for update;

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 that's not marked
 Tom> volatile, before there's a real need for that. Am I missing
 Tom> something? If I'm not, then we're in a really good place
 Tom> backwards-compatibility-wise, because the new default behavior
 Tom> shouldn't break any cases where people weren't cheating.

The cases where the new default will break things are where people used
WITH to force a choice of join order or otherwise constrain the planner
in order to avoid a misplan.

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 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 always chooses
only based on refcount).

-- 
Andrew (irc:RhodiumToad)

Andrew Gierth <andrew@tao11.riddles.org.uk> 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 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 always chooses
> only based on refcount).

I have no objection to documenting it like that; I just don't want us
to go off into the weeds trying to actually implement something smarter
for v12.

            regards, tom lane

On 1/27/19 4:21 PM, Tom Lane wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> 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 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 always chooses
>> only based on refcount).
> 
> I have no objection to documenting it like that; I just don't want us
> to go off into the weeds trying to actually implement something smarter
> for v12.

+1

Andreas

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 only remaining question I see is the one Andrew 
raised about if we should change the language to allow for future 
versions of PostgreSQL to add costing for when the same CTE is 
referenced multiple times.

Andreas

On Mon, Jan 21, 2019 at 10:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andreas Karlsson <andreas@proxel.se> 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, I'd independently thought that the latter is more readable,
> and probably less likely to have syntax problems with future
> extensions (since AS is already fully reserved).  Didn't get
> around to mentioning it yet, but +1 for putting AS first.

It seems to me that as long as the query has to be surrounded by
non-optional parentheses and the options list never starts with a
parenthesis, there isn't much room for a grammar conflict either way.
If the query didn't have to be surrounded by parentheses, you would
want to put the options before the word AS so that the word AS would
serve as an unambiguous terminator for the options specification, but
if the query must be preceded by an opening parenthesis then as long
as the options list can't include an option that begins with such a
parenthesis we are in good shape.

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.  And
then you're going to have trouble getting bison to figure out what to
do after...

WITH something AS ( some_keyword

...because some_keyword might be the an option name or the first word
of a query.  And putting that syntax before AS isn't any better,
because now it can be confused with a column name list.

I am not deeply worked up about this, just proposing some things to think about.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Sun, Jan 27, 2019 at 8:23 AM Andrew Gierth
<andrew@tao11.riddles.org.uk> 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 always chooses
> only based on refcount).

I, too, like this approach.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> 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
either before or after the AS.

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 think the only reason the syntax is MATERIALIZED with a D is that
that's already a keyword; it reads a bit awkwardly IMO.  But if we
were accepting a ColId there, there'd be room to adjust the spelling.

That said, this is still clunkier than the existing proposal, and
I'm not really convinced that we need to leave room for more options.

            regards, tom lane

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 think the only reason the syntax is MATERIALIZED with a D is that
> that's already a keyword; it reads a bit awkwardly IMO.  But if we
> were accepting a ColId there, there'd be room to adjust the spelling.

Or put it at the end?

    WITH ctename AS ( query ) MATERIALIZED

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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 (which isn't unlikely) I think people would
prefer to see the option(s) up front.

            regards, tom lane

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

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 looks more
natural to me.
--
Michael

Michael Paquier <michael@paquier.xyz> 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 at the front of the WITH clause looks more
> natural to me.

Well, we've managed to get agreement on the semantics of this thing,
let's not get hung up on the syntax details.

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 precedent of e.g. EXPLAIN, we could
probably allow just "MATERIALIZE" as well, with the boolean value
defaulting to true.

            regards, tom lane

On Tue, Jan 29, 2019 at 1:53 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Michael Paquier <michael@paquier.xyz> 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 at the front of the WITH clause looks more
> > natural to me.
>
> Well, we've managed to get agreement on the semantics of this thing,
> let's not get hung up on the syntax details.
>
> 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 precedent of e.g. EXPLAIN, we could
> probably allow just "MATERIALIZE" as well, with the boolean value
> defaulting to true.

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 )

merlin

Merlin Moncure <mmoncure@gmail.com> writes:
> On Tue, Jan 29, 2019 at 1:53 PM Tom Lane <tgl@sss.pgh.pa.us> 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 AS [ MATERIALIZATION { ON | OFF } ] ( query )

Hm.  Doesn't really seem to fit with our style for options elsewhere;
for instance, EXPLAIN's options are things like ANALYZE ON/OFF and
VERBOSE ON/OFF, not ANALYSIS and VERBOSITY.

            regards, tom lane

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 MATERIALIZED/NOT MATERIALIZED form seemed more
natural:

FROM GROCER OBTAIN WAXED CUCUMBERS. (this seems downright natural)
FROM GROCER OBTAIN NOT WAXED CUCUMBERS. (nearly natural, s/NOT /UN/)

FROM GROCER OBTAIN WAX ON CUCUMBERS. (these read oddly to me)
FROM GROCER OBTAIN WAX OFF CUCUMBERS.

I do understand Tom's point that the wax-on/wax-off form generalizes
more easily to non-boolean future options. It would really read
better as a parenthetical, so too bad parentheses are already taken
to go around the query.

While gawking at the bikeshed, one more thing came to mind:

I like to hold out hope [1] that, one day, the WITH grammar could
be extended to handle lexically-scoped option settings like those in
the ISO standard.

It doesn't seem to me that any of these current proposals would get
in the way of that. Just another thing to have in mind.

Regards,
-Chap


[1]
https://wiki.postgresql.org/wiki/PostgreSQL_vs_SQL/XML_Standards#XMLBINARY_and_XMLNAMESPACES

On Tue, Jan 29, 2019 at 02:52:44PM -0500, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> 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 at the front of the WITH clause looks more
> > natural to me.
> 
> Well, we've managed to get agreement on the semantics of this thing,
> let's not get hung up on the syntax details.
> 
> 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 } ) ] AS ( query ) [, ... ]

and it's a lot easier to add more query hints later.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

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

David Fetter <david@fetter.org> 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 } ) ] AS ( query ) [, ... ]

I take it you haven't actually been reading this thread.

            regards, tom lane

Merlin Moncure <mmoncure@gmail.com> writes:
> On Tue, Jan 29, 2019 at 1:53 PM Tom Lane <tgl@sss.pgh.pa.us> 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 AS [ MATERIALIZATION { ON | OFF } ] ( query )

Hm.  Doesn't really seem to fit with our style for options elsewhere;
for instance, EXPLAIN's options are things like ANALYZE ON/OFF and
VERBOSE ON/OFF, not ANALYSIS and VERBOSITY.

                        regards, 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 precedent of e.g. EXPLAIN, we could
> probably allow just "MATERIALIZE" as well, with the boolean value
> defaulting to true.

In hopes of moving things along, here's a version of the patch that
does it like that.  This demonstrates that, in fact, we can accept
"keyword [value] [, ...]" style options without any parens and
there's no syntax conflict.  We'd have to work a bit harder on the
actual code in gram.y if we wanted to handle multiple options,
but the Bison productions will work.

There's nothing particularly stopping us from accepting
"materialized" with a D in this syntax, instead of or in addition
to "materialize"; though I hesitate to mention it for fear of
another round of bikeshedding.

After further reflection I really don't like Andrew's suggestion
that we not document the rule that multiply-referenced CTEs won't
be inlined by default.  That would be giving up the principle
that WITH calculations are not done multiple times by default,
and I draw the line at that.  It's an often-useful behavior as
well as one that's been documented from day one, so I do not accept
the argument that we might someday override it on the basis of
nothing but planner cost estimates.

            regards, tom lane

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index ea2e4bc..9905593 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2927,6 +2927,7 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)

                 /* we store the string name because RTE_CTE RTEs need it */
                 APP_JUMB_STRING(cte->ctename);
+                APP_JUMB(cte->ctematerialized);
                 JumbleQuery(jstate, castNode(Query, cte->ctequery));
             }
             break;
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index b3894d0..226ba56 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -1888,7 +1888,7 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t

 -- join in CTE
 EXPLAIN (VERBOSE, COSTS OFF)
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZE ON (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
                                                              QUERY PLAN
              

-------------------------------------------------------------------------------------------------------------------------------------
  Limit
@@ -1905,7 +1905,7 @@ WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2
                Output: t.c1_1, t.c2_1, t.c1_3
 (12 rows)

-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZE ON (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
  c1_1 | c2_1
 ------+------
   101 |  101
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index f438165..6399b8b 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -493,8 +493,8 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
 SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR SHARE;
 -- join in CTE
 EXPLAIN (VERBOSE, COSTS OFF)
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1
FROMt ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZE ON (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZE ON (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1))
SELECTc1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10; 
 -- ctid with whole-row reference
 EXPLAIN (VERBOSE, COSTS OFF)
 SELECT t1.ctid, t1, t2, t1.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10;
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index 88bc189..0cd4056 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -2199,16 +2199,19 @@ SELECT n FROM t LIMIT 100;
   </para>

   <para>
-   A useful property of <literal>WITH</literal> queries is that they are evaluated
-   only once per execution of the parent query, even if they are referred to
-   more than once by the parent query or sibling <literal>WITH</literal> queries.
+   A useful property of <literal>WITH</literal> queries is that they are
+   normally evaluated only once per execution of the parent query, even if
+   they are referred to more than once by the parent query or
+   sibling <literal>WITH</literal> queries.
    Thus, expensive calculations that are needed in multiple places can be
    placed within a <literal>WITH</literal> query to avoid redundant work.  Another
    possible application is to prevent unwanted multiple evaluations of
    functions with side-effects.
-   However, the other side of this coin is that the optimizer is less able to
-   push restrictions from the parent query down into a <literal>WITH</literal> query
-   than an ordinary subquery.  The <literal>WITH</literal> query will generally be
+   However, the other side of this coin is that the optimizer is not able to
+   push restrictions from the parent query down into a multiply-referenced
+   <literal>WITH</literal> query, since that might affect all uses of the
+   <literal>WITH</literal> query's output when it should affect only one.
+   The multiply-referenced <literal>WITH</literal> query will be
    evaluated as written, without suppression of rows that the parent query
    might discard afterwards.  (But, as mentioned above, evaluation might stop
    early if the reference(s) to the query demand only a limited number of
@@ -2216,6 +2219,76 @@ SELECT n FROM t LIMIT 100;
   </para>

   <para>
+   However, if a <literal>WITH</literal> query is non-recursive and
+   side-effect-free (that is, it is a <literal>SELECT</literal> containing
+   no volatile functions) then it can be folded into the parent query,
+   allowing joint optimization of the two query levels.  By default, this
+   happens if the parent query references the <literal>WITH</literal> query
+   just once, but not if it references the <literal>WITH</literal> query
+   more than once.
+   You can override the decision by specifying <literal>MATERIALIZE
+   ON</literal> to force separate calculation of the <literal>WITH</literal>
+   query, or by specifying <literal>MATERIALIZE OFF</literal> to force it to
+   be merged into the parent query.
+   The latter choice risks duplicate computation of
+   the <literal>WITH</literal> query, but it can still give a net savings if
+   each usage of the <literal>WITH</literal> query needs only a small part
+   of the <literal>WITH</literal> query's full output.
+  </para>
+
+  <para>
+   A simple example of these rules is
+<programlisting>
+WITH w AS (
+    SELECT * FROM big_table
+)
+SELECT * FROM w WHERE key = 123;
+</programlisting>
+   This <literal>WITH</literal> query will be folded, producing the same
+   execution plan as
+<programlisting>
+SELECT * FROM big_table WHERE key = 123;
+</programlisting>
+   In particular, if there's an index on <structfield>key</structfield>,
+   it will probably be used to fetch just the rows having <literal>key =
+   123</literal>.  On the other hand, in
+<programlisting>
+WITH w AS (
+    SELECT * FROM big_table
+)
+SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
+WHERE w2.key = 123;
+</programlisting>
+   the <literal>WITH</literal> query will be materialized, producing a
+   temporary copy of <structname>big_table</structname> that is then
+   joined with itself — without benefit of any index.  This query
+   will be executed much more efficiently if written as
+<programlisting>
+WITH w AS MATERIALIZE OFF (
+    SELECT * FROM big_table
+)
+SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
+WHERE w2.key = 123;
+</programlisting>
+   so that the parent query's restrictions can be applied directly
+   to scans of <structname>big_table</structname>.
+  </para>
+
+  <para>
+   An example where <literal>MATERIALIZE OFF</literal> could be
+   undesirable is
+<programlisting>
+WITH w AS (
+    SELECT key, very_expensive_function(val) as f FROM some_table
+)
+SELECT * FROM w AS w1 JOIN w AS w2 ON w1.f = w2.f;
+</programlisting>
+   Here, materialization of the <literal>WITH</literal> query ensures
+   that <function>very_expensive_function</function> is evaluated only
+   once per table row, not twice.
+  </para>
+
+  <para>
    The examples above only show <literal>WITH</literal> being used with
    <command>SELECT</command>, but it can be attached in the same way to
    <command>INSERT</command>, <command>UPDATE</command>, or <command>DELETE</command>.
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 4db8142..6d29cf5 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -72,7 +72,7 @@ SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac

 <phrase>and <replaceable class="parameter">with_query</replaceable> is:</phrase>

-    <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> |
<replaceableclass="parameter">values</replaceable> | <replaceable class="parameter">insert</replaceable> | <replaceable
class="parameter">update</replaceable>| <replaceable class="parameter">delete</replaceable> ) 
+    <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS [ MATERIALIZE [ ON | OFF ] ] ( <replaceable
class="parameter">select</replaceable>| <replaceable class="parameter">values</replaceable> | <replaceable
class="parameter">insert</replaceable>| <replaceable class="parameter">update</replaceable> | <replaceable
class="parameter">delete</replaceable>) 

 TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
 </synopsis>
@@ -93,7 +93,8 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
       These effectively serve as temporary tables that can be referenced
       in the <literal>FROM</literal> list.  A <literal>WITH</literal> query
       that is referenced more than once in <literal>FROM</literal> is
-      computed only once.
+      computed only once,
+      unless specified otherwise with <literal>MATERIALIZE OFF</literal>.
       (See <xref linkend="sql-with" endterm="sql-with-title"/> below.)
      </para>
     </listitem>
@@ -273,8 +274,17 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
    </para>

    <para>
+    The primary query and the <literal>WITH</literal> queries are all
+    (notionally) executed at the same time.  This implies that the effects of
+    a data-modifying statement in <literal>WITH</literal> cannot be seen from
+    other parts of the query, other than by reading its <literal>RETURNING</literal>
+    output.  If two such data-modifying statements attempt to modify the same
+    row, the results are unspecified.
+   </para>
+
+   <para>
     A key property of <literal>WITH</literal> queries is that they
-    are evaluated only once per execution of the primary query,
+    are normally evaluated only once per execution of the primary query,
     even if the primary query refers to them more than once.
     In particular, data-modifying statements are guaranteed to be
     executed once and only once, regardless of whether the primary query
@@ -282,12 +292,35 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
    </para>

    <para>
-    The primary query and the <literal>WITH</literal> queries are all
-    (notionally) executed at the same time.  This implies that the effects of
-    a data-modifying statement in <literal>WITH</literal> cannot be seen from
-    other parts of the query, other than by reading its <literal>RETURNING</literal>
-    output.  If two such data-modifying statements attempt to modify the same
-    row, the results are unspecified.
+    However, a <literal>WITH</literal> query can be marked with
+    <literal>MATERIALIZE OFF</literal> to remove this guarantee.  In that
+    case, the <literal>WITH</literal> query can be folded into the primary
+    query much as though it were a simple sub-<literal>SELECT</literal> in
+    the primary query's <literal>FROM</literal> clause.  This results in
+    duplicate computations if the primary query refers to
+    that <literal>WITH</literal> query more than once; but if each such use
+    requires only a few rows of the <literal>WITH</literal> query's total
+    output, <literal>MATERIALIZE OFF</literal> can provide a net savings by
+    allowing the queries to be optimized jointly.
+    <literal>MATERIALIZE OFF</literal> is ignored if it is attached to
+    a <literal>WITH</literal> query that is recursive or is not
+    side-effect-free (i.e., is not a plain <literal>SELECT</literal>
+    containing no volatile functions).
+   </para>
+
+   <para>
+    By default, a side-effect-free <literal>WITH</literal> query is folded
+    into the primary query if it is used exactly once in the primary
+    query's <literal>FROM</literal> clause.  This allows joint optimization
+    of the two query levels in situations where that should be semantically
+    invisible.  However, such folding can be prevented by marking the
+    <literal>WITH</literal> query with <literal>MATERIALIZE [ ON ]</literal>.
+    That might be useful, for example, if the <literal>WITH</literal> query
+    is being used as an optimization fence to prevent the planner from
+    choosing a bad plan.
+    <productname>PostgreSQL</productname> versions before v12 never did
+    such folding, so queries written for older versions might rely on
+    <literal>WITH</literal> to act as an optimization fence.
    </para>

    <para>
@@ -2087,6 +2120,11 @@ SELECT distributors.* WHERE distributors.name = 'Westward';
    <para>
     <literal>ROWS FROM( ... )</literal> is an extension of the SQL standard.
    </para>
+
+   <para>
+    The <literal>MATERIALIZE ON/OFF</literal> options
+    of <literal>WITH</literal> are extensions of the SQL standard.
+   </para>
   </refsect2>

  </refsect1>
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index b44ead2..e15724b 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2536,6 +2536,7 @@ _copyCommonTableExpr(const CommonTableExpr *from)

     COPY_STRING_FIELD(ctename);
     COPY_NODE_FIELD(aliascolnames);
+    COPY_SCALAR_FIELD(ctematerialized);
     COPY_NODE_FIELD(ctequery);
     COPY_LOCATION_FIELD(location);
     COPY_SCALAR_FIELD(cterecursive);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 1e169e0..31499eb 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2791,6 +2791,7 @@ _equalCommonTableExpr(const CommonTableExpr *a, const CommonTableExpr *b)
 {
     COMPARE_STRING_FIELD(ctename);
     COMPARE_NODE_FIELD(aliascolnames);
+    COMPARE_SCALAR_FIELD(ctematerialized);
     COMPARE_NODE_FIELD(ctequery);
     COMPARE_LOCATION_FIELD(location);
     COMPARE_SCALAR_FIELD(cterecursive);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index f97cf37..0f31fca 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2979,6 +2979,7 @@ _outCommonTableExpr(StringInfo str, const CommonTableExpr *node)

     WRITE_STRING_FIELD(ctename);
     WRITE_NODE_FIELD(aliascolnames);
+    WRITE_ENUM_FIELD(ctematerialized, CTEMaterialize);
     WRITE_NODE_FIELD(ctequery);
     WRITE_LOCATION_FIELD(location);
     WRITE_BOOL_FIELD(cterecursive);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 3b00277..5aa4224 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -418,6 +418,7 @@ _readCommonTableExpr(void)

     READ_STRING_FIELD(ctename);
     READ_NODE_FIELD(aliascolnames);
+    READ_ENUM_FIELD(ctematerialized, CTEMaterialize);
     READ_NODE_FIELD(ctequery);
     READ_LOCATION_FIELD(location);
     READ_BOOL_FIELD(cterecursive);
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index b223972..5b0c90f 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -646,8 +646,8 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
     root->partColsUpdated = false;

     /*
-     * If there is a WITH list, process each WITH query and build an initplan
-     * SubPlan structure for it.
+     * If there is a WITH list, process each WITH query and either convert it
+     * to RTE_SUBQUERY RTE(s) or build an initplan SubPlan structure for it.
      */
     if (parse->cteList)
         SS_process_ctes(root);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 915c6d0..c721054 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -57,6 +57,14 @@ typedef struct finalize_primnode_context
     Bitmapset  *paramids;        /* Non-local PARAM_EXEC paramids found */
 } finalize_primnode_context;

+typedef struct inline_cte_walker_context
+{
+    const char *ctename;        /* name and relative level of target CTE */
+    int            levelsup;
+    int            refcount;        /* number of remaining references */
+    Query       *ctequery;        /* query to substitute */
+} inline_cte_walker_context;
+

 static Node *build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot,
               List *plan_params,
@@ -75,6 +83,10 @@ static Node *convert_testexpr_mutator(Node *node,
 static bool subplan_is_hashable(Plan *plan);
 static bool testexpr_is_hashable(Node *testexpr);
 static bool hash_ok_operator(OpExpr *expr);
+static bool contain_dml(Node *node);
+static bool contain_dml_walker(Node *node, void *context);
+static void inline_cte(PlannerInfo *root, CommonTableExpr *cte);
+static bool inline_cte_walker(Node *node, inline_cte_walker_context *context);
 static bool simplify_EXISTS_query(PlannerInfo *root, Query *query);
 static Query *convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect,
                       Node **testexpr, List **paramIds);
@@ -804,10 +816,13 @@ hash_ok_operator(OpExpr *expr)
 /*
  * SS_process_ctes: process a query's WITH list
  *
- * We plan each interesting WITH item and convert it to an initplan.
+ * Consider each CTE in the WITH list and either ignore it (if it's an
+ * unreferenced SELECT), "inline" it to create a regular sub-SELECT-in-FROM,
+ * or convert it to an initplan.
+ *
  * A side effect is to fill in root->cte_plan_ids with a list that
  * parallels root->parse->cteList and provides the subplan ID for
- * each CTE's initplan.
+ * each CTE's initplan, or a dummy ID (-1) if we didn't make an initplan.
  */
 void
 SS_process_ctes(PlannerInfo *root)
@@ -839,6 +854,44 @@ SS_process_ctes(PlannerInfo *root)
         }

         /*
+         * Consider inlining the CTE (creating RTE_SUBQUERY RTE(s)) instead of
+         * implementing it as a separately-planned CTE.
+         *
+         * We cannot inline if any of these conditions hold:
+         *
+         * 1. The user said not to (the CTEMaterializeAlways option).
+         *
+         * 2. The CTE is recursive.
+         *
+         * 3. The CTE has side-effects; this includes either not being a plain
+         * SELECT, or containing volatile functions.  Inlining might change
+         * the side-effects, which would be bad.
+         *
+         * Otherwise, we have an option whether to inline or not.  That should
+         * always be a win if there's just a single reference, but if the CTE
+         * is multiply-referenced then it's unclear: inlining adds duplicate
+         * computations, but the ability to absorb restrictions from the outer
+         * query level could outweigh that.  We do not have nearly enough
+         * information at this point to tell whether that's true, so we let
+         * the user express a preference.  Our default behavior is to inline
+         * only singly-referenced CTEs, but a CTE marked CTEMaterializeNever
+         * will be inlined even if multiply referenced.
+         */
+        if ((cte->ctematerialized == CTEMaterializeNever ||
+             (cte->ctematerialized == CTEMaterializeDefault &&
+              cte->cterefcount == 1)) &&
+            !cte->cterecursive &&
+            cmdType == CMD_SELECT &&
+            !contain_dml(cte->ctequery) &&
+            !contain_volatile_functions(cte->ctequery))
+        {
+            inline_cte(root, cte);
+            /* Make a dummy entry in cte_plan_ids */
+            root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
+            continue;
+        }
+
+        /*
          * Copy the source Query node.  Probably not necessary, but let's keep
          * this similar to make_subplan.
          */
@@ -935,6 +988,127 @@ SS_process_ctes(PlannerInfo *root)
 }

 /*
+ * contain_dml: is any subquery not a plain SELECT?
+ *
+ * We reject SELECT FOR UPDATE/SHARE as well as INSERT etc.
+ */
+static bool
+contain_dml(Node *node)
+{
+    return contain_dml_walker(node, NULL);
+}
+
+static bool
+contain_dml_walker(Node *node, void *context)
+{
+    if (node == NULL)
+        return false;
+    if (IsA(node, Query))
+    {
+        Query       *query = (Query *) node;
+
+        if (query->commandType != CMD_SELECT ||
+            query->rowMarks != NIL)
+            return true;
+
+        return query_tree_walker(query, contain_dml_walker, context, 0);
+    }
+    return expression_tree_walker(node, contain_dml_walker, context);
+}
+
+/*
+ * inline_cte: convert RTE_CTE references to given CTE into RTE_SUBQUERYs
+ */
+static void
+inline_cte(PlannerInfo *root, CommonTableExpr *cte)
+{
+    struct inline_cte_walker_context context;
+
+    context.ctename = cte->ctename;
+    /* Start at levelsup = -1 because we'll immediately increment it */
+    context.levelsup = -1;
+    context.refcount = cte->cterefcount;
+    context.ctequery = castNode(Query, cte->ctequery);
+
+    (void) inline_cte_walker((Node *) root->parse, &context);
+
+    /* Assert we replaced all references */
+    Assert(context.refcount == 0);
+}
+
+static bool
+inline_cte_walker(Node *node, inline_cte_walker_context *context)
+{
+    if (node == NULL)
+        return false;
+    if (IsA(node, Query))
+    {
+        Query       *query = (Query *) node;
+
+        context->levelsup++;
+
+        /*
+         * Visit the query's RTE nodes after their contents; otherwise
+         * query_tree_walker would descend into the newly inlined CTE query,
+         * which we don't want.
+         */
+        (void) query_tree_walker(query, inline_cte_walker, context,
+                                 QTW_EXAMINE_RTES_AFTER);
+
+        context->levelsup--;
+
+        return false;
+    }
+    else if (IsA(node, RangeTblEntry))
+    {
+        RangeTblEntry *rte = (RangeTblEntry *) node;
+
+        if (rte->rtekind == RTE_CTE &&
+            strcmp(rte->ctename, context->ctename) == 0 &&
+            rte->ctelevelsup == context->levelsup)
+        {
+            /*
+             * Found a reference to replace.  Generate a copy of the CTE query
+             * with appropriate level adjustment for outer references (e.g.,
+             * to other CTEs).
+             */
+            Query       *newquery = copyObject(context->ctequery);
+
+            if (context->levelsup > 0)
+                IncrementVarSublevelsUp((Node *) newquery, context->levelsup, 1);
+
+            /*
+             * Convert the RTE_CTE RTE into a RTE_SUBQUERY.
+             *
+             * Historically, a FOR UPDATE clause has been treated as extending
+             * into views and subqueries, but not into CTEs.  We preserve this
+             * distinction by not trying to push rowmarks into the new
+             * subquery.
+             */
+            rte->rtekind = RTE_SUBQUERY;
+            rte->subquery = newquery;
+            rte->security_barrier = false;
+
+            /* Zero out CTE-specific fields */
+            rte->ctename = NULL;
+            rte->ctelevelsup = 0;
+            rte->self_reference = false;
+            rte->coltypes = NIL;
+            rte->coltypmods = NIL;
+            rte->colcollations = NIL;
+
+            /* Count the number of replacements we've done */
+            context->refcount--;
+        }
+
+        return false;
+    }
+
+    return expression_tree_walker(node, inline_cte_walker, context);
+}
+
+
+/*
  * convert_ANY_sublink_to_join: try to convert an ANY SubLink to a join
  *
  * The caller has found an ANY SubLink at the top level of one of the query's
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c1faf41..5f3745a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -479,7 +479,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>    row explicit_row implicit_row type_list array_expr_list
 %type <node>    case_expr case_arg when_clause case_default
 %type <list>    when_clause_list
-%type <ival>    sub_type
+%type <ival>    sub_type cte_opts
 %type <value>    NumericOnly
 %type <list>    NumericOnly_list
 %type <alias>    alias_clause opt_alias_clause
@@ -11291,6 +11291,8 @@ simple_select:
  *
  * We don't currently support the SEARCH or CYCLE clause.
  *
+ * Postgres-specific options can be inserted after AS.
+ *
  * Recognizing WITH_LA here allows a CTE to be named TIME or ORDINALITY.
  */
 with_clause:
@@ -11322,17 +11324,50 @@ cte_list:
         | cte_list ',' common_table_expr        { $$ = lappend($1, $3); }
         ;

-common_table_expr:  name opt_name_list AS '(' PreparableStmt ')'
+common_table_expr:  name opt_name_list AS cte_opts '(' PreparableStmt ')'
             {
                 CommonTableExpr *n = makeNode(CommonTableExpr);
                 n->ctename = $1;
                 n->aliascolnames = $2;
-                n->ctequery = $5;
+                n->ctematerialized = $4;
+                n->ctequery = $6;
                 n->location = @1;
                 $$ = (Node *) n;
             }
         ;

+/*
+ * In general, we could allow arbitrary options for a CTE; for the moment
+ * this piggybacks on EXPLAIN's option productions.  Since only one option
+ * is actually supported, just reduce the list to an enum result immediately.
+ */
+cte_opts: explain_option_list
+            {
+                int            matopt = CTEMaterializeDefault;
+                ListCell   *option;
+                foreach(option, $1)
+                {
+                    DefElem   *defel = (DefElem *) lfirst(option);
+
+                    if (strcmp(defel->defname, "materialize") == 0)
+                    {
+                        if (defGetBoolean(defel))
+                            matopt = CTEMaterializeAlways;
+                        else
+                            matopt = CTEMaterializeNever;
+                    }
+                    else
+                        ereport(ERROR,
+                                (errcode(ERRCODE_SYNTAX_ERROR),
+                                 errmsg("unrecognized WITH option \"%s\"",
+                                        defel->defname),
+                                 parser_errposition(defel->location)));
+                }
+                $$ = matopt;
+            }
+        | /*EMPTY*/                                { $$ = CTEMaterializeDefault; }
+        ;
+
 opt_with_clause:
         with_clause                                { $$ = $1; }
         | /*EMPTY*/                                { $$ = NULL; }
@@ -16214,6 +16249,7 @@ makeRecursiveViewSelect(char *relname, List *aliases, Node *query)
     /* create common table expression */
     cte->ctename = relname;
     cte->aliascolnames = aliases;
+    cte->ctematerialized = CTEMaterializeDefault;
     cte->ctequery = query;
     cte->location = -1;

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 17a28c2..4103602 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -5166,7 +5166,19 @@ get_with_clause(Query *query, deparse_context *context)
             }
             appendStringInfoChar(buf, ')');
         }
-        appendStringInfoString(buf, " AS (");
+        appendStringInfoString(buf, " AS ");
+        switch (cte->ctematerialized)
+        {
+            case CTEMaterializeDefault:
+                break;
+            case CTEMaterializeAlways:
+                appendStringInfoString(buf, "MATERIALIZE ON ");
+                break;
+            case CTEMaterializeNever:
+                appendStringInfoString(buf, "MATERIALIZE OFF ");
+                break;
+        }
+        appendStringInfoChar(buf, '(');
         if (PRETTY_INDENT(context))
             appendContextKeyword(context, "", 0, 0, 0);
         get_query_def((Query *) cte->ctequery, buf, context->namespaces, NULL,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 2fe14d7..14bea39 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1402,11 +1402,19 @@ typedef struct OnConflictClause
  *
  * We don't currently support the SEARCH or CYCLE clause.
  */
+typedef enum CTEMaterialize
+{
+    CTEMaterializeDefault,        /* no option specified */
+    CTEMaterializeAlways,        /* MATERIALIZE ON */
+    CTEMaterializeNever            /* MATERIALIZE OFF */
+} CTEMaterialize;
+
 typedef struct CommonTableExpr
 {
     NodeTag        type;
     char       *ctename;        /* query name (never qualified) */
     List       *aliascolnames;    /* optional list of column names */
+    CTEMaterialize ctematerialized; /* is this an optimization fence? */
     /* SelectStmt/InsertStmt/etc before parse analysis, Query afterwards: */
     Node       *ctequery;        /* the CTE's subquery */
     int            location;        /* token location, or -1 if unknown */
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 1d12b01..8971f88 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -2179,7 +2179,7 @@ EXPLAIN (COSTS OFF) EXECUTE plancache_test;
    Filter: (((a % 2) = 0) AND f_leak(b))
 (2 rows)

-PREPARE plancache_test2 AS WITH q AS (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
+PREPARE plancache_test2 AS WITH q AS MATERIALIZE ON (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
 EXPLAIN (COSTS OFF) EXECUTE plancache_test2;
                    QUERY PLAN
 -------------------------------------------------
@@ -2192,7 +2192,7 @@ EXPLAIN (COSTS OFF) EXECUTE plancache_test2;
          ->  Seq Scan on z2
 (7 rows)

-PREPARE plancache_test3 AS WITH q AS (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
+PREPARE plancache_test3 AS WITH q AS MATERIALIZE ON (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test3;
                      QUERY PLAN
 -----------------------------------------------------
@@ -2826,7 +2826,7 @@ ALTER TABLE t1 ENABLE ROW LEVEL SECURITY;
 GRANT ALL ON t1 TO regress_rls_bob;
 INSERT INTO t1 (SELECT x, md5(x::text) FROM generate_series(0,20) x);
 SET SESSION AUTHORIZATION regress_rls_bob;
-WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+WITH cte1 AS MATERIALIZE ON (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
 NOTICE:  f_leak => cfcd208495d565ef66e7dff9f98764da
 NOTICE:  f_leak => c81e728d9d4c2f636f067f89cc14862c
 NOTICE:  f_leak => a87ff679a2f3e71d9181a67b7542122c
@@ -2853,7 +2853,8 @@ NOTICE:  f_leak => 98f13708210194c475687be6106a3b84
  20 | 98f13708210194c475687be6106a3b84
 (11 rows)

-EXPLAIN (COSTS OFF) WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+EXPLAIN (COSTS OFF)
+WITH cte1 AS MATERIALIZE ON (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
                    QUERY PLAN
 -------------------------------------------------
  CTE Scan on cte1
diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out
index 054faabb..e02f99a 100644
--- a/src/test/regress/expected/rowtypes.out
+++ b/src/test/regress/expected/rowtypes.out
@@ -1048,7 +1048,7 @@ from (values (1,row(1,2)), (1,row(null,null)), (1,null),
 (6 rows)

 explain (verbose, costs off)
-with r(a,b) as
+with r(a,b) as materialize on
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
@@ -1061,7 +1061,7 @@ select r, r is null as isnull, r is not null as isnotnull from r;
            Output: "*VALUES*".column1, "*VALUES*".column2
 (5 rows)

-with r(a,b) as
+with r(a,b) as materialize on
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 2c8e21b..2137abd 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3110,7 +3110,7 @@ explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
 (5 rows)

 -- ensure upserting into a rule, with a CTE (different offsets!) works
-WITH data(hat_name, hat_color) AS (
+WITH data(hat_name, hat_color) AS MATERIALIZE ON (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
@@ -3124,7 +3124,8 @@ RETURNING *;
  h9         | blue
 (2 rows)

-EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
+EXPLAIN (costs off)
+WITH data(hat_name, hat_color) AS MATERIALIZE ON (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index a54b4a5..a7ccbc1 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -1154,3 +1154,159 @@ fetch backward all in c1;
 (2 rows)

 commit;
+--
+-- Tests for CTE inlining behavior
+--
+-- Basic subquery that can be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+            QUERY PLAN
+----------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1
+   Filter: (subselect_tbl.f1 = 1)
+(3 rows)
+
+-- Explicitly request materialization
+explain (verbose, costs off)
+with x as materialize on (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+                QUERY PLAN
+------------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Seq Scan on public.subselect_tbl
+           Output: subselect_tbl.f1
+(6 rows)
+
+-- Stable functions are safe to inline
+explain (verbose, costs off)
+with x as (select * from (select f1, now() from subselect_tbl) ss)
+select * from x where f1 = 1;
+            QUERY PLAN
+-----------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1, now()
+   Filter: (subselect_tbl.f1 = 1)
+(3 rows)
+
+-- Volatile functions prevent inlining
+explain (verbose, costs off)
+with x as (select * from (select f1, random() from subselect_tbl) ss)
+select * from x where f1 = 1;
+                  QUERY PLAN
+----------------------------------------------
+ CTE Scan on x
+   Output: x.f1, x.random
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Seq Scan on public.subselect_tbl
+           Output: subselect_tbl.f1, random()
+(6 rows)
+
+-- SELECT FOR UPDATE cannot be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl for update) ss)
+select * from x where f1 = 1;
+                             QUERY PLAN
+--------------------------------------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   Filter: (x.f1 = 1)
+   CTE x
+     ->  Subquery Scan on ss
+           Output: ss.f1
+           ->  LockRows
+                 Output: subselect_tbl.f1, subselect_tbl.ctid
+                 ->  Seq Scan on public.subselect_tbl
+                       Output: subselect_tbl.f1, subselect_tbl.ctid
+(10 rows)
+
+-- Multiply-referenced CTEs are inlined only when requested
+explain (verbose, costs off)
+with x as (select * from (select f1, now() as n from subselect_tbl) ss)
+select * from x, x x2 where x.n = x2.n;
+                QUERY PLAN
+-------------------------------------------
+ Merge Join
+   Output: x.f1, x.n, x2.f1, x2.n
+   Merge Cond: (x.n = x2.n)
+   CTE x
+     ->  Seq Scan on public.subselect_tbl
+           Output: subselect_tbl.f1, now()
+   ->  Sort
+         Output: x.f1, x.n
+         Sort Key: x.n
+         ->  CTE Scan on x
+               Output: x.f1, x.n
+   ->  Sort
+         Output: x2.f1, x2.n
+         Sort Key: x2.n
+         ->  CTE Scan on x x2
+               Output: x2.f1, x2.n
+(16 rows)
+
+explain (verbose, costs off)
+with x as materialize off (select * from (select f1, now() as n from subselect_tbl) ss)
+select * from x, x x2 where x.n = x2.n;
+                                 QUERY PLAN
+----------------------------------------------------------------------------
+ Result
+   Output: subselect_tbl.f1, now(), subselect_tbl_1.f1, now()
+   One-Time Filter: (now() = now())
+   ->  Nested Loop
+         Output: subselect_tbl.f1, subselect_tbl_1.f1
+         ->  Seq Scan on public.subselect_tbl
+               Output: subselect_tbl.f1, subselect_tbl.f2, subselect_tbl.f3
+         ->  Materialize
+               Output: subselect_tbl_1.f1
+               ->  Seq Scan on public.subselect_tbl subselect_tbl_1
+                     Output: subselect_tbl_1.f1
+(11 rows)
+
+-- Check handling of outer references
+explain (verbose, costs off)
+with x as (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+         QUERY PLAN
+-----------------------------
+ Seq Scan on public.int4_tbl
+   Output: int4_tbl.f1
+(2 rows)
+
+explain (verbose, costs off)
+with x as materialize on (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+             QUERY PLAN
+-------------------------------------
+ CTE Scan on x
+   Output: x.f1
+   CTE x
+     ->  Seq Scan on public.int4_tbl
+           Output: int4_tbl.f1
+(5 rows)
+
+-- Ensure that we inline the currect CTE when there are
+-- multiple CTEs with the same name
+explain (verbose, costs off)
+with x as (select 1 as y)
+select * from (with x as (select 2 as y) select * from x) ss;
+ QUERY PLAN
+-------------
+ Result
+   Output: 2
+(2 rows)
+
+-- Row marks are not pushed into CTEs
+explain (verbose, costs off)
+with x as (select * from subselect_tbl)
+select * from x for update;
+                           QUERY PLAN
+----------------------------------------------------------------
+ Seq Scan on public.subselect_tbl
+   Output: subselect_tbl.f1, subselect_tbl.f2, subselect_tbl.f3
+(2 rows)
+
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 38e9b38..589e13f 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -840,10 +840,10 @@ EXPLAIN (COSTS OFF) SELECT * FROM z1 WHERE f_leak(b);
 PREPARE plancache_test AS SELECT * FROM z1 WHERE f_leak(b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test;

-PREPARE plancache_test2 AS WITH q AS (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
+PREPARE plancache_test2 AS WITH q AS MATERIALIZE ON (SELECT * FROM z1 WHERE f_leak(b)) SELECT * FROM q,z2;
 EXPLAIN (COSTS OFF) EXECUTE plancache_test2;

-PREPARE plancache_test3 AS WITH q AS (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
+PREPARE plancache_test3 AS WITH q AS MATERIALIZE ON (SELECT * FROM z2) SELECT * FROM q,z1 WHERE f_leak(z1.b);
 EXPLAIN (COSTS OFF) EXECUTE plancache_test3;

 SET ROLE regress_rls_group1;
@@ -1071,8 +1071,9 @@ INSERT INTO t1 (SELECT x, md5(x::text) FROM generate_series(0,20) x);

 SET SESSION AUTHORIZATION regress_rls_bob;

-WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
-EXPLAIN (COSTS OFF) WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+WITH cte1 AS MATERIALIZE ON (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;
+EXPLAIN (COSTS OFF)
+WITH cte1 AS MATERIALIZE ON (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1;

 WITH cte1 AS (UPDATE t1 SET a = a + 1 RETURNING *) SELECT * FROM cte1; --fail
 WITH cte1 AS (UPDATE t1 SET a = a RETURNING *) SELECT * FROM cte1; --ok
diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql
index 454d462..8eda763 100644
--- a/src/test/regress/sql/rowtypes.sql
+++ b/src/test/regress/sql/rowtypes.sql
@@ -434,12 +434,12 @@ from (values (1,row(1,2)), (1,row(null,null)), (1,null),
              (null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);

 explain (verbose, costs off)
-with r(a,b) as
+with r(a,b) as materialize on
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;

-with r(a,b) as
+with r(a,b) as materialize on
   (values (1,row(1,2)), (1,row(null,null)), (1,null),
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index f4ee30e..dc7317c 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1132,7 +1132,7 @@ SELECT tablename, rulename, definition FROM pg_rules
 explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;

 -- ensure upserting into a rule, with a CTE (different offsets!) works
-WITH data(hat_name, hat_color) AS (
+WITH data(hat_name, hat_color) AS MATERIALIZE ON (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
@@ -1140,7 +1140,8 @@ WITH data(hat_name, hat_color) AS (
 INSERT INTO hats
     SELECT * FROM data
 RETURNING *;
-EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
+EXPLAIN (costs off)
+WITH data(hat_name, hat_color) AS MATERIALIZE ON (
     VALUES ('h8', 'green'),
         ('h9', 'blue'),
         ('h7', 'forbidden')
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 843f511..336cf61 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -625,3 +625,61 @@ move forward all in c1;
 fetch backward all in c1;

 commit;
+
+--
+-- Tests for CTE inlining behavior
+--
+
+-- Basic subquery that can be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Explicitly request materialization
+explain (verbose, costs off)
+with x as materialize on (select * from (select f1 from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Stable functions are safe to inline
+explain (verbose, costs off)
+with x as (select * from (select f1, now() from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- Volatile functions prevent inlining
+explain (verbose, costs off)
+with x as (select * from (select f1, random() from subselect_tbl) ss)
+select * from x where f1 = 1;
+
+-- SELECT FOR UPDATE cannot be inlined
+explain (verbose, costs off)
+with x as (select * from (select f1 from subselect_tbl for update) ss)
+select * from x where f1 = 1;
+
+-- Multiply-referenced CTEs are inlined only when requested
+explain (verbose, costs off)
+with x as (select * from (select f1, now() as n from subselect_tbl) ss)
+select * from x, x x2 where x.n = x2.n;
+
+explain (verbose, costs off)
+with x as materialize off (select * from (select f1, now() as n from subselect_tbl) ss)
+select * from x, x x2 where x.n = x2.n;
+
+-- Check handling of outer references
+explain (verbose, costs off)
+with x as (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+
+explain (verbose, costs off)
+with x as materialize on (select * from int4_tbl)
+select * from (with y as (select * from x) select * from y) ss;
+
+-- Ensure that we inline the currect CTE when there are
+-- multiple CTEs with the same name
+explain (verbose, costs off)
+with x as (select 1 as y)
+select * from (with x as (select 2 as y) select * from x) ss;
+
+-- Row marks are not pushed into CTEs
+explain (verbose, costs off)
+with x as (select * from subselect_tbl)
+select * from x for update;

[snip]
There's nothing particularly stopping us from accepting
"materialized" with a D in this syntax, instead of or in addition
to "materialize"; though I hesitate to mention it for fear of
another round of bikeshedding.

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 times by default, and I
 Tom> draw the line at that. It's an often-useful behavior as well as
 Tom> one that's been documented from day one, so I do not accept the
 Tom> argument that we might someday override it on the basis of nothing
 Tom> but planner cost estimates.

The case that springs to mind is when a CTE with grouping is then joined
multiple times in the main query with different conditions. If the
planner is able to deduce (e.g. via ECs) that restrictions on grouped
columns can be pushed into the CTE, then inlining the CTE multiple times
might be a significant win. But if that isn't possible, then inlining
multiple times might be a significant loss.

If such a query is made into a view, then (given your position) the
decision of whether to inline has to be made at view creation time,
which doesn't seem desirable (even if we have to put up with it for the
present).

-- 
Andrew (irc:RhodiumToad)

On 28/01/2019 23:05, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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 (which isn't unlikely) I think people would
> prefer to see the option(s) up front.

On the other hand, the end is where the other options go (that we
haven't implemented yet).  See <search or cycle clause>.
-- 
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

Vik Fearing <vik.fearing@2ndquadrant.com> writes:
> On 28/01/2019 23:05, Tom Lane wrote:
>> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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 people would
>> prefer to see the option(s) up front.

> On the other hand, the end is where the other options go (that we
> haven't implemented yet).  See <search or cycle clause>.

Yeah, I noticed that too while working on the latest patch revision.
ISTM that's actually an argument for *not* putting PG-specific
syntax there.  We'd increase the risk of conflicting with future spec
additions, assuming that they continue to add stuff at the end rather
than just after AS.

            regards, tom lane

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 optionvalue [ , ... ] ]" if we
> > ever need to.  Looking at the precedent of e.g. EXPLAIN, we could
> > probably allow just "MATERIALIZE" as well, with the boolean value
> > defaulting to true.
> 
> In hopes of moving things along, here's a version of the patch that
> does it like that.  This demonstrates that, in fact, we can accept
> "keyword [value] [, ...]" style options without any parens and
> there's no syntax conflict.  We'd have to work a bit harder on the
> actual code in gram.y if we wanted to handle multiple options,
> but the Bison productions will work.
> 
> There's nothing particularly stopping us from accepting
> "materialized" with a D in this syntax, instead of or in addition
> to "materialize"; though I hesitate to mention it for fear of
> another round of bikeshedding.

I think "materialize" is the right word since "materialized" would be
past tense.

> After further reflection I really don't like Andrew's suggestion
> that we not document the rule that multiply-referenced CTEs won't
> be inlined by default.  That would be giving up the principle
> that WITH calculations are not done multiple times by default,
> and I draw the line at that.  It's an often-useful behavior as
> well as one that's been documented from day one, so I do not accept
> the argument that we might someday override it on the basis of
> nothing but planner cost estimates.

Thinking of the history of documenting optimizer issues, I think we
should document when CTEs are inlined by default, because the user will
want to know when they should override the default behavior.  When we
didn't document how PREPARED queries worked, we got many questions about
odd query performance until we finally documented it in 2016 in commit
fab9d1da4a213fab08fe2d263eedf2408bc4a27a.  If we change the inlining
behavior later, we can update the docs.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 times by default, and I
>  Tom> draw the line at that. It's an often-useful behavior as well as
>  Tom> one that's been documented from day one, so I do not accept the
>  Tom> argument that we might someday override it on the basis of nothing
>  Tom> but planner cost estimates.

> The case that springs to mind is when a CTE with grouping is then joined
> multiple times in the main query with different conditions. If the
> planner is able to deduce (e.g. via ECs) that restrictions on grouped
> columns can be pushed into the CTE, then inlining the CTE multiple times
> might be a significant win. But if that isn't possible, then inlining
> multiple times might be a significant loss.

Sure, but this is exactly the sort of situation where we should offer
a way for the user to force either decision to be made.  I think it's
very unlikely that we'll ever be in a position to make a realistic
cost-based decision for that.  Actually planning it out both ways would
be horrendously expensive (and probably none too reliable anyway, given
how shaky ndistinct estimates tend to be); and we certainly don't have
enough info to make a smart choice without doing that.

            regards, 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 lane

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.

If we're not really planning to add any more options, I'd register a
light vote for MATERIALIZED.  It reads easier, seems more grammatically
correct, and uses an existing word.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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 [MATERIALIZE [ON|OFF]] syntax).  But we gotta pick something.
> 
> If we're not really planning to add any more options, I'd register a
> light vote for MATERIALIZED.  It reads easier, seems more grammatically
> correct, and uses an existing word.

+1 for MATERIALIZED, as I proposed in
https://postgr.es/m/20170503173305.fetj4tz7kd56tjlr@alvherre.pgsql

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera
<alvherre@2ndquadrant.com> 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 versions
> > > I posted (that is, either AS [[NOT] MATERIALIZED] or
> > > AS [MATERIALIZE [ON|OFF]] syntax).  But we gotta pick something.
> >
> > If we're not really planning to add any more options, I'd register a
> > light vote for MATERIALIZED.  It reads easier, seems more grammatically
> > correct, and uses an existing word.
>
> +1 for MATERIALIZED, as I proposed in
> https://postgr.es/m/20170503173305.fetj4tz7kd56tjlr@alvherre.pgsql

Seconded!

merlin

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 $main_query".
> 
> It's the same as "materialized view".

Agreed, thanks.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

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.

I am happy with either of those.

Andreas

On 2/14/19 8:22 PM, Merlin Moncure wrote:
> On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera
> <alvherre@2ndquadrant.com> 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 versions
>>>> I posted (that is, either AS [[NOT] MATERIALIZED] or
>>>> AS [MATERIALIZE [ON|OFF]] syntax).  But we gotta pick something.
>>>
>>> If we're not really planning to add any more options, I'd register a
>>> light vote for MATERIALIZED.  It reads easier, seems more grammatically
>>> correct, and uses an existing word.
>>
>> +1 for MATERIALIZED, as I proposed in
>> https://postgr.es/m/20170503173305.fetj4tz7kd56tjlr@alvherre.pgsql
> 
> Seconded!
> 

+1 to MATERIALIZED too

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> On 2/14/19 8:22 PM, Merlin Moncure wrote:
>> On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera
>> <alvherre@2ndquadrant.com> 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, seems more grammatically
>>>> correct, and uses an existing word.

>>> +1 for MATERIALIZED, as I proposed in
>>> https://postgr.es/m/20170503173305.fetj4tz7kd56tjlr@alvherre.pgsql

>> Seconded!

> +1 to MATERIALIZED too

I was expecting more controversy ... pushed that way.

            regards, tom lane

On 2/16/19 4:14 PM, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>> On 2/14/19 8:22 PM, Merlin Moncure wrote:
>>> On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera
>>> <alvherre@2ndquadrant.com> 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, seems more grammatically
>>>>> correct, and uses an existing word.
>
>>>> +1 for MATERIALIZED, as I proposed in
>>>> https://postgr.es/m/20170503173305.fetj4tz7kd56tjlr@alvherre.pgsql
>
>>> Seconded!
>
>> +1 to MATERIALIZED too
>
> I was expecting more controversy ... pushed that way.

This is awesome. Thank you everyone for working on this!

On 2/16/19 10:14 PM, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>> On 2/14/19 8:22 PM, Merlin Moncure wrote:
>>> On Thu, Feb 14, 2019 at 10:02 AM Alvaro Herrera
>>> <alvherre@2ndquadrant.com> 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, seems more grammatically
>>>>> correct, and uses an existing word.
> 
>>>> +1 for MATERIALIZED, as I proposed in
>>>> https://postgr.es/m/20170503173305.fetj4tz7kd56tjlr@alvherre.pgsql
> 
>>> Seconded!
> 
>> +1 to MATERIALIZED too
> 
> I was expecting more controversy ... pushed that way.
> 

As you wish. I withdraw my previous vote and I propose

   AS [NOT] MATERIALIZED [yes|false]

;-)

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

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)
            SELECT z.a || z1.a AS a FROM z CROSS JOIN z AS z1
             WHERE length(z.a || z1.a) < 5))
SELECT * FROM x;

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.

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. We only need to make this check if
we're somewhere (possibly nested) inside a recursive query, but I don't
know how practical it will be to test that condition at the point we do
inlining; doing it unconditionally will (I think) be harmless because
self_reference will not be set.

-- 
Andrew (irc:RhodiumToad)

Andrew Gierth <andrew@tao11.riddles.org.uk> 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
> 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 of "ugh" too: it sounds expensive, and doing it in a way
that doesn't produce false positives would be even more complicated.

Idle uncaffeinated speculation: is it practical to fix the restriction
about multiple worktable scans?

Also, I thought of a somewhat-related scenario that the code isn't
accounting for: you can break the restrictions about single evaluation
with nested WITHs, like

with x as not materialized (with y as materialized (select random() r) select * from y)
select * from x, x x1;

In this particular example, we're saved from computing random() twice
by the checks for volatile functions.  But without that, y is inlined
and computed twice, e.g.

explain verbose with x as not materialized (with y as (select now() r) select * from y)
select * from x, x x1;
                   QUERY PLAN
------------------------------------------------
 Nested Loop  (cost=0.00..0.06 rows=1 width=16)
   Output: (now()), (now())
   ->  Result  (cost=0.00..0.01 rows=1 width=8)
         Output: now()
   ->  Result  (cost=0.00..0.01 rows=1 width=8)
         Output: now()
(6 rows)

As a user I think I'd find that surprising, and bad if y were expensive.

Is it practical to inline the outer "x" level and still compute "y"
only once?  If not, I think we need to disallow inlining anything
that contains a "with".

            regards, tom lane

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 user explicitly wrote NOT MATERIALIZED, then we should
assume they mean it.

-- 
Andrew (irc:RhodiumToad)

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 user explicitly wrote NOT MATERIALIZED, then we should
> assume they mean it.

Ah, but the example I gave also had MATERIALIZED on the inner WITH.
Why should the user not also mean that?

            regards, tom lane

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> 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 user not also mean that?

The inner WITH does get materialized, it just gets materialized twice.
If the user doesn't want that, then they can avoid using NOT MATERIALIZED
on the outer CTE; but if we force it to materialize the outer query,
then that leaves the user without recourse.

Consider a case like:

create view foo as
  with s as materialized (select something)
  select * from large l
  where l.foo in (select * from s) or l.bar in (select * from s);

with
  bar as not materialized (select * from foo)
select * from bar b1, bar b2 where b1.col='x' and b2.col='y';

In a case like this, materializing "s" twice may be far less expensive
than materializing the result of "select * from large..." without
benefit of pushed-down quals.

-- 
Andrew (irc:RhodiumToad)

> On 2018-08-08 16:55:22 +1200, Thomas Munro wrote:
>> On Fri, Jul 27, 2018 at 8:10 PM, David Fetter <david@fetter.org> 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 <david@fetter.org> 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 patch that does.
>> >
>> > It doesn't always pass make installcheck-world, but I need to sleep
>> > rather than investigate that at the moment.
>> 
>> One observation I wanted to share: CTE scans inhibit parallelism today
>> (something we might eventually want to fix with shared tuplestores).
>> This patch therefore allows parallelism in some WITH queries, which
>> seems like a very valuable thing.
> 
> Might be interesting to see how big a difference it makes for
> TPC-DS. Currently the results are bad (as in many queries don't finish
> in a relevant time) because it uses CTEs so widely, and there's often
> predicates outside the CTE that could be pushed down.

Now that the patch was committed, I played with TPCS-DS and found at
least one of their queries gets speedup. Query 2 runs 2 times faster
than 11. In 12, it seems that CTE wscs is pushed down.

with wscs as
 (select sold_date_sk
        ,sales_price
  from (select ws_sold_date_sk sold_date_sk
              ,ws_ext_sales_price sales_price
        from web_sales 
        union all
        select cs_sold_date_sk sold_date_sk
              ,cs_ext_sales_price sales_price
        from catalog_sales) as s1),
 wswscs as 
 (select d_week_seq,
        sum(case when (d_day_name='Sunday') then sales_price else null end) sun_sales,
        sum(case when (d_day_name='Monday') then sales_price else null end) mon_sales,
        sum(case when (d_day_name='Tuesday') then sales_price else  null end) tue_sales,
        sum(case when (d_day_name='Wednesday') then sales_price else null end) wed_sales,
        sum(case when (d_day_name='Thursday') then sales_price else null end) thu_sales,
        sum(case when (d_day_name='Friday') then sales_price else null end) fri_sales,
        sum(case when (d_day_name='Saturday') then sales_price else null end) sat_sales
 from wscs
     ,date_dim
 where d_date_sk = sold_date_sk
 group by d_week_seq)
 select d_week_seq1
       ,round(sun_sales1/sun_sales2,2)
       ,round(mon_sales1/mon_sales2,2)
       ,round(tue_sales1/tue_sales2,2)
       ,round(wed_sales1/wed_sales2,2)
       ,round(thu_sales1/thu_sales2,2)
       ,round(fri_sales1/fri_sales2,2)
       ,round(sat_sales1/sat_sales2,2)
 from
 (select wswscs.d_week_seq d_week_seq1
        ,sun_sales sun_sales1
        ,mon_sales mon_sales1
        ,tue_sales tue_sales1
        ,wed_sales wed_sales1
        ,thu_sales thu_sales1
        ,fri_sales fri_sales1
        ,sat_sales sat_sales1
  from wswscs,date_dim 
  where date_dim.d_week_seq = wswscs.d_week_seq and
        d_year = 1998) y,
 (select wswscs.d_week_seq d_week_seq2
        ,sun_sales sun_sales2
        ,mon_sales mon_sales2
        ,tue_sales tue_sales2
        ,wed_sales wed_sales2
        ,thu_sales thu_sales2
        ,fri_sales fri_sales2
        ,sat_sales sat_sales2
  from wswscs
      ,date_dim 
  where date_dim.d_week_seq = wswscs.d_week_seq and
        d_year = 1998+1) z
 where d_week_seq1=d_week_seq2-53
 order by d_week_seq1;

Here's the 12's plan:
                                                                                  QUERY PLAN
                                                     
 

------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=118929.39..118929.43 rows=13 width=228) (actual time=792.588..792.710 rows=2513 loops=1)
   Sort Key: wswscs.d_week_seq
   Sort Method: quicksort  Memory: 323kB
   CTE wswscs
     ->  Finalize GroupAggregate  (cost=110164.09..113672.71 rows=10447 width=228) (actual time=766.232..768.415
rows=263loops=1)
 
           Group Key: date_dim_2.d_week_seq
           ->  Gather Merge  (cost=110164.09..112601.89 rows=20894 width=228) (actual time=766.209..767.158 rows=789
loops=1)
                 Workers Planned: 2
                 Workers Launched: 2
                 ->  Sort  (cost=109164.06..109190.18 rows=10447 width=228) (actual time=763.059..763.078 rows=263
loops=3)
                       Sort Key: date_dim_2.d_week_seq
                       Sort Method: quicksort  Memory: 160kB
                       Worker 0:  Sort Method: quicksort  Memory: 160kB
                       Worker 1:  Sort Method: quicksort  Memory: 160kB
                       ->  Partial HashAggregate  (cost=108179.39..108466.69 rows=10447 width=228) (actual
time=762.202..762.889rows=263 loops=3)
 
                             Group Key: date_dim_2.d_week_seq
                             ->  Parallel Hash Join  (cost=2371.82..74413.79 rows=900416 width=20) (actual
time=17.166..424.834rows=717854 loops=3)
 
                                   Hash Cond: (catalog_sales.cs_sold_date_sk = date_dim_2.d_date_sk)
                                   ->  Parallel Append  (cost=0.00..69678.24 rows=900416 width=10) (actual
time=0.029..248.992rows=720311 loops=3)
 
                                         ->  Parallel Seq Scan on catalog_sales  (cost=0.00..43411.73 rows=600673
width=10)(actual time=0.018..130.163 rows=480516 loops=3)
 
                                         ->  Parallel Seq Scan on web_sales  (cost=0.00..21764.43 rows=299743 width=10)
(actualtime=0.026..95.629 rows=359692 loops=2)
 
                                   ->  Parallel Hash  (cost=1834.70..1834.70 rows=42970 width=18) (actual
time=16.610..16.610rows=24350 loops=3)
 
                                         Buckets: 131072  Batches: 1  Memory Usage: 5056kB
                                         ->  Parallel Seq Scan on date_dim date_dim_2  (cost=0.00..1834.70 rows=42970
width=18)(actual time=0.020..7.617 rows=24350 loops=3)
 
   ->  Hash Join  (cost=5007.74..5256.44 rows=13 width=228) (actual time=785.300..792.123 rows=2513 loops=1)
         Hash Cond: (wswscs.d_week_seq = date_dim.d_week_seq)
         ->  CTE Scan on wswscs  (cost=0.00..208.94 rows=10447 width=228) (actual time=766.236..766.263 rows=263
loops=1)
         ->  Hash  (cost=5007.58..5007.58 rows=13 width=232) (actual time=19.033..19.033 rows=2513 loops=1)
               Buckets: 4096 (originally 1024)  Batches: 1 (originally 1)  Memory Usage: 288kB
               ->  Merge Join  (cost=5001.97..5007.58 rows=13 width=232) (actual time=17.739..18.210 rows=2513
loops=1)
                     Merge Cond: (((wswscs_1.d_week_seq - 53)) = date_dim.d_week_seq)
                     ->  Sort  (cost=2668.33..2669.24 rows=365 width=228) (actual time=9.906..9.924 rows=365 loops=1)
                           Sort Key: ((wswscs_1.d_week_seq - 53))
                           Sort Method: quicksort  Memory: 76kB
                           ->  Hash Join  (cost=2322.68..2652.79 rows=365 width=228) (actual time=7.864..9.764 rows=365
loops=1)
                                 Hash Cond: (wswscs_1.d_week_seq = date_dim_1.d_week_seq)
                                 ->  CTE Scan on wswscs wswscs_1  (cost=0.00..208.94 rows=10447 width=228) (actual
time=0.001..2.287rows=263 loops=1)
 
                                 ->  Hash  (cost=2318.11..2318.11 rows=365 width=4) (actual time=7.389..7.389 rows=365
loops=1)
                                       Buckets: 1024  Batches: 1  Memory Usage: 21kB
                                       ->  Seq Scan on date_dim date_dim_1  (cost=0.00..2318.11 rows=365 width=4)
(actualtime=3.876..7.348 rows=365 loops=1)
 
                                             Filter: (d_year = 1999)
                                             Rows Removed by Filter: 72684
                     ->  Sort  (cost=2333.65..2334.56 rows=365 width=4) (actual time=7.824..7.930 rows=2514 loops=1)
                           Sort Key: date_dim.d_week_seq
                           Sort Method: quicksort  Memory: 42kB
                           ->  Seq Scan on date_dim  (cost=0.00..2318.11 rows=365 width=4) (actual time=3.950..7.765
rows=365loops=1)
 
                                 Filter: (d_year = 1998)
                                 Rows Removed by Filter: 72684
 Planning Time: 0.956 ms
 Execution Time: 794.072 ms
(50 rows)

Here's 11's plan:
                                                                          QUERY PLAN
                                     
 

--------------------------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=227105.32..227105.35 rows=13 width=228) (actual time=2019.071..2019.163 rows=2513 loops=1)
   Sort Key: wswscs.d_week_seq
   Sort Method: quicksort  Memory: 323kB
   CTE wscs
     ->  Append  (cost=0.00..88586.64 rows=2160976 width=10) (actual time=0.015..521.937 rows=2160932 loops=1)
           ->  Seq Scan on web_sales  (cost=0.00..25960.84 rows=719384 width=10) (actual time=0.014..137.557
rows=719384loops=1)
 
           ->  Seq Scan on catalog_sales  (cost=0.00..51820.92 rows=1441592 width=10) (actual time=0.011..269.559
rows=1441548loops=1)
 
   CTE wswscs
     ->  HashAggregate  (cost=132977.62..133264.03 rows=10415 width=228) (actual time=1996.856..1997.387 rows=263
loops=1)
           Group Key: date_dim_2.d_week_seq
           ->  Hash Join  (cost=3048.60..51941.02 rows=2160976 width=28) (actual time=36.414..1323.387 rows=2153563
loops=1)
                 Hash Cond: (wscs.sold_date_sk = date_dim_2.d_date_sk)
                 ->  CTE Scan on wscs  (cost=0.00..43219.52 rows=2160976 width=18) (actual time=0.017..942.090
rows=2160932loops=1)
 
                 ->  Hash  (cost=2135.49..2135.49 rows=73049 width=18) (actual time=35.870..35.871 rows=73049 loops=1)
                       Buckets: 131072  Batches: 1  Memory Usage: 4734kB
                       ->  Seq Scan on date_dim date_dim_2  (cost=0.00..2135.49 rows=73049 width=18) (actual
time=0.011..17.749rows=73049 loops=1)
 
   ->  Hash Join  (cost=5006.47..5254.41 rows=13 width=228) (actual time=2012.260..2018.602 rows=2513 loops=1)
         Hash Cond: (wswscs.d_week_seq = date_dim.d_week_seq)
         ->  CTE Scan on wswscs  (cost=0.00..208.30 rows=10415 width=228) (actual time=1996.858..1996.876 rows=263
loops=1)
         ->  Hash  (cost=5006.31..5006.31 rows=13 width=232) (actual time=15.380..15.380 rows=2513 loops=1)
               Buckets: 4096 (originally 1024)  Batches: 1 (originally 1)  Memory Usage: 288kB
               ->  Merge Join  (cost=5000.73..5006.31 rows=13 width=232) (actual time=14.272..14.683 rows=2513
loops=1)
                     Merge Cond: (((wswscs_1.d_week_seq - 53)) = date_dim.d_week_seq)
                     ->  Sort  (cost=2667.18..2668.09 rows=363 width=228) (actual time=7.374..7.393 rows=365 loops=1)
                           Sort Key: ((wswscs_1.d_week_seq - 53))
                           Sort Method: quicksort  Memory: 76kB
                           ->  Hash Join  (cost=2322.65..2651.75 rows=363 width=228) (actual time=6.386..7.154 rows=365
loops=1)
                                 Hash Cond: (wswscs_1.d_week_seq = date_dim_1.d_week_seq)
                                 ->  CTE Scan on wswscs wswscs_1  (cost=0.00..208.30 rows=10415 width=228) (actual
time=0.000..0.707rows=263 loops=1)
 
                                 ->  Hash  (cost=2318.11..2318.11 rows=363 width=4) (actual time=6.367..6.367 rows=365
loops=1)
                                       Buckets: 1024  Batches: 1  Memory Usage: 21kB
                                       ->  Seq Scan on date_dim date_dim_1  (cost=0.00..2318.11 rows=363 width=4)
(actualtime=3.000..6.330 rows=365 loops=1)
 
                                             Filter: (d_year = 1999)
                                             Rows Removed by Filter: 72684
                     ->  Sort  (cost=2333.55..2334.45 rows=363 width=4) (actual time=6.890..6.975 rows=2514 loops=1)
                           Sort Key: date_dim.d_week_seq
                           Sort Method: quicksort  Memory: 42kB
                           ->  Seq Scan on date_dim  (cost=0.00..2318.11 rows=363 width=4) (actual time=3.832..6.841
rows=365loops=1)
 
                                 Filter: (d_year = 1998)
                                 Rows Removed by Filter: 72684
 Planning Time: 0.962 ms
 Execution Time: 2027.758 ms
(42 rows)

BTW, in my small TPC-DS environment (2GB), only two queries were not
finished within 30 minutes (query 4 and 11). My guess is these seem to
be suffered from statistic errors (I got this hint from Kaigai-san's
slide: https://www.slideshare.net/kaigai/tpcdspostgresql, especially
page 20. Unfortunately the slide is in Japanese).  And it leads to
choosing a bad plan: nested loop join. If I disable nested loop join,
these two queries finished quickly.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

I wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> 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 of "ugh" too: it sounds expensive, and doing it in a way
> that doesn't produce false positives would be even more complicated.

After further investigation, I concluded that that wasn't that awful,
so done that way.

I'm still not entirely convinced about the behavior for nested WITHs
with different materialization specifications, but that seems like
a separate topic.

            regards, tom lane