On Sun, Jan 16, 2011 at 12:07:44PM -0500, Tom Lane wrote: > Robert Haas <robertmh...@gmail.com> writes: > > On Sat, Jan 15, 2011 at 10:25 AM, Noah Misch <n...@leadboat.com> wrote: > >> Do you value test coverage so little? > > > If you're asking whether I think real-world usability is more > > important than test coverage, then yes. > > Quite honestly, I'd be inclined to rip out most of the DEBUG messages I > see in that regression test altogether. They are useless, and so is the > regression test itself. An appropriate regression test would involve > something more like checking that the relfilenode changed and then > checking that the contained data is still sane.
This patch is the first of a series. Divorced from the other patches, many of the test cases exercise the same code path, making them redundant. Even so, the tests revealed a defect we released with 9.0; that seems sufficient to promote them out of the "useless" bucket. One can easily confirm by inspection that the relfilenode will change if and only if the "rewriting" DEBUG message appears. Your proposed direct comparison of the relfilenode in the regression tests adds negligible sensitivity. If that were all, I'd call it a question of style. However, a relfilenode comparison does not distinguish no-op changes from changes entailing a verification scan. A similar ambiguity would arise without the "foreign key" DEBUG message. As for "checking that the contained data is still sane", what do you have in mind? After the test cases, I SELECT the table-under-test and choice catalog entries. If later patches in the series leave these expected outputs unchanged, that confirms the continued sanity of the data. Perhaps I should do this after every test, or also test forced index scans. nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers