Re: Accessing an invalid pointer in BufferManagerRelation structure

Daniil Davydov <3danissimo@gmail.com>

From: Daniil Davydov <3danissimo@gmail.com>
To: Stepan Neretin <slpmcf@gmail.com>
Cc: pgsql-hackers@postgresql.org
Date: 2025-04-14T06:09:52Z
Lists: pgsql-hackers

Attachments

Hi,

On Wed, Mar 26, 2025 at 2:14 PM Stepan Neretin <slpmcf@gmail.com> wrote:
>
> The first thing we both noticed is that the macro calls a function that won't be available without an additional header. This seems a bit inconvenient.

Well, I rebased the patch onto the latest `master`
(b51f86e49a7f119004c0ce5d0be89cdf98309141) and noticed that we don't
need to include `rel.h` in `localbuf.c` directly anymore, because
`#include lmgr.h` was added in memutils.h
I guess it solves this issue. Please, see v3 patch.

> I also have a question: is the logic correct that if the relation is valid, we should fetch it rather than the other way around? Additionally, is checking only the `rd_isvalid` flag sufficient, or should we also consider the flag below?
>
> ```
> bool rd_isvalid; /* relcache entry is valid */
>

I don't think that we should check any Relation's flags here. We are
checking `RelationIsValid((bmr).rel) ?` to decide whether
BufferManagerRelation was created via BMR_REL or BMR_SMGR.
If the `rel` field is not NULL, we can definitely say that BMR_REL was
used, so we should call RelationGetSmgr in order to access smgr.

--
Best regards,
Daniil Davydov

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Make smgr access for a BufferManagerRelation safer in relcache inval

  2. Doc: use "an SQL" consistently rather than "a SQL"