Amit Langote <amitlangote09@gmail.com> writes:
> I've attached just the patch that we should move forward with, as
> Alvaro might agree.
I've looked at this briefly but don't like it very much, specifically
the business about retroactively adding an RTE and RTEPermissionInfo
into the view's replacement subquery. That seems expensive and bug-prone:
if you're going to do that you might as well just leave the OLD entry
in place, as the earlier patch did, because you're just reconstructing
that same state of the parsetree a bit later on.
Furthermore, if that's where we end up then I'm not really sure this is
worth doing at all. The idea driving this was that if we could get rid
of *both* OLD and NEW RTE entries then we'd not have O(N^2) behavior in
deep subquery pull-ups due to the rangetable getting longer with each one.
But getting it down from two extra entries to one extra entry isn't going
to fix that big-O problem. (The patch as presented still has O(N^2)
planning time for the nested-views example discussed earlier.)
Conceivably we could make it work by allowing RTE_SUBQUERY RTEs to
carry a relation OID and associated RTEPermissionInfo, so that when a
view's RTE_RELATION RTE is transmuted to an RTE_SUBQUERY RTE it still
carries the info needed to let us lock and permission-check the view.
That might be a bridge too far from the ugliness perspective ...
although certainly the business with OLD and/or NEW RTEs isn't very
pretty either.
Anyway, if you don't feel like tackling that then I'd go back to the
patch version that kept the OLD RTE. (Maybe we could rename that to
something else, though, such as "*VIEW*"?)
BTW, I don't entirely understand why this patch is passing regression
tests, because it's failed to deal with numerous places that have
hard-wired knowledge about these extra RTEs. Look for references to
PRS2_OLD_VARNO and PRS2_NEW_VARNO. I think the original rationale
for UpdateRangeTableOfViewParse was that we needed to keep the rtables
of ON SELECT rules looking similar to those of other types of rules.
Maybe we've cleaned up all the places that used to depend on that,
but I'm not convinced.
regards, tom lane