Re: Early WIP/PoC for inlining CTEs

From: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andreas Karlsson <andreas(at)proxel(dot)se>, David Fetter <david(at)fetter(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Early WIP/PoC for inlining CTEs
Date: 2018-11-16 23:04:38
Message-ID: 87in0wu0v9.fsf@news-spur.riddles.org.uk
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>>>> "Tom" == Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-11-16 23:10:41 Re: BUG #15449: file_fdw using program cause exit code error when using LIMIT
Previous Message David Fetter 2018-11-16 22:41:07 Re: Early WIP/PoC for inlining CTEs