> On Mar 12, 2021, at 1:43 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> 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
This does nothing to change the verbiage from contrib/amcheck, but it should address the problems discussed here in pg_amcheck's regression tests.