From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
---|---|
To: | Karina Litskevich <litskevichkarina(at)gmail(dot)com>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
Cc: | Gurjeet Singh <gurjeet(at)singh(dot)im>, Richard Guo <guofenglinux(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, michael(at)paquier(dot)xyz, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c) |
Date: | 2023-09-04 12:32:56 |
Message-ID: | e96ba136-282d-995f-d956-453a029cf5d3@eisentraut.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Bapat | 2023-09-04 12:34:28 | Re: [17] CREATE SUBSCRIPTION ... SERVER |
Previous Message | Ashutosh Bapat | 2023-09-04 12:31:57 | Re: [17] CREATE SUBSCRIPTION ... SERVER |