Re: A spot of redundant initialization of brin memtuple
Richard Guo writes: > On Mon, Nov 22, 2021 at 12:52 PM Bharath Rupireddy < > bharath.rupireddyforpostg...@gmail.com> wrote: >> Thanks. The patch looks good to me. Let's add it to the commitfest to >> not lose track of it. > Done. Here it is: > https://commitfest.postgresql.org/36/3424/ Pushed, thanks for the patch. regards, tom lane
Re: A spot of redundant initialization of brin memtuple
On Mon, Nov 22, 2021 at 12:52 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > On Mon, Nov 22, 2021 at 8:53 AM Richard Guo > wrote: > > > > > > On Sat, Nov 20, 2021 at 12:23 AM Bharath Rupireddy < > bharath.rupireddyforpostg...@gmail.com> wrote: > >> > >> On Fri, Nov 19, 2021 at 1:13 PM Richard Guo > wrote: > >> > > >> > Happened to notice this when reading around the codes. The > BrinMemTuple > >> > would be initialized in brin_new_memtuple(), right after being > created. > >> > So we don't need to initialize it again outside. > >> > > >> > diff --git a/src/backend/access/brin/brin.c > b/src/backend/access/brin/brin.c > >> > index ccc9fa0959..67a277e1f9 100644 > >> > --- a/src/backend/access/brin/brin.c > >> > +++ b/src/backend/access/brin/brin.c > >> > @@ -1261,8 +1261,6 @@ initialize_brin_buildstate(Relation idxRel, > BrinRevmap *revmap, > >> > state->bs_bdesc = brin_build_desc(idxRel); > >> > state->bs_dtuple = brin_new_memtuple(state->bs_bdesc); > >> > > >> > - brin_memtuple_initialize(state->bs_dtuple, state->bs_bdesc); > >> > - > >> > return state; > >> > } > >> > >> Good catch. +1 for the change. Please submit a patch. > > > > > > Thanks for the review. Attached is the patch. > > Thanks. The patch looks good to me. Let's add it to the commitfest to > not lose track of it. > Done. Here it is: https://commitfest.postgresql.org/36/3424/ Thanks again for the review. Thanks Richard
Re: A spot of redundant initialization of brin memtuple
On Mon, Nov 22, 2021 at 8:53 AM Richard Guo wrote: > > > On Sat, Nov 20, 2021 at 12:23 AM Bharath Rupireddy > wrote: >> >> On Fri, Nov 19, 2021 at 1:13 PM Richard Guo wrote: >> > >> > Happened to notice this when reading around the codes. The BrinMemTuple >> > would be initialized in brin_new_memtuple(), right after being created. >> > So we don't need to initialize it again outside. >> > >> > diff --git a/src/backend/access/brin/brin.c >> > b/src/backend/access/brin/brin.c >> > index ccc9fa0959..67a277e1f9 100644 >> > --- a/src/backend/access/brin/brin.c >> > +++ b/src/backend/access/brin/brin.c >> > @@ -1261,8 +1261,6 @@ initialize_brin_buildstate(Relation idxRel, >> > BrinRevmap *revmap, >> > state->bs_bdesc = brin_build_desc(idxRel); >> > state->bs_dtuple = brin_new_memtuple(state->bs_bdesc); >> > >> > - brin_memtuple_initialize(state->bs_dtuple, state->bs_bdesc); >> > - >> > return state; >> > } >> >> Good catch. +1 for the change. Please submit a patch. > > > Thanks for the review. Attached is the patch. Thanks. The patch looks good to me. Let's add it to the commitfest to not lose track of it. Regards, Bharath Rupireddy.
Re: A spot of redundant initialization of brin memtuple
On Sat, Nov 20, 2021 at 12:23 AM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > On Fri, Nov 19, 2021 at 1:13 PM Richard Guo > wrote: > > > > Happened to notice this when reading around the codes. The BrinMemTuple > > would be initialized in brin_new_memtuple(), right after being created. > > So we don't need to initialize it again outside. > > > > diff --git a/src/backend/access/brin/brin.c > b/src/backend/access/brin/brin.c > > index ccc9fa0959..67a277e1f9 100644 > > --- a/src/backend/access/brin/brin.c > > +++ b/src/backend/access/brin/brin.c > > @@ -1261,8 +1261,6 @@ initialize_brin_buildstate(Relation idxRel, > BrinRevmap *revmap, > > state->bs_bdesc = brin_build_desc(idxRel); > > state->bs_dtuple = brin_new_memtuple(state->bs_bdesc); > > > > - brin_memtuple_initialize(state->bs_dtuple, state->bs_bdesc); > > - > > return state; > > } > > Good catch. +1 for the change. Please submit a patch. > Thanks for the review. Attached is the patch. Thanks Richard v1-0001-Remove-redundant-initialization-of-brin-memtuple.patch Description: Binary data
Re: A spot of redundant initialization of brin memtuple
On Fri, Nov 19, 2021 at 1:13 PM Richard Guo wrote: > > Happened to notice this when reading around the codes. The BrinMemTuple > would be initialized in brin_new_memtuple(), right after being created. > So we don't need to initialize it again outside. > > diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c > index ccc9fa0959..67a277e1f9 100644 > --- a/src/backend/access/brin/brin.c > +++ b/src/backend/access/brin/brin.c > @@ -1261,8 +1261,6 @@ initialize_brin_buildstate(Relation idxRel, BrinRevmap > *revmap, > state->bs_bdesc = brin_build_desc(idxRel); > state->bs_dtuple = brin_new_memtuple(state->bs_bdesc); > > - brin_memtuple_initialize(state->bs_dtuple, state->bs_bdesc); > - > return state; > } Good catch. +1 for the change. Please submit a patch. Regards, Bharath Rupireddy.
