Avoid copying index tuples when building an index.

The previous code, perhaps out of concern for avoid memory leaks, formed
the tuple in one memory context and then copied it to another memory
context.  However, this doesn't appear to be necessary, since
index_form_tuple and the functions it calls take precautions against
leaking memory.  In my testing, building the tuple directly inside the
sort context shaves several percent off the index build time.
Rearrange things so we do that.

Patch by me.  Review by Amit Kapila, Tom Lane, Andres Freund.

diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c
index 5fd400990b7e137ef59a10f7eb6507f50588911b..8d9a89303891fae171a4f081ab5af946daf7017e 100644 (file)
--- a/src/backend/access/common/indextuple.c
+++ b/src/backend/access/common/indextuple.c
@@ -28,6 +28,9 @@
 
 /* ----------------
  *             index_form_tuple
+ *
+ *             This shouldn't leak any memory; otherwise, callers such as
+ *             tuplesort_putindextuplevalues() will be very unhappy.
  * ----------------
  */
 IndexTuple

diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 7abb7a47fc22778ad2f1865e1abb843d8460874e..925a58f4f64544895eb7882ffd0e44b861f8bbde 100644 (file)
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -142,26 +142,23 @@ hashbuildCallback(Relation index,
        HashBuildState *buildstate = (HashBuildState *) state;
        IndexTuple      itup;
 
-       /* form an index tuple and point it at the heap tuple */
-       itup = _hash_form_tuple(index, values, isnull);
-       itup->t_tid = htup->t_self;
-
        /* Hash indexes don't index nulls, see notes in hashinsert */
-       if (IndexTupleHasNulls(itup))
-       {
-               pfree(itup);
+       if (isnull[0])
                return;
-       }
 
        /* Either spool the tuple for sorting, or just put it into the index */
        if (buildstate->spool)
-               _h_spool(itup, buildstate->spool);
+               _h_spool(buildstate->spool, &htup->t_self, values, isnull);
        else
+       {
+               /* form an index tuple and point it at the heap tuple */
+               itup = _hash_form_tuple(index, values, isnull);
+               itup->t_tid = htup->t_self;
                _hash_doinsert(index, itup);
+               pfree(itup);
+       }
 
        buildstate->indtuples += 1;
-
-       pfree(itup);
 }
 
 /*
@@ -184,10 +181,6 @@ hashinsert(PG_FUNCTION_ARGS)
 #endif
        IndexTuple      itup;
 
-       /* generate an index tuple */
-       itup = _hash_form_tuple(rel, values, isnull);
-       itup->t_tid = *ht_ctid;
-
        /*
         * If the single index key is null, we don't insert it into the index.
         * Hash tables support scans on '='. Relational algebra says that A = B
@@ -197,11 +190,12 @@ hashinsert(PG_FUNCTION_ARGS)
         * NOTNULL scans, but that's an artifact of the strategy map architecture
         * chosen in 1986, not of the way nulls are handled here.
         */
-       if (IndexTupleHasNulls(itup))
-       {
-               pfree(itup);
+       if (isnull[0])
                PG_RETURN_BOOL(false);
-       }
+
+       /* generate an index tuple */
+       itup = _hash_form_tuple(rel, values, isnull);
+       itup->t_tid = *ht_ctid;
 
        _hash_doinsert(rel, itup);
 

diff --git a/src/backend/access/hash/hashsort.c b/src/backend/access/hash/hashsort.c
index c0d6fec25678a8757cbef5b57de96a62fd9367fa..faf4fc24e69b04953f6853c699c02c0b48f8bc8e 100644 (file)
--- a/src/backend/access/hash/hashsort.c
+++ b/src/backend/access/hash/hashsort.c
@@ -90,9 +90,10 @@ _h_spooldestroy(HSpool *hspool)
  * spool an index entry into the sort file.
  */
 void
-_h_spool(IndexTuple itup, HSpool *hspool)
+_h_spool(HSpool *hspool, ItemPointer self, Datum *values, bool *isnull)
 {
-       tuplesort_putindextuple(hspool->sortstate, itup);
+       tuplesort_putindextuplevalues(hspool->sortstate, hspool->index,
+                                                                 self, values, isnull);
 }
 
 /*

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 36dc6c278ea671b87fd8de475e8c755096d6f924..89a98270798e14b95cfe8a8b8efc239030954791 100644 (file)
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -171,28 +171,21 @@ btbuildCallback(Relation index,
                                void *state)
 {
        BTBuildState *buildstate = (BTBuildState *) state;
-       IndexTuple      itup;
-
-       /* form an index tuple and point it at the heap tuple */
-       itup = index_form_tuple(RelationGetDescr(index), values, isnull);
-       itup->t_tid = htup->t_self;
 
        /*
         * insert the index tuple into the appropriate spool file for subsequent
         * processing
         */
        if (tupleIsAlive || buildstate->spool2 == NULL)
-               _bt_spool(itup, buildstate->spool);
+               _bt_spool(buildstate->spool, &htup->t_self, values, isnull);
        else
        {
                /* dead tuples are put into spool2 */
                buildstate->haveDead = true;
-               _bt_spool(itup, buildstate->spool2);
+               _bt_spool(buildstate->spool2, &htup->t_self, values, isnull);
        }
 
        buildstate->indtuples += 1;
-
-       pfree(itup);
 }
 
 /*

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 1281a120c560b01e1b61ee7dcc011b580a7b18d3..048b215118642837c76e8cc2d06b1b6cb6ebc52e 100644 (file)
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -185,9 +185,10 @@ _bt_spooldestroy(BTSpool *btspool)
  * spool an index entry into the sort file.
  */
 void
-_bt_spool(IndexTuple itup, BTSpool *btspool)
+_bt_spool(BTSpool *btspool, ItemPointer self, Datum *values, bool *isnull)
 {
-       tuplesort_putindextuple(btspool->sortstate, itup);
+       tuplesort_putindextuplevalues(btspool->sortstate, btspool->index,
+                                                                 self, values, isnull);
 }
 
 /*

diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index aa0f6d8e047a9c77384d773e8673cf6f39f0cf3d..426a64af8673e70a28196fa5773cef4f3c6daa1f 100644 (file)
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -1134,22 +1134,25 @@ tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup)
 }
 
 /*
- * Accept one index tuple while collecting input data for sort.
- *
- * Note that the input tuple is always copied; the caller need not save it.
+ * Collect one index tuple while collecting input data for sort, building
+ * it from caller-supplied values.
  */
 void
-tuplesort_putindextuple(Tuplesortstate *state, IndexTuple tuple)
+tuplesort_putindextuplevalues(Tuplesortstate *state, Relation rel,
+                                                         ItemPointer self, Datum *values,
+                              bool *isnull)
 {
        MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
        SortTuple       stup;
 
-       /*
-        * Copy the given tuple into memory we control, and decrease availMem.
-        * Then call the common code.
-        */
-       COPYTUP(state, &stup, (void *) tuple);
-
+       stup.tuple = index_form_tuple(RelationGetDescr(rel), values, isnull);
+       ((IndexTuple) stup.tuple)->t_tid = *self;
+       USEMEM(state, GetMemoryChunkSpace(stup.tuple));
+       /* set up first-column key value */
+       stup.datum1 = index_getattr((IndexTuple) stup.tuple,
+                                                               1,
+                                                               RelationGetDescr(state->indexRel),
+                                                               &stup.isnull1);
        puttuple_common(state, &stup);
 
        MemoryContextSwitchTo(oldcontext);

diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 2062801db5c3d18389b76ac8a626508fdbff2660..42a1d949a55dbca398390b6fc26c4883aa05289a 100644 (file)
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -336,7 +336,8 @@ typedef struct HSpool HSpool;       /* opaque struct in hashsort.c */
 
 extern HSpool *_h_spoolinit(Relation heap, Relation index, uint32 num_buckets);
 extern void _h_spooldestroy(HSpool *hspool);
-extern void _h_spool(IndexTuple itup, HSpool *hspool);
+extern void _h_spool(HSpool *hspool, ItemPointer self,
+                Datum *values, bool *isnull);
 extern void _h_indexbuild(HSpool *hspool);
 
 /* hashutil.c */

diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index ed6f697c8ede2f11a998204e90371b9cc9d10cda..9fa943f39e38a00f175efbf0e722f5d370167717 100644 (file)
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -717,7 +717,8 @@ typedef struct BTSpool BTSpool; /* opaque type known only within nbtsort.c */
 extern BTSpool *_bt_spoolinit(Relation heap, Relation index,
                          bool isunique, bool isdead);
 extern void _bt_spooldestroy(BTSpool *btspool);
-extern void _bt_spool(IndexTuple itup, BTSpool *btspool);
+extern void _bt_spool(BTSpool *btspool, ItemPointer self,
+                 Datum *values, bool *isnull);
 extern void _bt_leafbuild(BTSpool *btspool, BTSpool *spool2);
 
 /*

diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h
index 7d828e064bfa59131e9ad1c1233c5ded2fc7fe3e..253788322f13083ab4e82229b78ae44ec2c7d400 100644 (file)
--- a/src/include/utils/tuplesort.h
+++ b/src/include/utils/tuplesort.h
@@ -84,7 +84,9 @@ extern void tuplesort_set_bound(Tuplesortstate *state, int64 bound);
 extern void tuplesort_puttupleslot(Tuplesortstate *state,
                                           TupleTableSlot *slot);
 extern void tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup);
-extern void tuplesort_putindextuple(Tuplesortstate *state, IndexTuple tuple);
+extern void tuplesort_putindextuplevalues(Tuplesortstate *state,
+                                                         Relation rel, ItemPointer self,
+                                                         Datum *values, bool *isnull);
 extern void tuplesort_putdatum(Tuplesortstate *state, Datum val,
                                   bool isNull);