On Wed, Mar 28, 2018 at 7:59 AM, Anastasia Lubennikova <a.lubennik...@postgrespro.ru> wrote: > Here is the new version of the patch set. > All patches are rebased to apply without conflicts. > > Besides, they contain following fixes: > - pg_dump bug is fixed > - index_truncate_tuple() now has 3rd argument new_indnatts. > - new tests for amcheck, dblink and subscription/t/001_rep_changes.pl > - info about opclass implementors and included columns is now in sgml doc
This only changes the arguments given to index_truncate_tuple(), which is a superficial change. It does not actually change anything about the on-disk representation, which is what I sought. Why is that a problem? I don't think it's very complicated. The patch needs a rebase, as Erik mentioned: 1 out of 19 hunks FAILED -- saving rejects to file src/backend/utils/cache/relcache.c.rej (Stripping trailing CRs from patch; use --binary to disable.) I also noticed that you still haven't done anything differently with this code in _bt_checksplitloc(), which I mentioned in April of last year: /* Account for all the old tuples */ leftfree = state->leftspace - olddataitemstoleft; rightfree = state->rightspace - (state->olddataitemstotal - olddataitemstoleft); /* * The first item on the right page becomes the high key of the left page; * therefore it counts against left space as well as right space. */ leftfree -= firstrightitemsz; /* account for the new item */ if (newitemonleft) leftfree -= (int) state->newitemsz; else rightfree -= (int) state->newitemsz; With an extreme enough case, this could result in a failure to find a split point. Or at least, if that isn't true then it's not clear why, and I think it needs to be explained. -- Peter Geoghegan