> On Mar 12, 2021, at 1:43 PM, Robert Haas <robertmh...@gmail.com> wrote: > > On Fri, Mar 12, 2021 at 2:31 PM Robert Haas <robertmh...@gmail.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.
v1-0001-Fixing-portability-issues-in-pg_amcheck-regressio.patch
Description: Binary data
— Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
