Re: Accessing an invalid pointer in BufferManagerRelation structure

Stepan Neretin <slpmcf@gmail.com>

From: Stepan Neretin <slpmcf@gmail.com>

To: Daniil Davydov <3danissimo@gmail.com>

Cc: pgsql-hackers@postgresql.org

Date: 2025-05-10T17:36:27Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Make smgr access for a BufferManagerRelation safer in relcache inval
- b7cc6474e930 19 (unreleased) landed
Doc: use "an SQL" consistently rather than "a SQL"
- b51f86e49a7f 18.0 cited

On Mon, Apr 14, 2025 at 1:10 PM Daniil Davydov <3danissimo@gmail.com> wrote:

> 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
>

Hi, now looks good for me.
--
Best regards,
Stepan Neretin