Re: Surround CheckRelation[Oid]LockedByMe() with USE_ASSERT_CHECKING - Mailing list pgsql-hackers

Hi,

On Mon, Jul 01, 2024 at 12:35:34PM +0530, Bharath Rupireddy wrote:
> Hi,
> 
> On Mon, Jul 1, 2024 at 12:12 PM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > Hi hackers,
> >
> > While working on a rebase for [1] due to 0cecc908e97, I noticed that
> > CheckRelationLockedByMe() and CheckRelationOidLockedByMe() are used only in
> > assertions.
> >
> > I think it would make sense to declare / define those functions only for
> > assert enabled build: please find attached a tiny patch doing so.
> >
> > Thoughts?
> 
> If turning the CheckRelationXXXLocked() compile for non-assert builds,
> why not do the same for LWLockHeldByMe, LWLockAnyHeldByMe and
> LWLockHeldByMeInMode that are debug-only and being used in asserts?
> While it might reduce the compiled binary size a bit for release
> builds, we may have to be cautious about external or out of core
> modules using them.

Thanks for the feedback.

CheckRelationOidLockedByMe() is new (as it has been added in 0cecc908e97. While
its counterpart CheckRelationLockedByMe() has been added since a few years (2018)
in commit b04aeb0a053, I thought it would make sense to surround both of them.

While it's true that we could also surround LWLockHeldByMe() (added in e6cba71503f
, 2004 and signature change in ea9df812d85, 2014), LWLockAnyHeldByMe() (added in
eed959a457e, 2022) and LWLockHeldByMeInMode() (added in 016abf1fb83, 2016), I'm
not sure we should (due to their "age" and as you said we have to be cautious
about out of core modules / extensions that may use them).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com