On Sun, 18 Apr 2021 at 21:42, Tom Lane <t...@sss.pgh.pa.us> wrote: > > > If multiple references are actually possible then this'd break it. > > I think this patch doesn't make things any worse for such a case though. > If we re-introduced such a bug, the result would be an immediate null > pointer crash while trying to process the second reference to a > flattenable subquery. That's probably better for debuggability than > what happens now, where we just silently process the duplicate reference. >
I took a look at this and wasn't able to find any way to break it, and your argument that it can't really make such rewriter bugs any worse makes sense. Would it make sense to update the comment prior to copying the subquery? Out of curiosity, I also tested DML against these deeply nested views to see how the pull-up code in the rewriter compares in terms of performance, since it does a very similar job. As expected, it's O(N^2) as well, but it's about an order of magnitude faster: (times to run a plain EXPLAIN in ms, with patch) | SELECT | INSERT | UPDATE | DELETE -----+--------+--------+--------+-------- v64 | 1.259 | 0.189 | 0.250 | 0.187 v128 | 5.035 | 0.506 | 0.547 | 0.509 v256 | 20.393 | 1.633 | 1.607 | 1.638 v512 | 81.101 | 6.649 | 6.517 | 6.699 Maybe that's not surprising, since there's less to do at that stage. Anyway, it's reassuring to know that it copes OK with this (I've seen some quite deeply nested views in practice, but never that deep). For comparison, this is what SELECT performance looked like for me without the patch: | SELECT -----+---------- v64 | 9.589 v128 | 73.292 v256 | 826.964 v512 | 7844.419 so, for a one-line change, that's pretty impressive. Regards, Dean