Remove should_free arguments to tuplesort routines.

Since commit e94568ecc10f2638e542ae34f2990b821bbf90ac, the answer is
always "false", and we do not need to complicate the API by arranging
to return a constant value.

Peter Geoghegan

Discussion: http://postgr.es/m/CAM3SWZQWZZ_N=DmmL7tKy_OUjGH_5mN=N=A6h7kHyyDvEhg2DA@mail.gmail.com

diff --git a/src/backend/access/hash/hashsort.c b/src/backend/access/hash/hashsort.c
index 8938ab5b2477a7008c2bb709474ef6d0a97504df..12fb78a5c0eb5cd23db9e2f4f874edecedb8e239 100644 (file)
--- a/src/backend/access/hash/hashsort.c
+++ b/src/backend/access/hash/hashsort.c
@@ -104,15 +104,13 @@ void
 _h_indexbuild(HSpool *hspool)
 {
        IndexTuple      itup;
-       bool            should_free;
 #ifdef USE_ASSERT_CHECKING
        uint32          hashkey = 0;
 #endif
 
        tuplesort_performsort(hspool->sortstate);
 
-       while ((itup = tuplesort_getindextuple(hspool->sortstate,
-                                                                                  true, &should_free)) != NULL)
+       while ((itup = tuplesort_getindextuple(hspool->sortstate, true)) != NULL)
        {
                /*
                 * Technically, it isn't critical that hash keys be found in sorted
@@ -129,7 +127,5 @@ _h_indexbuild(HSpool *hspool)
 #endif
 
                _hash_doinsert(hspool->index, itup);
-               if (should_free)
-                       pfree(itup);
        }
 }

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 99a014e8f47c77268c63e1ef7a4a1a01805d4729..7f65b5ad6f704a17b9190ce35deeff2577a5a87c 100644 (file)
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -680,9 +680,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
        bool            merge = (btspool2 != NULL);
        IndexTuple      itup,
                                itup2 = NULL;
-       bool            should_free,
-                               should_free2,
-                               load1;
+       bool            load1;
        TupleDesc       tupdes = RelationGetDescr(wstate->index);
        int                     i,
                                keysz = RelationGetNumberOfAttributes(wstate->index);
@@ -697,10 +695,8 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
                 */
 
                /* the preparation of merge */
-               itup = tuplesort_getindextuple(btspool->sortstate,
-                                                                          true, &should_free);
-               itup2 = tuplesort_getindextuple(btspool2->sortstate,
-                                                                               true, &should_free2);
+               itup = tuplesort_getindextuple(btspool->sortstate, true);
+               itup2 = tuplesort_getindextuple(btspool2->sortstate, true);
                indexScanKey = _bt_mkscankey_nodata(wstate->index);
 
                /* Prepare SortSupport data for each column */
@@ -775,18 +771,12 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
                        if (load1)
                        {
                                _bt_buildadd(wstate, state, itup);
-                               if (should_free)
-                                       pfree(itup);
-                               itup = tuplesort_getindextuple(btspool->sortstate,
-                                                                                          true, &should_free);
+                               itup = tuplesort_getindextuple(btspool->sortstate, true);
                        }
                        else
                        {
                                _bt_buildadd(wstate, state, itup2);
-                               if (should_free2)
-                                       pfree(itup2);
-                               itup2 = tuplesort_getindextuple(btspool2->sortstate,
-                                                                                               true, &should_free2);
+                               itup2 = tuplesort_getindextuple(btspool2->sortstate, true);
                        }
                }
                pfree(sortKeys);
@@ -795,15 +785,13 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
        {
                /* merge is unnecessary */
                while ((itup = tuplesort_getindextuple(btspool->sortstate,
-                                                                                          true, &should_free)) != NULL)
+                                                                                          true)) != NULL)
                {
                        /* When we see first tuple, create first index page */
                        if (state == NULL)
                                state = _bt_pagestate(wstate, 0);
 
                        _bt_buildadd(wstate, state, itup);
-                       if (should_free)
-                               pfree(itup);
                }
        }
 

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index dc1f79f5948965e8ed5363daab0ee85bad286d80..21312268a02fc309f38f5366878128d6fc168987 100644 (file)
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1057,11 +1057,10 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
                for (;;)
                {
                        HeapTuple       tuple;
-                       bool            shouldfree;
 
                        CHECK_FOR_INTERRUPTS();
 
-                       tuple = tuplesort_getheaptuple(tuplesort, true, &shouldfree);
+                       tuple = tuplesort_getheaptuple(tuplesort, true);
                        if (tuple == NULL)
                                break;
 
@@ -1069,9 +1068,6 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
                                                                         oldTupDesc, newTupDesc,
                                                                         values, isnull,
                                                                         NewHeap->rd_rel->relhasoids, rwstate);
-
-                       if (shouldfree)
-                               heap_freetuple(tuple);
                }
 
                tuplesort_end(tuplesort);

diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index baf87b3cdfc9c4bb15de24305e806de828bde9d7..71524c267f5fce1b3c49b57dbc812b8a22b17c80 100644 (file)
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -1841,12 +1841,12 @@ tuplesort_performsort(Tuplesortstate *state)
 /*
  * Internal routine to fetch the next tuple in either forward or back
  * direction into *stup.  Returns FALSE if no more tuples.
- * If *should_free is set, the caller must pfree stup.tuple when done with it.
- * Otherwise, caller should not use tuple following next call here.
+ * Returned tuple belongs to tuplesort memory context, and must not be freed
+ * by caller.  Caller should not use tuple following next call here.
  */
 static bool
 tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
-                                                 SortTuple *stup, bool *should_free)
+                                                 SortTuple *stup)
 {
        unsigned int tuplen;
 
@@ -1855,7 +1855,6 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
                case TSS_SORTEDINMEM:
                        Assert(forward || state->randomAccess);
                        Assert(!state->slabAllocatorUsed);
-                       *should_free = false;
                        if (forward)
                        {
                                if (state->current < state->memtupcount)
@@ -1927,7 +1926,6 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
                                         */
                                        state->lastReturnedTuple = stup->tuple;
 
-                                       *should_free = false;
                                        return true;
                                }
                                else
@@ -2008,14 +2006,12 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
                         */
                        state->lastReturnedTuple = stup->tuple;
 
-                       *should_free = false;
                        return true;
 
                case TSS_FINALMERGE:
                        Assert(forward);
                        /* We are managing memory ourselves, with the slab allocator. */
                        Assert(state->slabAllocatorUsed);
-                       *should_free = false;
 
                        /*
                         * The slab slot holding the tuple that we returned in previous
@@ -2097,9 +2093,8 @@ tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
 {
        MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
        SortTuple       stup;
-       bool            should_free;
 
-       if (!tuplesort_gettuple_common(state, forward, &stup, &should_free))
+       if (!tuplesort_gettuple_common(state, forward, &stup))
                stup.tuple = NULL;
 
        MemoryContextSwitchTo(oldcontext);
@@ -2110,12 +2105,8 @@ tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
                if (state->sortKeys->abbrev_converter && abbrev)
                        *abbrev = stup.datum1;
 
-               if (!should_free)
-               {
-                       stup.tuple = heap_copy_minimal_tuple((MinimalTuple) stup.tuple);
-                       should_free = true;
-               }
-               ExecStoreMinimalTuple((MinimalTuple) stup.tuple, slot, should_free);
+               stup.tuple = heap_copy_minimal_tuple((MinimalTuple) stup.tuple);
+               ExecStoreMinimalTuple((MinimalTuple) stup.tuple, slot, true);
                return true;
        }
        else
@@ -2127,18 +2118,17 @@ tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
 
 /*
  * Fetch the next tuple in either forward or back direction.
- * Returns NULL if no more tuples.  If *should_free is set, the
- * caller must pfree the returned tuple when done with it.
- * If it is not set, caller should not use tuple following next
- * call here.
+ * Returns NULL if no more tuples.  Returned tuple belongs to tuplesort memory
+ * context, and must not be freed by caller.  Caller should not use tuple
+ * following next call here.
  */
 HeapTuple
-tuplesort_getheaptuple(Tuplesortstate *state, bool forward, bool *should_free)
+tuplesort_getheaptuple(Tuplesortstate *state, bool forward)
 {
        MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
        SortTuple       stup;
 
-       if (!tuplesort_gettuple_common(state, forward, &stup, should_free))
+       if (!tuplesort_gettuple_common(state, forward, &stup))
                stup.tuple = NULL;
 
        MemoryContextSwitchTo(oldcontext);
@@ -2148,19 +2138,17 @@ tuplesort_getheaptuple(Tuplesortstate *state, bool forward, bool *should_free)
 
 /*
  * Fetch the next index tuple in either forward or back direction.
- * Returns NULL if no more tuples.  If *should_free is set, the
- * caller must pfree the returned tuple when done with it.
- * If it is not set, caller should not use tuple following next
- * call here.
+ * Returns NULL if no more tuples.  Returned tuple belongs to tuplesort memory
+ * context, and must not be freed by caller.  Caller should not use tuple
+ * following next call here.
  */
 IndexTuple
-tuplesort_getindextuple(Tuplesortstate *state, bool forward,
-                                               bool *should_free)
+tuplesort_getindextuple(Tuplesortstate *state, bool forward)
 {
        MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
        SortTuple       stup;
 
-       if (!tuplesort_gettuple_common(state, forward, &stup, should_free))
+       if (!tuplesort_gettuple_common(state, forward, &stup))
                stup.tuple = NULL;
 
        MemoryContextSwitchTo(oldcontext);
@@ -2173,7 +2161,8 @@ tuplesort_getindextuple(Tuplesortstate *state, bool forward,
  * Returns FALSE if no more datums.
  *
  * If the Datum is pass-by-ref type, the returned value is freshly palloc'd
- * and is now owned by the caller.
+ * and is now owned by the caller (this differs from similar routines for
+ * other types of tuplesorts).
  *
  * Caller may optionally be passed back abbreviated value (on TRUE return
  * value) when abbreviation was used, which can be used to cheaply avoid
@@ -2188,9 +2177,8 @@ tuplesort_getdatum(Tuplesortstate *state, bool forward,
 {
        MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
        SortTuple       stup;
-       bool            should_free;
 
-       if (!tuplesort_gettuple_common(state, forward, &stup, &should_free))
+       if (!tuplesort_gettuple_common(state, forward, &stup))
        {
                MemoryContextSwitchTo(oldcontext);
                return false;
@@ -2208,11 +2196,7 @@ tuplesort_getdatum(Tuplesortstate *state, bool forward,
        else
        {
                /* use stup.tuple because stup.datum1 may be an abbreviation */
-
-               if (should_free)
-                       *val = PointerGetDatum(stup.tuple);
-               else
-                       *val = datumCopy(PointerGetDatum(stup.tuple), false, state->datumTypeLen);
+               *val = datumCopy(PointerGetDatum(stup.tuple), false, state->datumTypeLen);
                *isNull = false;
        }
 
@@ -2270,16 +2254,12 @@ tuplesort_skiptuples(Tuplesortstate *state, int64 ntuples, bool forward)
                        while (ntuples-- > 0)
                        {
                                SortTuple       stup;
-                               bool            should_free;
 
-                               if (!tuplesort_gettuple_common(state, forward,
-                                                                                          &stup, &should_free))
+                               if (!tuplesort_gettuple_common(state, forward, &stup))
                                {
                                        MemoryContextSwitchTo(oldcontext);
                                        return false;
                                }
-                               if (should_free && stup.tuple)
-                                       pfree(stup.tuple);
                                CHECK_FOR_INTERRUPTS();
                        }
                        MemoryContextSwitchTo(oldcontext);

diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h
index 5cecd6d1b8657fa1d03ca074b70d78ef0c6768b9..38af746c85b31b47b40b076bd7755b52230125c0 100644 (file)
--- a/src/include/utils/tuplesort.h
+++ b/src/include/utils/tuplesort.h
@@ -94,10 +94,8 @@ extern void tuplesort_performsort(Tuplesortstate *state);
 
 extern bool tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
                                           TupleTableSlot *slot, Datum *abbrev);
-extern HeapTuple tuplesort_getheaptuple(Tuplesortstate *state, bool forward,
-                                          bool *should_free);
-extern IndexTuple tuplesort_getindextuple(Tuplesortstate *state, bool forward,
-                                               bool *should_free);
+extern HeapTuple tuplesort_getheaptuple(Tuplesortstate *state, bool forward);
+extern IndexTuple tuplesort_getindextuple(Tuplesortstate *state, bool forward);
 extern bool tuplesort_getdatum(Tuplesortstate *state, bool forward,
                                   Datum *val, bool *isNull, Datum *abbrev);