On Mon, Apr 14, 2025 at 1:10 PM Daniil Davydov <3daniss...@gmail.com> wrote:
> Hi, > > On Wed, Mar 26, 2025 at 2:14 PM Stepan Neretin <slp...@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
