On Thu, Mar 10, 2022 at 10:18 PM Robert Haas <robertmh...@gmail.com> wrote: > > On Thu, Mar 10, 2022 at 6:02 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > I have completely changed the logic for this refactoring. Basically, > > write_relmap_file(), is already having parameters to control whether > > to write wal, send inval and we are already passing the dbpath. > > Instead of making a new function I just pass one additional parameter > > to this function itself about whether we are creating a new map or not > > and I think with that changes are very less and this looks cleaner to > > me. Similarly for load_relmap_file() also I just had to pass the > > dbpath and memory for destination map. Please have a look and let me > > know your thoughts. > > It's not terrible, but how about something like the attached instead? > I think this has the effect of reducing the number of cases that the > low-level code needs to know about from 2 to 1, instead of making it > go up from 2 to 3.
Yeah this looks cleaner, I will rebase the remaining patch. > > I think we should also write the test cases for create database > > strategy. But I do not see any test case for create database for > > testing the existing options. So I am wondering whether we should add > > the test case only for the new option we are providing or we should > > create a separate path which tests the new option as well as the > > existing options. > > FWIW, src/bin/scripts/t/020_createdb.pl does a little bit of testing > of this kind. Okay, I think we need to support the strategy in createdb bin as well. I will do that. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
