Re: Small refactoring around vacuum_open_relation

On Fri, Aug 2, 2024 at 1:55 PM Kirill Reshke <reshkekirill@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.

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)

> 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.

Case3, worth changing to lockmode1 and lockmode2.

--
Best Wishes,
Ashutosh Bapat