Re: [bug?] Missed parallel safety checks, and wrong parallel safety

On Tue, Jun 15, 2021 at 10:41 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > I don't think that finding the relation involved and registering an
> > invalidation for the same will work properly. Suppose there is a
> > concurrently-running transaction which has created a new table and
> > attached a trigger function to it. You can't see any of the catalog
> > entries for that relation yet, so you can't invalidate it, but
> > invalidation needs to happen. Even if you used some snapshot that can
> > see those catalog entries before they are committed, I doubt it fixes
> > the race condition. You can't hold any lock on that relation, because
> > the creating transaction holds AccessExclusiveLock, but the whole
> > invalidation mechanism is built around the assumption that the sender
> > puts messages into the shared queue first and then releases locks,
> > while the receiver first acquires a conflicting lock and then
> > processes messages from the queue.
>
> Won't such messages be proceesed at start transaction time
> (AtStart_Cache->AcceptInvalidationMessages)?

Only if they show up in the queue before that. But there's nothing
forcing that to happen. You don't seem to understand how important
heavyweight locking is to the whole shared invalidation message
system....

> Okay, so, in this scheme, we have allowed changing the function
> definition during statement execution but even though the rel's
> parallel-safe property gets modified (say to parallel-unsafe), we will
> still proceed in parallel-mode as if it's not changed. I guess this
> may not be a big deal as we can anyway allow breaking the running
> statement by changing its definition and users may be okay if the
> parallel statement errors out or behave in an unpredictable way in
> such corner cases.

Yeah, I mean, it's no different than leaving the parallel-safety
marking exactly as it was and changing the body of the function to
call some other function marked parallel-unsafe. I don't think we've
gotten any complaints about that, because I don't think it would
normally have any really bad consequences; most likely you'd just get
an error saying that something-or-other isn't allowed in parallel
mode. If it does have bad consequences, then I guess we'll have to fix
it when we find out about it, but in the meantime there's no reason to
hold the parallel-safety flag to a stricter standard than the function
body.

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