Re: new heapcheck contrib module

> On Jun 30, 2020, at 11:44 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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