Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c) - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)
Date
Msg-id 20230614.100159.450192360046344001.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)  (Ranier Vilela <ranier.vf@gmail.com>)
Responses Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)
List pgsql-hackers
At Tue, 13 Jun 2023 08:21:20 -0300, Ranier Vilela <ranier.vf@gmail.com> wrote in
> Em ter., 13 de jun. de 2023 às 02:51, Gurjeet Singh <gurjeet@singh.im>
> escreveu:
>
> > Please see attached v2 of the patch; it includes both occurrences of
> > the spurious checks identified in this thread.
> >
> +1
> LGTM.


>          LockRelationForExtension(eb.rel, ExclusiveLock);
>
> -        /* could have been closed while waiting for lock */
> -        if (eb.rel)
> -            eb.smgr = RelationGetSmgr(eb.rel);
> +        eb.smgr = RelationGetSmgr(eb.rel);

(It seems to me) The removed comment does refer to smgr, so we could
consider keeping it.  There are places instances where the function
calls are accompanied by similar comments and others where they
aren't. However, personally, I inclined towards its removal. That's
because our policy is to call RelationGetSmgr() each time before using
smgr, and this is well documented in the function's comment.

If we decide to remove it, the preceding blank line seems to be a
separator from the previous function call. So, we might want to
consider removing that blank line, too.

Otherwise it LGTM.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center



pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Inconsistent results with libc sorting on Windows
Next
From: Michael Paquier
Date:
Subject: Re: [PATCH] Using named captures in Catalog::ParseHeader()