> On Jun 30, 2020, at 11:44 AM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
>
> I think there are two very large patches here. One adds checking of
> heapam tables to amcheck, and the other adds a binary that eases calling
> amcheck from the command line. I think these should be two separate
> patches.
contrib/amcheck has pretty limited regression test coverage. I wrote
pg_amcheck in large part because the infrastructure I was writing for testing
contrib/amcheck was starting to look like a stand-alone tool, so I made it one.
I can split contrib/pg_amcheck into a separate patch, but I would expect
reviewers to use it to review contrib/amcheck Say the word, and I'll resubmit
as two separate patches.
> I don't know what to think of a module contrib/pg_amcheck. I kinda lean
> towards fitting it in src/bin/scripts rather than as a contrib module.
> However, it seems a bit weird that it depends on a contrib module.
Agreed.
> Maybe amcheck should not be a contrib module at all but rather a new
> extension in src/extensions/ that is compiled and installed (in the
> filesystem, not in databases) by default.
Fine with me, but I'll have to see what others think about that.
> I strongly agree with hardening backend code so that all the crashes
> that Mark has found can be repaired. (We discussed this topic
> before[1]: we'd repair all crashes when run with production code, not
> all assertion crashes.)
I'm guessing that hardening the backend would be a separate patch? Or did you
want that as part of this one?
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
