From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
cc: vignesh C <vignesh21@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: on placeholder entries in view rule action query's range table
In-reply-to: <2016650.1680719613@sss.pgh.pa.us>
References: <CA+HiwqEf7gPN4Hn+LoZ4tP2q_Qt7n3vw7-6fJKOf92tSEnX6Gg@mail.gmail.com> <20221208091245.k2bgz7one53dulef@alvherre.pgsql> <CA+HiwqGQQzBf9ORMUwTr+J+zE-JE=1hCthTqLs4uG_z5rTLGEg@mail.gmail.com> <CA+HiwqErh+VCA+5UoqTNPV4Xou7-LQJ-hv+GNkeDiX-pbiyYAQ@mail.gmail.com> <CALDaNm1O0mF5QGi0jy2b7sitZgYmNhjQZgb_wJ-EDV2qV=sebw@mail.gmail.com> <CA+HiwqEGSopmajB9wn+swwK=gh6_Y8i8OtMsD-g4cFZ4EURA2A@mail.gmail.com> <3684848.1673211536@sss.pgh.pa.us> <CA+HiwqFp=pOfKsMkZSG+H1E-a4Jnbi5uXYxynyfi_wkWWvTK4A@mail.gmail.com> <782120.1673485582@sss.pgh.pa.us> <CA+HiwqH0xZOQ+GQAdKeckY1R4NOeHdzhtfxkAMJLSchpapNk5w@mail.gmail.com> <2016650.1680719613@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Wed, 05 Apr 2023 14:33:33 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2060562.1680725102.1@sss.pgh.pa.us>
Date: Wed, 05 Apr 2023 16:05:02 -0400
Message-ID: <2060563.1680725102@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/2060563.1680725102%40sss.pgh.pa.us>
Precedence: bulk

I wrote:
> Amit Langote <amitlangote09@gmail.com> writes:
>> While thinking about query view locking in context of [1], I realized
>> that we have missed also fixing AcquirePlannerLocks() /
>> ScanQueryForLocks() to consider that an RTE_SUBQUERY rte may belong to
>> a view, which must be locked the same as RTE_RELATION entries.

> I think you're right about that, because AcquirePlannerLocks is supposed
> to reacquire whatever locks parsing+rewriting would have gotten.

After poking at this a bit more, I'm not sure there is any observable bug,
because we still notice the view change in AcquireExecutorLocks and
loop back to re-plan after that.  It still seems like a good idea to
notice such changes sooner not later to reduce wasted work, so I went
ahead and pushed the patch.

The only way it'd be a live bug is if the planner actually fails because
it's working with a stale view definition.  I tried to make it fail by
adjusting the view to no longer use an underlying table and then
dropping that table ... but AcquirePlannerLocks still detected that,
because of course it recurses and locks the table reference it finds
in the view subquery.  Maybe you could make a failure case involving
dropping a user-defined function instead, but I thought that was getting
pretty far afield, so I didn't pursue it.

			regards, tom lane