>>>>> "Tom" == Tom Lane <t...@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.
