Thread
Commits
-
Remove redundant initialization of BrinMemTuple.
- 913a03ec29ab 15.0 landed
-
A spot of redundant initialization of brin memtuple
Richard Guo <guofenglinux@gmail.com> — 2021-11-19T07:43:14Z
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; } Thanks Richard -
Re: A spot of redundant initialization of brin memtuple
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-11-19T16:23:21Z
On Fri, Nov 19, 2021 at 1:13 PM Richard Guo <guofenglinux@gmail.com> 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.
-
Re: A spot of redundant initialization of brin memtuple
Richard Guo <guofenglinux@gmail.com> — 2021-11-22T03:23:42Z
On Sat, Nov 20, 2021 at 12:23 AM Bharath Rupireddy < bharath.rupireddyforpostgres@gmail.com> wrote: > On Fri, Nov 19, 2021 at 1:13 PM Richard Guo <guofenglinux@gmail.com> > 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
-
Re: A spot of redundant initialization of brin memtuple
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-11-22T04:52:44Z
On Mon, Nov 22, 2021 at 8:53 AM Richard Guo <guofenglinux@gmail.com> wrote: > > > On Sat, Nov 20, 2021 at 12:23 AM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: >> >> On Fri, Nov 19, 2021 at 1:13 PM Richard Guo <guofenglinux@gmail.com> 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
Richard Guo <guofenglinux@gmail.com> — 2021-11-22T06:34:56Z
On Mon, Nov 22, 2021 at 12:52 PM Bharath Rupireddy < bharath.rupireddyforpostgres@gmail.com> wrote: > On Mon, Nov 22, 2021 at 8:53 AM Richard Guo <guofenglinux@gmail.com> > wrote: > > > > > > On Sat, Nov 20, 2021 at 12:23 AM Bharath Rupireddy < > bharath.rupireddyforpostgres@gmail.com> wrote: > >> > >> On Fri, Nov 19, 2021 at 1:13 PM Richard Guo <guofenglinux@gmail.com> > 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
Tom Lane <tgl@sss.pgh.pa.us> — 2022-01-04T21:54:29Z
Richard Guo <guofenglinux@gmail.com> writes: > On Mon, Nov 22, 2021 at 12:52 PM Bharath Rupireddy < > bharath.rupireddyforpostgres@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