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.