Re: Test to dump and restore objects left behind by regression
| От | Ashutosh Bapat |
|---|---|
| Тема | Re: Test to dump and restore objects left behind by regression |
| Дата | |
| Msg-id | CAExHW5v1BkPZHnau-m5b4-GSyxw7GOvq9faBYXrJoNNTjPH+PQ@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Test to dump and restore objects left behind by regression (Michael Paquier <michael@paquier.xyz>) |
| Список | pgsql-hackers |
Sorry for delay, but here's next version of the patchset for review.
On Thu, Jun 6, 2024 at 5:07 AM Michael Paquier <michael@paquier.xyz> wrote:
On Wed, Jun 05, 2024 at 05:09:58PM +0530, Ashutosh Bapat wrote:
> Thanks for the suggestion. I didn't understand the dependency with the
> buildfarm module. Will the new module be used in buildfarm separately? I
> will work on this soon. Thanks for changing CF entry to WoA.
I had some doubts about PGBuild/Modules/TestUpgradeXversion.pm, but
after double-checking it loads dynamically AdjustUpgrade from the core
tree based on the base path where all the modules are:
# load helper module from source tree
unshift(@INC, "$srcdir/src/test/perl");
require PostgreSQL::Test::AdjustUpgrade;
PostgreSQL::Test::AdjustUpgrade->import;
shift(@INC);
It would be annoying to tweak the buildfarm code more to have a
different behavior depending on the branch of Postgres tested.
Anyway, from what I can see, you could create a new module with the
dump filtering rules that AdjustUpgrade requires without having to
update the buildfarm code.
The two filtering rules that I picked from AdjustUpgrade() are a. use unix style newline b. eliminate blank lines. I think we could copy those rule into the new module (as done in the patch) without creating any dependency between modules. There's little gained by creating another perl function just for those two sed commands. There's no way to do that otherwise. If we keep those two modules independent, we will be free to change each module as required in future. Do we need to change buildfarm code to load the AdjustDump module like above? I am not familiar with the buildfarm code.
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.
0002
------
This commit expands the previous test to test all dump formats. But as expected that increases the time taken by this test. On my laptop 0001 takes approx 28 seconds to run the test and with 0002 it takes approx 35 seconds. But there's not much impact on the duration of running all the tests (2m30s vs 2m40s). The code which creates the DDL statements in the dump is independent of the dump format. So usually we shouldn't require to test all the formats in this test. But each format stores the dependencies between dumped objects in a different manner which would be tested with the changes in this patch. I think this patch is also useful. If we decide to keep this test, the patch is intended to be merged into 0001.
Best Wishes,
Ashutosh Bapat
Вложения
В списке pgsql-hackers по дате отправления:
Следующее
От: Ashutosh BapatДата:
Сообщение: Re: pgindent exit status if a file encounters an error