Re: [bug?] Missed parallel safety checks, and wrong parallel safety - Mailing list pgsql-hackers

On Mon, Jun 21, 2021 at 12:56 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> I thought if we scan a system catalog using DirtySnapshot, then we
> should be able to find such a relation. But, if the system catalog is
> updated after our scan then surely we won't be able to see it and in
> that case, we won't be able to send invalidation. Now, say if the rel
> is not visible to us because of the snapshot we used or due to some
> race condition then we won't be able to send the invalidation but why
> we want to consider it worse than the case where we miss such
> invalidations (invalidations due to change of parallel-safe property)
> when the insertion into relation is in-progress.

A concurrent change is something quite different than a change that
happened some time in the past. We all know that DROP TABLE blocks if
it is run while the table is in use, and everybody considers that
acceptable, but if DROP TABLE were to block because the table was in
use at some previous time, everybody would complain, and rightly so.
The same principle applies here. It's not possible to react to a
change that happens in the middle of the query. Somebody could argue
that we ought to lock all the functions we're using against concurrent
changes so that attempts to change their properties block on a lock
rather than succeeding. But given that that's not how it works, we can
hardly go back in time and switch to a non-parallel plan after we've
already decided on a parallel one. On the other hand, we should be
able to notice a change that has *already completed* at the time we do
planning. I don't see how we can blame failure to do that on anything
other than bad coding.

> Yeah, the session in which we are doing Alter Function won't be able
> to lock it but it will wait for the AccessExclusiveLock on the rel to
> be released because it will also try to acquire it before sending
> invalidation.

I think users would not be very happy with such behavior. Users accept
that if they try to access a relation, they might end up needing to
wait for a lock on it, but what you are proposing here might make a
session block waiting for a lock on a relation which it never
attempted to access.

I think this whole line of attack is a complete dead-end. We can
invent new types of invalidations if we want, but they need to be sent
based on which objects actually got changed, not based on what we
think might be affected indirectly as a result of those changes. It's
reasonable to regard something like a trigger or constraint as a
property of the table because it is really a dependent object. It is
associated with precisely one table when it is created and the
association can never be changed. On the other hand, functions clearly
have their own existence. They can be created and dropped
independently of any table and the tables with which they are
associated can change at any time. In that kind of situation,
invalidating the table based on changes to the function is riddled
with problems which I am pretty convinced we're never going to be able
to solve. I'm not 100% sure what we ought to do here, but I'm pretty
sure that looking up the tables that happen to be associated with the
function in the session that is modifying the function is not it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com