From: | Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: verify_heapam for sequences? |
Date: | 2021-08-26 19:02:31 |
Message-ID: | 65B8C20A-2F35-4573-8041-807E2462471F@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On Aug 26, 2021, at 3:03 AM, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>
>
> Is there a reason why contrib/amcheck/verify_heapam.c does not want to run on sequences? If I take out the checks, it appears to work. Is this an oversight? Or if there is a reason, maybe it could be stated in a comment, at least.
Testing the corruption checking logic on all platforms is a bit arduous, because the data layout on disk changes with alignment difference, endianness, etc. The work I did with Tom's help finally got good test coverage across the entire buildfarm, but that test (contrib/amcheck/t/001_verify_heapam.pl) doesn't work for sequences even on my one platform (mac laptop).
I have added a modicum of test coverage for sequences in the attached WIP patch, which is enough to detect sequence corruption on my laptop. It would have to be tested across the buildfarm after being extended to cover more cases. As it stands now, it uses blunt force to corrupt the relation, and only verifies that verify_heapam() returns some corruption, not that it reports the right corruption.
I understand that sequences are really just heap tables, and since we already test corrupted heap tables, we could assume that we already have sufficient coverage. I'm not entirely comfortable with that, though, because future patch authors who modify how tables or sequences work are not necessarily going to think carefully about whether their modifications invalidate that assumption.
Attachment | Content-Type | Size |
---|---|---|
v1-0001-WIP-patch-to-support-amcheck-of-sequences.patch.WIP | application/octet-stream | 5.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2021-08-26 19:42:09 | Re: Mark all GUC variable as PGDLLIMPORT |
Previous Message | Bruce Momjian | 2021-08-26 17:37:50 | Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce) |