Re: pg_amcheck contrib application

> On Apr 23, 2021, at 10:28 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Thu, Apr 22, 2021 at 7:28 PM Mark Dilger
> <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>> I have refactored the patch to address your other concerns. Breaking the patch into multiple pieces didn't add any clarity, but refactoring portions of it made things simpler to read, I think, so here it is as one patch file.
>
> I was hoping that this version was going to be smaller than the last
> version, but instead it went from 300+ lines to 500+ lines.
>
> The main thing I'm unhappy about in the status quo is the use of
> chunkno in error messages. I have suggested several times making that
> concept go away, because I think users will be confused. Here's a
> minimal patch that does just that. It's 32 lines and results in a net
> removal of 4 lines. It differs somewhat from my earlier suggestions,
> because my priority here is to get reasonably understandable output
> without needing a ton of code, and as I was working on this I found
> that some of my earlier suggestions would have needed more code to
> implement and I didn't think it bought enough to be worth it. It's
> possible this is too simple, or that it's buggy, so let me know what
> you think. But basically, I think what got committed before is
> actually mostly fine and doesn't need major revision. It just needs
> tidying up to avoid the confusing chunkno concept.
>
> Now, the other thing we've talked about is adding a few more checks,
> to verify for example that the toastrelid is what we expect, and I see
> in your v22 you thought of a few other things. I think we can consider
> those, possibly as things where we consider it tidying up loose ends
> for v14, or else as improvements for v15. But I don't think that the
> fairly large size of your patch comes primarily from additional
> checks. I think it mostly comes from the code to produce error reports
> getting a lot more complicated. I apologize if my comments have driven
> that complexity, but they weren't intended to.
>
> One tiny problem with the attached patch is that it does not make any
> regression tests fail, which also makes it hard for me to tell if it
> breaks anything, or if the existing code works. I don't know how
> practical it is to do anything about that. Do you have a patch handy
> that allows manual updates and deletes on TOAST tables, for manual
> testing purposes?

Yes, I haven't been posting that with the patch because, but I will test your patch and see what differs.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company