On Sun, 18 Apr 2021 at 21:42, Tom Lane <tgl@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
