From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Gierth <andrew@tao11.riddles.org.uk>
cc: Andreas Karlsson <andreas@proxel.se>,
        Tomas Vondra <tomas.vondra@2ndquadrant.com>,
        Merlin Moncure <mmoncure@gmail.com>,
        Alvaro Herrera <alvherre@2ndquadrant.com>,
        Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
        Michael Paquier <michael@paquier.xyz>,
        Robert Haas <robertmhaas@gmail.com>,
        Andres Freund <andres@anarazel.de>,
        Thomas Munro <thomas.munro@enterprisedb.com>,
        David Fetter <david@fetter.org>,
        Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Early WIP/PoC for inlining CTEs
In-reply-to: <87wolmg60q.fsf@news-spur.riddles.org.uk>
References: <c94d0ddb-d7f8-84be-46a2-8eb569561948@2ndquadrant.com>
 <20190214160209.GA13250@alvherre.pgsql>
 <CAHyXU0z6s6gWVYPCuRvNc633+EyG+Y2x_SDp=HhdObNe-U1NEQ@mail.gmail.com>
 <ff927085-883e-6a68-d2e0-e70b6971af4a@2ndquadrant.com>
 <31353.1550351689@sss.pgh.pa.us>
 <62080f95-3934-4700-00ca-7c35630ab7a0@proxel.se>
 <87wolmg60q.fsf@news-spur.riddles.org.uk>
Comments: In-reply-to Andrew Gierth <andrew@tao11.riddles.org.uk>
	message dated "Tue, 26 Feb 2019 11:41:51 +0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6695.1551192698.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Tue, 26 Feb 2019 09:51:38 -0500
Message-ID: <6696.1551192698@sss.pgh.pa.us>
Precedence: bulk

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) sel=
ect * 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) sel=
ect * from y)
select * from x, x x1;
                   QUERY PLAN                   =

------------------------------------------------
 Nested Loop  (cost=3D0.00..0.06 rows=3D1 width=3D16)
   Output: (now()), (now())
   ->  Result  (cost=3D0.00..0.01 rows=3D1 width=3D8)
         Output: now()
   ->  Result  (cost=3D0.00..0.01 rows=3D1 width=3D8)
         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