Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

At Tue, 13 Jun 2023 08:21:20 -0300, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote in
> Em ter., 13 de jun. de 2023 às 02:51, Gurjeet Singh <gurjeet(at)singh(dot)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