On Mon, Mar 14, 2022 at 10:04 PM Robert Haas <robertmhaas@gmail.com> wrote:
> I think it would make sense to have two different WAL records e.g.
> XLOG_DBASE_CREATE_WAL_LOG and XLOG_DBASE_CREATE_FILE_COPY. Then it's
> easy to see how this could be generalized to other strategies in the
> future.
Done that way. In dbase_desc(), for XLOG_DBASE_CREATE_FILE_COPY I
have kept the older description i.e. "copy dir" and for
XLOG_DBASE_CREATE_WAL_LOG it is "create dir", because logically the
first one is actually copying and the second one is just creating the
directory. Do you think we should be using "copy dir file_copy" and
"copy dir wal_log" in the description as well?
> On Mon, Mar 14, 2022 at 12:04 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > I was looking at 0001 and 0002 again and realized that I swapped the
> > names load_relmap_file() and read_relmap_file() from what I should
> > have done. Here's a revised version. With this, read_relmap_file() and
> > write_relmap_file() become functions that just read and write the file
> > without touching any global variables, and load_relmap_file() is the
> > function that reads data from the file and puts it into a global
> > variable, which seems more sensible than the way I had it before.
Okay, I have included this patch and rebased other patches on top of that.
> Regarding 0003 and 0005, I'm not a fan of 'bool isunlogged'. I think
> 'bool permanent' would be better (note BM_PERMANENT). This would
> involve reversing true and false.
Okay changed.
> Regarding 0005:
>
> + CREATEDB_WAL_LOG = 0,
> + CREATEDB_FILE_COPY = 1
>
> I still think you don't need = 0 and = 1 here.
Done
> I'll probably go through and do a pass over the comments once you post
> the next version of this. There seems to be work needed in a bunch of
> places, but it probably makes more sense for me to go through and
> adjust the things that seem to need it rather than listing a bunch of
> changes for you to make.
Sure, thanks.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com