Thanks for review! On Fri, 2 Aug 2024 at 14:31, Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > On Fri, Aug 2, 2024 at 1:55 PM Kirill Reshke <reshkekir...@gmail.com> wrote: > > > > I hate to be "that guy", but there are many places in sources where we use > > LOCKMODE lockmode; variable and exactly one where we use LOCKMODE > > lmode: it is vacuum_open_relation function. > > There are more instances of LOCKMODE lmode; I spotted one in plancat.c as > well.
Nice catch! > Case1: > toast_get_valid_index(Oid toastoid, LOCKMODE lock) > toast_close_indexes(Relation *toastidxs, int num_indexes, LOCKMODE lock) > GetLockmodeName(LOCKMETHODID lockmethodid, LOCKMODE mode) > LOCKMODE mode = 0; > Case 2: > qualified variable names like > LOCKMODE heapLockmode; > LOCKMODE memlockmode; > LOCKMODE table_lockmode; > LOCKMODE parentLockmode; > LOCKMODE cmd_lockmode = AccessExclusiveLock; /* default for compiler */ > LOCK_PRINT(const char *where, const LOCK *lock, LOCKMODE type) > > case3: some that have two LOCKMODE instances like > DoLockModesConflict(LOCKMODE mode1, LOCKMODE mode2) Nice catch! > > Is it worth a patch? > > When I see a variable with name lockmode, I know it's of type > LOCKMODE. So changing the Case1 may be worth it. It's not a whole lot > of code churn as well. May be patch backbranches. > > Case2 we should leave as is since the variable name has lockmode in it. +1 > Case3, worth changing to lockmode1 and lockmode2. Agree > -- > Best Wishes, > Ashutosh Bapat Attached v2 patch with your suggestions addressed.
v2-0001-Rename-LOCKMODE-type-vairables-to-lockmode.patch
Description: Binary data
