On Tue, May 20, 2025 at 5:12 PM Jeff Davis <pg...@j-davis.com> wrote:
> But relation_open() is sort of an exception. There are lots of places
> where that takes a lock because we happen to want something out of the
> relcache, like generate_partition_qual() taking a lock on the parent or
> CheckAttributeType() taking a lock on the typrelid. You could say those
> are fairly obvious, but that's because we already know, and we could
> make it more widely known that recording a dependency takes a lock.

To me, relation_open() feels like the kind of operation that I would
expect to take a lock. If I open something, I must have acquired some
resource on it that I will then use for a while before closing the
object.

> One compromise might be to have recordDependencyOn() take a LOCKMODE
> parameter, which would both inform the caller that a lock will be
> taken, and allow the caller to do it their own way and specify NoLock
> if necessary. That still results in a huge diff, but the end result
> would not be any more complex than the current code.

Yeah, that's not a terrible idea. I still like the idea I thought
Bertrand was pursuing, namely, to take no lock in recordDependencyOn()
but assert that the caller has previously acquired one. However, we
could still do the Assert() check with this design when NoLock is
passed, so I think this is a reasonable alternative to that design.

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


Reply via email to