]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
btree source code cleanups:
authorBruce Momjian <bruce@momjian.us>
Wed, 21 Feb 2007 20:02:17 +0000 (20:02 +0000)
committerBruce Momjian <bruce@momjian.us>
Wed, 21 Feb 2007 20:02:17 +0000 (20:02 +0000)
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.

Heikki Linnakangas

src/backend/access/nbtree/nbtinsert.c
src/backend/storage/page/bufpage.c
src/include/storage/bufpage.h

index 98a46ab585791ecd579de55396834955553424a3..e229083b8a1b8f86c8f14fb8a3da3477af4d441a 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.151 2007/02/08 13:52:55 alvherre Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.152 2007/02/21 20:02:17 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -29,6 +29,10 @@ typedef struct
        int                     fillfactor;             /* needed when splitting rightmost page */
        bool            is_leaf;                /* T if splitting a leaf page */
        bool            is_rightmost;   /* T if splitting a rightmost page */
+       OffsetNumber newitemoff;        /* where the new item is to be inserted */
+       int                     leftspace;              /* space available for items on left page */
+       int                     rightspace;             /* space available for items on right page */
+       int                     olddataitemstotal; /* space taken by old items */
 
        bool            have_split;             /* found a valid split? */
 
@@ -57,9 +61,9 @@ static OffsetNumber _bt_findsplitloc(Relation rel, Page page,
                                 OffsetNumber newitemoff,
                                 Size newitemsz,
                                 bool *newitemonleft);
-static void _bt_checksplitloc(FindSplitData *state, OffsetNumber firstright,
-                                 int leftfree, int rightfree,
-                                 bool newitemonleft, Size firstrightitemsz);
+static void _bt_checksplitloc(FindSplitData *state, 
+                                 OffsetNumber firstoldonright, bool newitemonleft,
+                                 int dataitemstoleft, Size firstoldonrightsz);
 static void _bt_pgaddtup(Relation rel, Page page,
                         Size itemsize, IndexTuple itup,
                         OffsetNumber itup_off, const char *where);
@@ -1101,13 +1105,31 @@ _bt_findsplitloc(Relation rel,
        int                     leftspace,
                                rightspace,
                                goodenough,
-                               dataitemtotal,
-                               dataitemstoleft;
+                               olddataitemstotal,
+                               olddataitemstoleft;
+       bool            goodenoughfound;
 
        opaque = (BTPageOpaque) PageGetSpecialPointer(page);
 
        /* Passed-in newitemsz is MAXALIGNED but does not include line pointer */
        newitemsz += sizeof(ItemIdData);
+
+       /* Total free space available on a btree page, after fixed overhead */
+       leftspace = rightspace =
+               PageGetPageSize(page) - SizeOfPageHeaderData -
+               MAXALIGN(sizeof(BTPageOpaqueData));
+
+       /* The right page will have the same high key as the old page */
+       if (!P_RIGHTMOST(opaque))
+       {
+               itemid = PageGetItemId(page, P_HIKEY);
+               rightspace -= (int) (MAXALIGN(ItemIdGetLength(itemid)) +
+                                                        sizeof(ItemIdData));
+       }
+
+       /* Count up total space in data items without actually scanning 'em */
+       olddataitemstotal = rightspace - (int) PageGetExactFreeSpace(page);
+
        state.newitemsz = newitemsz;
        state.is_leaf = P_ISLEAF(opaque);
        state.is_rightmost = P_RIGHTMOST(opaque);
@@ -1120,11 +1142,10 @@ _bt_findsplitloc(Relation rel,
        state.newitemonleft = false;    /* these just to keep compiler quiet */
        state.firstright = 0;
        state.best_delta = 0;
-
-       /* Total free space available on a btree page, after fixed overhead */
-       leftspace = rightspace =
-               PageGetPageSize(page) - SizeOfPageHeaderData -
-               MAXALIGN(sizeof(BTPageOpaqueData));
+       state.leftspace = leftspace;
+       state.rightspace = rightspace;
+       state.olddataitemstotal = olddataitemstotal;
+       state.newitemoff = newitemoff;
 
        /*
         * Finding the best possible split would require checking all the possible
@@ -1137,74 +1158,60 @@ _bt_findsplitloc(Relation rel,
         */
        goodenough = leftspace / 16;
 
-       /* The right page will have the same high key as the old page */
-       if (!P_RIGHTMOST(opaque))
-       {
-               itemid = PageGetItemId(page, P_HIKEY);
-               rightspace -= (int) (MAXALIGN(ItemIdGetLength(itemid)) +
-                                                        sizeof(ItemIdData));
-       }
-
-       /* Count up total space in data items without actually scanning 'em */
-       dataitemtotal = rightspace - (int) PageGetFreeSpace(page);
-
        /*
         * Scan through the data items and calculate space usage for a split at
         * each possible position.
         */
-       dataitemstoleft = 0;
+       olddataitemstoleft = 0;
+       goodenoughfound = false;
        maxoff = PageGetMaxOffsetNumber(page);
-
+       
        for (offnum = P_FIRSTDATAKEY(opaque);
                 offnum <= maxoff;
                 offnum = OffsetNumberNext(offnum))
        {
                Size            itemsz;
-               int                     leftfree,
-                                       rightfree;
 
                itemid = PageGetItemId(page, offnum);
                itemsz = MAXALIGN(ItemIdGetLength(itemid)) + sizeof(ItemIdData);
 
-               /*
-                * We have to allow for the current item becoming the high key of the
-                * left page; therefore it counts against left space as well as right
-                * space.
-                */
-               leftfree = leftspace - dataitemstoleft - (int) itemsz;
-               rightfree = rightspace - (dataitemtotal - dataitemstoleft);
-
                /*
                 * Will the new item go to left or right of split?
                 */
                if (offnum > newitemoff)
-                       _bt_checksplitloc(&state, offnum, leftfree, rightfree,
-                                                         true, itemsz);
+                       _bt_checksplitloc(&state, offnum, true,
+                                                         olddataitemstoleft, itemsz);
+
                else if (offnum < newitemoff)
-                       _bt_checksplitloc(&state, offnum, leftfree, rightfree,
-                                                         false, itemsz);
+                       _bt_checksplitloc(&state, offnum, false, 
+                                                         olddataitemstoleft, itemsz);
                else
                {
                        /* need to try it both ways! */
-                       _bt_checksplitloc(&state, offnum, leftfree, rightfree,
-                                                         true, itemsz);
-                       /*
-                        * Here we are contemplating newitem as first on right.  In this
-                        * case it, not the current item, will become the high key of the
-                        * left page, and so we have to correct the allowance made above.
-                        */
-                       leftfree += (int) itemsz - (int) newitemsz;
-                       _bt_checksplitloc(&state, offnum, leftfree, rightfree,
-                                                         false, newitemsz);
+                       _bt_checksplitloc(&state, offnum, true,
+                                                         olddataitemstoleft, itemsz);
+
+                       _bt_checksplitloc(&state, offnum, false,
+                                                         olddataitemstoleft, itemsz);
                }
 
                /* Abort scan once we find a good-enough choice */
                if (state.have_split && state.best_delta <= goodenough)
+               {
+                       goodenoughfound = true;
                        break;
+               }
 
-               dataitemstoleft += itemsz;
+               olddataitemstoleft += itemsz;
        }
 
+       /* If the new item goes as the last item, check for splitting so that
+        * all the old items go to the left page and the new item goes to the
+        * right page.
+        */
+       if (newitemoff > maxoff && !goodenoughfound)
+               _bt_checksplitloc(&state, newitemoff, false, olddataitemstotal, 0);
+
        /*
         * I believe it is not possible to fail to find a feasible split, but just
         * in case ...
@@ -1220,15 +1227,50 @@ _bt_findsplitloc(Relation rel,
 /*
  * Subroutine to analyze a particular possible split choice (ie, firstright
  * and newitemonleft settings), and record the best split so far in *state.
+ *
+ * firstoldonright is the offset of the first item on the original page
+ * that goes to the right page, and firstoldonrightsz is the size of that
+ * tuple. firstoldonright can be > max offset, which means that all the old
+ * items go to the left page and only the new item goes to the right page.
+ * In that case, firstoldonrightsz is not used.
+ *
+ * olddataitemstoleft is the total size of all old items to the left of 
+ * firstoldonright. 
  */
 static void
-_bt_checksplitloc(FindSplitData *state, OffsetNumber firstright,
-                                 int leftfree, int rightfree,
-                                 bool newitemonleft, Size firstrightitemsz)
+_bt_checksplitloc(FindSplitData *state, 
+                                 OffsetNumber firstoldonright,
+                                 bool newitemonleft,
+                                 int olddataitemstoleft,
+                                 Size firstoldonrightsz)
 {
+       int             leftfree,
+                       rightfree;
+       Size    firstrightitemsz;
+       bool    newitemisfirstonright;
+
+       /* Is the new item going to be the first item on the right page? */
+       newitemisfirstonright = (firstoldonright == state->newitemoff
+                                                        && !newitemonleft);
+
+       if(newitemisfirstonright)
+               firstrightitemsz = state->newitemsz;
+       else
+               firstrightitemsz = firstoldonrightsz;
+
+       /* Account for all the old tuples */
+       leftfree = state->leftspace - olddataitemstoleft;
+       rightfree = state->rightspace - 
+               (state->olddataitemstotal - olddataitemstoleft);
+
        /*
-        * Account for the new item on whichever side it is to be put.
+        * 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
@@ -1270,7 +1312,7 @@ _bt_checksplitloc(FindSplitData *state, OffsetNumber firstright,
                {
                        state->have_split = true;
                        state->newitemonleft = newitemonleft;
-                       state->firstright = firstright;
+                       state->firstright = firstoldonright;
                        state->best_delta = delta;
                }
        }
index 2e5c7c412bff264d6087abce6d8404320936e56e..8299f550d6e02303db13c289565832367003758e 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/storage/page/bufpage.c,v 1.70 2007/01/05 22:19:38 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/page/bufpage.c,v 1.71 2007/02/21 20:02:17 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -418,7 +418,8 @@ PageRepairFragmentation(Page page, OffsetNumber *unused)
 
 /*
  * PageGetFreeSpace
- *             Returns the size of the free (allocatable) space on a page.
+ *             Returns the size of the free (allocatable) space on a page,
+ *             deducted by the space needed for a new line pointer.
  */
 Size
 PageGetFreeSpace(Page page)
@@ -434,7 +435,26 @@ PageGetFreeSpace(Page page)
 
        if (space < (int) sizeof(ItemIdData))
                return 0;
-       space -= sizeof(ItemIdData);    /* XXX not always appropriate */
+       space -= sizeof(ItemIdData);
+
+       return (Size) space;
+}
+
+/*
+ * PageGetExactFreeSpace
+ *             Returns the size of the free (allocatable) space on a page.
+ */
+Size
+PageGetExactFreeSpace(Page page)
+{
+       int                     space;
+
+       /*
+        * Use signed arithmetic here so that we behave sensibly if pd_lower >
+        * pd_upper.
+        */
+       space = (int) ((PageHeader) page)->pd_upper -
+               (int) ((PageHeader) page)->pd_lower;
 
        return (Size) space;
 }
index abb51404a54855b71fb8bca100e06c3270e8c08f..44ab48c9b8f6d8daf33630067febcb3bf246c6bd 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/bufpage.h,v 1.70 2007/02/09 03:35:34 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/storage/bufpage.h,v 1.71 2007/02/21 20:02:17 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -322,6 +322,7 @@ extern Page PageGetTempPage(Page page, Size specialSize);
 extern void PageRestoreTempPage(Page tempPage, Page oldPage);
 extern int     PageRepairFragmentation(Page page, OffsetNumber *unused);
 extern Size PageGetFreeSpace(Page page);
+extern Size PageGetExactFreeSpace(Page page);
 extern void PageIndexTupleDelete(Page page, OffsetNumber offset);
 extern void PageIndexMultiDelete(Page page, OffsetNumber *itemnos, int nitems);