On Fri, Jul 5, 2024 at 10:59 AM Michael Paquier <mich...@paquier.xyz> wrote:
> On Fri, Jun 28, 2024 at 06:00:07PM +0530, Ashutosh Bapat wrote: > > Here's a description of patches and some notes > > 0001 > > ------- > > 1. Per your suggestion the logic to handle dump output differences is > > externalized in PostgreSQL::Test::AdjustDump. Instead of eliminating > those > > differences altogether from both the dump outputs, the corresponding DDL > in > > the original dump output is adjusted to look like that from the restored > > database. Thus we retain full knowledge of what differences to expect. > > 2. I have changed the name filter_dump to filter_dump_for_upgrade so as > to > > differentiate between two adjustments 1. for upgrade and 2. for > > dump/restore. Ideally the name should have been > adjust_dump_for_ugprade() . > > It's more of an adjustment than filtering as indicated by the function it > > calls. But I haven't changed that. The new function to adjust dumps for > > dump and restore tests is named adjust_dump_for_restore() however. > > 3. As suggested by Daniel upthread, the test for dump and restore happens > > before upgrade which might change the old cluster thus changing the state > > of objects left behind by regression. The test is not executed if > > regression is not used to create the old cluster. > > 4. The code to compare two dumps and report differences if any is moved > to > > its own function compare_dumps() which is used for both upgrade and > > dump/restore tests. > > The test uses the custom dump format for dumping and restoring the > > database. > > At quick glance, that seems to be going in the right direction. Note > that you have forgotten install and uninstall rules for the new .pm > file. > Before submitting the patch, I looked for all the places which mention AdjustUpgrade or AdjustUpgrade.pm to find places where the new module needs to be mentioned. But I didn't find any. AdjustUpgrade is not mentioned in src/test/perl/Makefile or src/test/perl/meson.build. Do we want to also add AdjustUpgrade.pm in those files? > > 0002 increases more the runtime of a test that's already one of the > longest ones in the tree is not really appealing, I am afraid. > We could forget 0002. I am fine with that. But I can change the code such that formats other than "plain" are tested when PG_TEST_EXTRAS contains "regress_dump_formats". Would that be acceptable? -- Best Wishes, Ashutosh Bapat
