Re: WIP: Covering + unique indexes.

On Wed, Mar 28, 2018 at 7:59 AM, Anastasia Lubennikova
<a(dot)lubennikova(at)postgrespro(dot)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