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

On 30.06.23 20:48, Karina Litskevich wrote:
> So as for me calling LockRelationForExtension() and 
> UnlockRelationForExtension()
> without testing eb.rel first looks more like a bug here. However, they 
> are never
> actually called with eb.rel=NULL because of the EB_* flags, so there is 
> no bug
> here. I believe we should add Assert checking that when eb.rel is NULL, 
> flags
> are such that we won't use eb.rel. And yes, we can remove unnecessary checks
> where the flags value guaranty us that eb.rel is not NULL.
> 
> And another minor observation. It seems to me that we don't need a 
> "could have
> been closed while waiting for lock" in ExtendBufferedRelShared(), because I
> believe the comment above already explains why updating eb.smgr:
> 
>   * Note that another backend might have extended the relation by the time
>   * we get the lock.
> 
> I attached the new version of the patch as I see it.

This patch version looks the most sensible to me.  But as commented 
further downthread, some explanation around the added assertions would 
be good.