Re: Small refactoring around vacuum_open_relation - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: Small refactoring around vacuum_open_relation
Date
Msg-id CAExHW5tKSqkGOYFiOZcSOG5YrKULAT20EFtDJv7m4bZDw4zUKw@mail.gmail.com
Whole thread Raw
In response to Small refactoring around vacuum_open_relation  (Kirill Reshke <reshkekirill@gmail.com>)
Responses Re: Small refactoring around vacuum_open_relation
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: "Andrey M. Borodin"
Date:
Subject: Re: [PATCH] Add min/max aggregate functions to BYTEA
Next
From: Richard Guo
Date:
Subject: Re: Wrong results with grouping sets