Re: pg_amcheck contrib application

On Fri, Mar 12, 2021 at 2:31 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I'll commit something shortly to address these.

There are some interesting failures in the test cases on the
buildfarm. One of the tests ($offnum == 13) corrupts the TOAST pointer
with a garbage value, expecting to get the message "final toast chunk
number 0 differs from expected value 6". But on florican and maybe
other systems we instead get "final toast chunk number 0 differs from
expected value 5". That's because the value of TOAST_MAX_CHUNK_SIZE
depends on MAXIMUM_ALIGNOF. I think that on 4-byte alignment systems
it works out to 2000 and on 8-byte alignment systems it works out to
1996, and the value being stored is 10000 bytes, hence the problem.
The place where the calculation goes different seems to be in
MaximumBytesPerTuple(), where it uses MAXALIGN_DOWN() on a value that,
according to my calculations, will be 2038 on all platforms, but the
output of MAXALIGN_DOWN() will be 2032 or 2036 depending on the
platform. I think the solution to this is just to change the message
to match \d+ chunks instead of exactly 6. We should do that right away
to avoid having the buildfarm barf.

But, I also notice a couple of other things I think could be improved here:

1. amcheck is really reporting the complete absence of any TOAST rows
here due to a corrupted va_valueid. It could pick a better phrasing of
that message than "final toast chunk number 0 differs from expected
value XXX". I mean, there is no chunk 0. There are no chunks at all.

2. Using SSSSSSSSS as the perl unpack code for the varlena header is
not ideal, because it's really 2 1-byte fields followed by 4 4-byte
fields. So I think you should be using CCllLL, for two unsigned bytes
and then two signed 4-byte quantities and then two unsigned 4-byte
quantities. I think if you did that you'd be overwriting the
va_valueid with the *same* garbage value on every platform, which
would be better than different ones. Perhaps when we improve the
message as suggested in (1) this will become a live issue, since we
might choose to say something like "no TOAST entries for value %u".

--
Robert Haas
EDB: http://www.enterprisedb.com