Re: TopoSort() fix

On Tue, Jul 30, 2019 at 1:36 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> No, there's a sufficient reason why we should force advisory locks
> to be taken in the leader process, namely that the behavior is totally
> different if we don't: they will disappear at the end of the parallel
> worker run, not at end of transaction or session as documented.

Oh, good point. I forgot about that.

> However, that argument doesn't seem to be a reason why the advisory-lock
> functions couldn't be parallel-restricted rather than parallel-unsafe.

Agreed.

> In any case, my question at the moment is whether we need the belt-and-
> suspenders-too approach of having both non-parallel-safe marking and an
> explicit check inside these functions. We've largely moved away from
> hard-wired checks for e.g. superuserness, and surely these things are
> less dangerous than most formerly-superuser-only functions.

If we can't think of a way that the lack of these checks could crash
it, then I think it's OK to remove the hardwired checks. If we can,
I'd favor keeping them.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company