Date: Tue, 23 Jul 2024 05:34:00 -0700
From: Noah Misch <noah@leadboat.com>
To: Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Use read streams in CREATE DATABASE command when the strategy is
 wal_log
Message-ID: <20240723123400.64@rfd.leadboat.com>
References: <20240711235209.f6.nmisch@google.com>
 <CAN55FZ0pHWZrLPwKGggnigJCj9Cp7fCXY8-e5yGNLzzp5kPvgg@mail.gmail.com>
 <20240716121935.fa.nmisch@google.com>
 <CAN55FZ33iZ3LmfqGhGH9YU8-b5SQisD_KJmmtFAEsUR0=XL-gg@mail.gmail.com>
 <20240717204147.09@rfd.leadboat.com>
 <CAN55FZ0TH_igXHFpVAicoDOhAn3S-JHutMDsa7JbrJx6HEgO3g@mail.gmail.com>
 <20240720112718.68.nmisch@google.com>
 <CAN55FZ1bOfGBszLqO_YF-wb0tCQa_PeHouf3Np0OA0N+Vy++eA@mail.gmail.com>
 <20240720181405.5a.nmisch@google.com>
 <CAN55FZ2+AnVKBimqwLSf2hvmUuU5a=WpfgQO=oGXUBrd9jkEKw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAN55FZ2+AnVKBimqwLSf2hvmUuU5a=WpfgQO=oGXUBrd9jkEKw@mail.gmail.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240723123400.64%40rfd.leadboat.com>
Precedence: bulk

On Mon, Jul 22, 2024 at 12:00:45PM +0300, Nazir Bilal Yavuz wrote:
> On Sat, 20 Jul 2024 at 21:14, Noah Misch <noah@leadboat.com> wrote:
> > On a different naming topic, my review missed that field name
> > copy_storage_using_buffer_read_stream_private.last_block doesn't fit how the
> > field is used.  Code uses it like an nblocks.  So let's either rename the
> > field or change the code to use it as a last_block (e.g. initialize it to
> > nblocks-1, not nblocks).
> 
> I prefered renaming it as nblocks, since that is how it was used in
> RelationCopyStorageUsingBuffer() before. Also, I realized that instead
> of setting p.blocknum = 0; initializing blkno as 0 and using
> p.blocknum = blkno makes sense. Because, p.blocknum and blkno should
> always start with the same block number. The relevant patch is
> attached.

I felt the local variable change was not a clear improvement.  It would have
been fine for the original patch to do it in that style, but the style of the
original patch was also fine.  So I've pushed just the struct field rename.