On Fri, Oct 23, 2020 at 2:04 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Seems to work, so I pushed it (after some compulsive fooling > about with whitespace and perltidy-ing). It appears to me that > the code coverage for verify_heapam.c is not very good though, > only circa 50%. Do we care to expend more effort on that?
There are two competing goods here. On the one hand, more test coverage is better than less. On the other hand, finicky tests that have platform-dependent results or fail for strange reasons not indicative of actual problems with the code are often judged not to be worth the trouble. An early version of this patch set had a very extensive chunk of Perl code in it that actually understood the page layout and, if we adopt something like that, it would probably be easier to test a whole bunch of scenarios. The downside is that it was a lot of code that basically duplicated a lot of backend logic in Perl, and I was (and am) afraid that people will complain about the amount of code and/or the difficulty of maintaining it. On the other hand, having all that code might allow better testing not only of this particular patch but also other scenarios involving corrupted pages, so maybe it's wrong to view all that code as a burden that we have to carry specifically to test this; or, alternatively, maybe it's worth carrying even if we only use it for this. On the third hand, as Mark points out, if we get 0002 committed, that will help somewhat with test coverage even if we do nothing else. Thanks for committing (and adjusting) the patches for the existing buildfarm failures. If I understand the buildfarm results correctly, hornet is still unhappy even after 321633e17b07968e68ca5341429e2c8bbf15c331? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
