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