Date: Mon, 5 Jul 2021 11:45:05 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Nitin Jadhav <nitinjadhavpostgres@gmail.com>,
	Zhihong Yu <zyu@yugabyte.com>, pgsql-hackers@lists.postgresql.org
Subject: Re: Removed extra memory allocations from create_list_bounds
Message-ID: <20210705164505.GZ21248@telsasoft.com>
References: 
 <CAMm1aWYFTqEio3bURzZh47jveiHRwgQTiSDvBORczNEz2duZ1Q@mail.gmail.com>
 <20210516170029.GB373@telsasoft.com>
 <CALNJ-vS4g3QNmPe8WcgBmJQOzS0K_OAujW_1T0J2JXqpBQoBFw@mail.gmail.com>
 <CAMm1aWYr9=qPCTeNzVCEbF=Nr=RLSnt_vdCDTm9VjLS5L5JuOw@mail.gmail.com>
 <20210517150310.GG373@telsasoft.com>
 <CAMm1aWb0KSOCHexbaeit7gy_p4Uhs18EWyBdPyis4bfw4ids7A@mail.gmail.com>
 <CAApHDvpouaAv6P+UHbwZ1OL=z1sBcf5vuMJXGauhBZZMEAHb2w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAApHDvpouaAv6P+UHbwZ1OL=z1sBcf5vuMJXGauhBZZMEAHb2w@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210705164505.GZ21248%40telsasoft.com>
Precedence: bulk

On Tue, Jul 06, 2021 at 01:48:52AM +1200, David Rowley wrote:
> On Wed, 19 May 2021 at 05:28, Nitin Jadhav <nitinjadhavpostgres@gmail.com> wrote:
> > I have rebased all the patches on top of
> > 'v2_0001-removed_extra_mem_alloc_from_create_list_bounds.patch'.
> > Attaching all the patches here.
> 
> I had a look over these and I think what's being done here is fine.

Thanks for loooking.

0001 is missing a newline before create_list_bounds()

0003 is missing pfree(all_bounds), which I had as 0005.
It 1) allocates all_bounds; 2) allocates rbounds; 3) copies all_bounds into
rbounds; 4) allocates boundDatums; 5) copies rbounds into boundDatums; 6) frees
rbounds; 7) returns boundInfo with boundinfo->datums.

> The good news is, we can just give partition_bounds_copy() the same
> treatment. 0004 does that.

+1

> I've attached another set of patches. I squashed all the changes to
> each create_*_bounds function into a patch of their own. Perhaps 0002
> and 0003 which handle range and hash partitioning can be one patch
> since Justin seemed to write that one.  I kept 0001 separate since
> that's Nitin's patch plus Justin's extra parts. It seems easier to
> credit properly having the patches broken out like this.  I think it's
> excessive to break down 0001 into Nitin and Justin's individual parts.

If you wanted to further squish the patches together, I don't mind being a
co-author.

Cheers,
-- 
Justin