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

On Thu, Nov 14, 2024 at 4:16 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> > But see next
> >
> > >
> > > What's the advantage of testing all the formats?  Would that stuff
> > > have been able to catch up more issues related to specific format(s)
> > > when it came to the compression improvements with inheritance?
> >
> > I haven't caught any more issues with formats other than "plain". It
> > is more for future-proof testing. I am fine if we want to test just
> > plain dump format for now. Adding more formats would be easier if
> > required.
>
> Not done for now. Given that the 'directory' formats dumps the tables
> in separate directories, and thus has some impact on how child tables
> would be dumped and restored, I think we should at least have plain
> and directory tested in this test. But I will wait for other opinion
> before removing formats other than plain.

I gave this another thought. Looking at the documentation [1], each
format does something different that affects the way objects are
dumped and restored. Eliminating one or the other means we lose
corresponding coverage in dump or restore. So I have left this
untouched again.

>
> Interestingly, I have caught a new difference in dump from original
> and restored database. See the difference between attached plain dump
> files. I will start a new thread to see if this difference is
> legitimate. Had this test been part of core, we would have caught it
> earlier.
>
> Because of this difference, the test is failing. I will wait for the
> conclusion on the other thread before adding more adjustments.
>

The new test uncovered an issue related to NOT NULL constraints [2].
We have committed a fix for that bug. So far this test has unearthed
two bugs in committed changes in just one year. That proves the worth
of this test. There are many projects, in flight, which implement new
objects or new states of existing objects. I think this test will help
in all those projects.

I have rebased my patches on the current HEAD. The test now passes and
does not show any new diff or bug.

Squashed all the patches into one. While rebasing I found that
002_compare_backups has changed the way it compares dumps slightly. I
have left it outside of this patch right now.

>
> I am not against the other suggestions to make the functions, code
> added by this patch more general and extensible. But without an
> example or case for such generalization and/or extensibility, it's
> hard to get it right. And the functions and code are isolated enough
> that we could generalize and extend them if the need arises.

We can work on extending this further after the basic test is
committed. But if we delay committing the test for the extensibility
we might lose another bug.

[1] https://www.postgresql.org/docs/current/app-pgdump.html
[2] https://www.postgresql.org/message-id/CAExHW5tbdgAKDfqjDJ-7Fk6PJtHg8D4zUF6FQ4H2Pq8zK38Nyw@mail.gmail.com

-- 
Best Wishes,
Ashutosh Bapat