Horton

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

Daniel Gustafsson <daniel@yesql.se>

View thread View on postgr.es
From: Daniel Gustafsson <daniel@yesql.se>
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: 2024-12-27T12:46:49Z
Lists: pgsql-hackers 
> 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

Commits

  1. Hide expensive pg_upgrade test behind PG_TEST_EXTRA

  2. Set log_statement=none in t/002_pg_upgrade.pl

  3. 002_pg_upgrade.pl: Move pg_dump test code for better stability

  4. 002_pg_upgrade.pl: rename some variables for clarity

  5. Verify roundtrip dump/restore of regression database

  6. Refactor TAP test code for file comparisons into new routine in Utils.pm

  7. Virtual generated columns

  8. Put generated_stored test objects in a schema

  9. Introduce "builtin" collation provider.

  10. Revert "Improve compression and storage support with inheritance"