Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump

Alvaro, Tom,

* Alvaro Herrera (alvherre(at)alvh(dot)no-ip(dot)org) wrote:
> Tom Lane wrote:
>
> > The changes in t/002_pg_dump.pl largely failed to apply, which is
> > partially due to the age of the patch but IMO speaks more to the
> > unmaintainability of that TAP test. It still didn't run after I'd
> > manually fixed the merge failures, so I gave up in disgust and
> > did not push any of the test changes. If someone is excited enough
> > about testing this, they can submit a followon patch for that,
> > but I judge it not really worth either the human effort or the
> > future testing cycles.
> >
> > (Am I right in thinking that 002_pg_dump.pl is, by design, roughly
> > O(N^2) in the number of features tested? Ick.)

It certainly tries to cover a lot of combinations, but I did try to
write it to not completely cross everything against everything but
instead to try and cover the different code paths.

> Yeah, that 002 test is pretty nasty stuff. I think we only put up with
> it because it's the only idea we've come up with; maybe there are better
> ideas.

I've discussed the 'better idea' for it previously, which is to
essentially break out the various parts of the big perl hash into
multilpe independent files that would be a lot easier to work with and
manage, and perhaps make them not be perl hashes but something else. In
other words, make the core perl code similar to pg_regress and the
input/output files likewise similar to how pg_regress works. Maybe we
should rework the whole thing to work with pg_regress directly but we'd
need to teach it about regexp's, I'd think..

> Crazy idea: maybe a large fraction of that test could be replaced with
> comparisons of the "pg_restore -l" output file rather than pg_dump's
> text output (i.e. the TOC entry for each object, rather than the
> object's textual representation.) Sounds easier in code than current
> implementation. Separately, verify that textual representation for each
> TOC entry type is what we expect.

I'm not sure how that's different..? We do check that the textual
representation is what we expect, both directly from pg_dump and from
pg_restore output, and using the exact same code to check both (which I
generally think is a good thing since we want the results from both to
more-or-less match up). What you're proposing here sounds like we're
throwing that away in favor of keeping all the same code to test the
textual representation but then only checking for listed contents from
pg_restore instead of checking that the textual representation is
correct. That doesn't actually reduce the amount of code though..

Thanks!

Stephen