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

> On 20 Dec 2024, at 11:01, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
> On Wed, Dec 18, 2024 at 7:39 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>>
>>> On 18 Dec 2024, at 12:28, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

>> +   if (   $ENV{PG_TEST_EXTRA}
>> +       && $ENV{PG_TEST_EXTRA} =~ /\bregress_dump_test\b/)
>> Should this also test that $oldnode and $newnode have matching pg_version to
>> keep this from running in a cross-version upgrade test?  While it can be argued
>> that running this in a cross-version upgrade is breaking it and getting to keep
>> both pieces, it's also not ideal to run a resource intensive test we know will
>> fail.  (It can't be done at this exact callsite, just picked to illustrate.)
>
> You already wrote it in parenthesis. At the exact callsite $oldnode
> and $newnode can not be of different versions. In fact newnode is yet
> to be created at this point. But $oldnode has the same version as the
> server run from the code. In a cross-version upgrade this test will
> not be executed. I am confused as to what this comment is about.

Sure, it can't be checked until $newnode is created, but it seems like a cheap
test to ensure it's not executed as part of someones cross-version tests.

>> +  my $format_spec = substr($format, 0, 1);
>> This doesn't seem great for readability, how about storing the formats and
>> specfiers in an array of Perl hashes which can be iterated over with
>> descriptive names, like $format{'name'} and $format{'spec'}?
>
> Instead of an array of hashes, I used a single hash with format
> description as key and format spec as value. Hope that's acceptable.

LGTM.

--
Daniel Gustafsson