Re: Test to dump and restore objects left behind by regression

On Wed, Feb 05, 2025 at 03:28:04PM +0900, Michael Paquier wrote:
> Hmm. I was reading through the patch and there is something that
> clearly stands out IMO: the new compare_dumps(). It is in Utils.pm,
> and it acts as a wrapper of `diff` with its formalized output format.
> It is not really about dumps, but about file comparisons. This should
> be renamed compare_files(), with internals adjusted as such, and
> reused in all the existing tests. Good idea to use that in
> 027_stream_regress.pl, actually. I'll go extract that first, to
> reduce the presence of `diff` in the whole set of TAP tests.

The result of this part is pretty neat, resulting in 0001 where it is
possible to use the refactored routine as well in pg_combinebackup
where there is a piece comparing dumps. There are three more tests
with diff commands and assumptions of their own, that I've left out.
This has the merit of unifying the output generated should any diffs
show up, while removing a nice chunk from the main patch.

> AdjustDump.pm looks like a fine concept as it stands. I still need to
> think more about it. It feels like we don't have the most optimal
> interface, though, but perhaps that will be clearer once
> compare_dumps() is moved out of the way.

+ my %dump_formats = ('plain' => 'p', 'tar' => 't', 'directory' => 'd', 'custom' => 'c');

No need for this mapping, let's just use the long options.

+ # restore data as well to catch any errors while doing so.
+ command_ok(
+ [
+ 'pg_dump', "-F$format_spec", '--no-sync',
+ '-d', $src_node->connstr('regression'),
+ '-f', $dump_file
+ ],
+ "pg_dump on source instance in $format format");

The use of command_ok() looks incorrect here. Shouldn't we use
$src_node->command_ok() here to ensure a correct PATH? That would be
more consistent with the other dump commands. Same remark about
@restore_command.

+ # The order of columns in COPY statements dumped from the original database
+ # and that from the restored database differs. These differences are hard to

What are the relations we are talking about here?

I am attaching the patch set, with 0002 being the main patch adjusted
with the changes of 0001 that I'm planning to apply, before diving
more into the internals of 0002.
--
Michael