Re: [HACKERS] Dead code in _bt_split?

Tom Lane wrote:
> While testing it I realized that there seems to be a nearby bug in
> _bt_findsplitloc: it fails to consider the possibility of moving all the
> extant items to the left side. It will always return a firstright <=
> maxoff. ISTM this would mean that it could choose a bad split if the
> incoming item goes at the end and both it and the last extant item are
> large: in this case they should be split apart, but they won't be.
>
> Heikki, do you feel like looking at that, or shall I?

I refactored findsplitloc and checksplitloc so that the division of
labor is more clear IMO. I pushed all the space calculation inside the
loop to checksplitloc.

I also fixed the off by 4 in free space calculation caused by
PageGetFreeSpace subtracting sizeof(ItemIdData), even though it was
harmless, because it was distracting and I felt it might come back to
bite us in the future if we change the page layout or alignments.
There's now a new function PageGetExactFreeSpace that doesn't do the
subtraction.

findsplitloc now tries the "just the new item to right page" split as
well. If people don't like the refactoring, I can write a patch to just
add that.

Some of the long variable names look ugly. camelCase or underscores
between words would be more readable, but I retained the old style for
the sake of consistency.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com