]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Remove Tuplesortstate.copytup function
authorAlexander Korotkov <akorotkov@postgresql.org>
Wed, 27 Jul 2022 05:26:53 +0000 (08:26 +0300)
committerAlexander Korotkov <akorotkov@postgresql.org>
Wed, 27 Jul 2022 05:26:53 +0000 (08:26 +0300)
It's currently unclear how do we split functionality between
Tuplesortstate.copytup() function and tuplesort_put*() functions.
For instance, copytup_index() and copytup_datum() return error while
tuplesort_putindextuplevalues() and tuplesort_putdatum() do their work.
This commit removes Tuplesortstate.copytup() altogether, putting the
corresponding code into tuplesort_put*().

Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent
Reviewed-by: Andres Freund, John Naylor
src/backend/utils/sort/tuplesort.c

index 421afcf47d33443de798419ef503cdff90e0a1c4..3e06e5391a6a6762ea79bee316a190e36c364892 100644 (file)
@@ -279,14 +279,6 @@ struct Tuplesortstate
         */
        SortTupleComparator comparetup;
 
-       /*
-        * Function to copy a supplied input tuple into palloc'd space and set up
-        * its SortTuple representation (ie, set tuple/datum1/isnull1).  Also,
-        * state->availMem must be decreased by the amount of space used for the
-        * tuple copy (note the SortTuple struct itself is not counted).
-        */
-       void            (*copytup) (Tuplesortstate *state, SortTuple *stup, void *tup);
-
        /*
         * Function to write a stored tuple onto tape.  The representation of the
         * tuple on tape need not be the same as it is in memory; requirements on
@@ -549,7 +541,6 @@ struct Sharedsort
        } while(0)
 
 #define COMPARETUP(state,a,b)  ((*(state)->comparetup) (a, b, state))
-#define COPYTUP(state,stup,tup) ((*(state)->copytup) (state, stup, tup))
 #define WRITETUP(state,tape,stup)      ((*(state)->writetup) (state, tape, stup))
 #define READTUP(state,stup,tape,len) ((*(state)->readtup) (state, stup, tape, len))
 #define LACKMEM(state)         ((state)->availMem < 0 && !(state)->slabAllocatorUsed)
@@ -600,10 +591,7 @@ struct Sharedsort
  * a lot better than what we were doing before 7.3.  As of 9.6, a
  * separate memory context is used for caller passed tuples.  Resetting
  * it at certain key increments significantly ameliorates fragmentation.
- * Note that this places a responsibility on copytup routines to use the
- * correct memory context for these tuples (and to not use the reset
- * context for anything whose lifetime needs to span multiple external
- * sort runs).  readtup routines use the slab allocator (they cannot use
+ * readtup routines use the slab allocator (they cannot use
  * the reset context because it gets deleted at the point that merging
  * begins).
  */
@@ -643,14 +631,12 @@ static void markrunend(LogicalTape *tape);
 static void *readtup_alloc(Tuplesortstate *state, Size tuplen);
 static int     comparetup_heap(const SortTuple *a, const SortTuple *b,
                                                        Tuplesortstate *state);
-static void copytup_heap(Tuplesortstate *state, SortTuple *stup, void *tup);
 static void writetup_heap(Tuplesortstate *state, LogicalTape *tape,
                                                  SortTuple *stup);
 static void readtup_heap(Tuplesortstate *state, SortTuple *stup,
                                                 LogicalTape *tape, unsigned int len);
 static int     comparetup_cluster(const SortTuple *a, const SortTuple *b,
                                                           Tuplesortstate *state);
-static void copytup_cluster(Tuplesortstate *state, SortTuple *stup, void *tup);
 static void writetup_cluster(Tuplesortstate *state, LogicalTape *tape,
                                                         SortTuple *stup);
 static void readtup_cluster(Tuplesortstate *state, SortTuple *stup,
@@ -659,14 +645,12 @@ static int        comparetup_index_btree(const SortTuple *a, const SortTuple *b,
                                                                   Tuplesortstate *state);
 static int     comparetup_index_hash(const SortTuple *a, const SortTuple *b,
                                                                  Tuplesortstate *state);
-static void copytup_index(Tuplesortstate *state, SortTuple *stup, void *tup);
 static void writetup_index(Tuplesortstate *state, LogicalTape *tape,
                                                   SortTuple *stup);
 static void readtup_index(Tuplesortstate *state, SortTuple *stup,
                                                  LogicalTape *tape, unsigned int len);
 static int     comparetup_datum(const SortTuple *a, const SortTuple *b,
                                                         Tuplesortstate *state);
-static void copytup_datum(Tuplesortstate *state, SortTuple *stup, void *tup);
 static void writetup_datum(Tuplesortstate *state, LogicalTape *tape,
                                                   SortTuple *stup);
 static void readtup_datum(Tuplesortstate *state, SortTuple *stup,
@@ -1059,7 +1043,6 @@ tuplesort_begin_heap(TupleDesc tupDesc,
                                                                PARALLEL_SORT(state));
 
        state->comparetup = comparetup_heap;
-       state->copytup = copytup_heap;
        state->writetup = writetup_heap;
        state->readtup = readtup_heap;
        state->haveDatum1 = true;
@@ -1135,7 +1118,6 @@ tuplesort_begin_cluster(TupleDesc tupDesc,
                                                                PARALLEL_SORT(state));
 
        state->comparetup = comparetup_cluster;
-       state->copytup = copytup_cluster;
        state->writetup = writetup_cluster;
        state->readtup = readtup_cluster;
        state->abbrevNext = 10;
@@ -1240,7 +1222,6 @@ tuplesort_begin_index_btree(Relation heapRel,
                                                                PARALLEL_SORT(state));
 
        state->comparetup = comparetup_index_btree;
-       state->copytup = copytup_index;
        state->writetup = writetup_index;
        state->readtup = readtup_index;
        state->abbrevNext = 10;
@@ -1317,7 +1298,6 @@ tuplesort_begin_index_hash(Relation heapRel,
        state->nKeys = 1;                       /* Only one sort column, the hash code */
 
        state->comparetup = comparetup_index_hash;
-       state->copytup = copytup_index;
        state->writetup = writetup_index;
        state->readtup = readtup_index;
        state->haveDatum1 = true;
@@ -1358,7 +1338,6 @@ tuplesort_begin_index_gist(Relation heapRel,
        state->nKeys = IndexRelationGetNumberOfKeyAttributes(indexRel);
 
        state->comparetup = comparetup_index_btree;
-       state->copytup = copytup_index;
        state->writetup = writetup_index;
        state->readtup = readtup_index;
        state->haveDatum1 = true;
@@ -1422,7 +1401,6 @@ tuplesort_begin_datum(Oid datumType, Oid sortOperator, Oid sortCollation,
                                                                PARALLEL_SORT(state));
 
        state->comparetup = comparetup_datum;
-       state->copytup = copytup_datum;
        state->writetup = writetup_datum;
        state->readtup = readtup_datum;
        state->abbrevNext = 10;
@@ -1839,14 +1817,75 @@ noalloc:
 void
 tuplesort_puttupleslot(Tuplesortstate *state, TupleTableSlot *slot)
 {
-       MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
+       MemoryContext oldcontext = MemoryContextSwitchTo(state->tuplecontext);
        SortTuple       stup;
+       Datum           original;
+       MinimalTuple tuple;
+       HeapTupleData htup;
 
-       /*
-        * Copy the given tuple into memory we control, and decrease availMem.
-        * Then call the common code.
-        */
-       COPYTUP(state, &stup, (void *) slot);
+       /* copy the tuple into sort storage */
+       tuple = ExecCopySlotMinimalTuple(slot);
+       stup.tuple = (void *) tuple;
+       USEMEM(state, GetMemoryChunkSpace(tuple));
+       /* set up first-column key value */
+       htup.t_len = tuple->t_len + MINIMAL_TUPLE_OFFSET;
+       htup.t_data = (HeapTupleHeader) ((char *) tuple - MINIMAL_TUPLE_OFFSET);
+       original = heap_getattr(&htup,
+                                                       state->sortKeys[0].ssup_attno,
+                                                       state->tupDesc,
+                                                       &stup.isnull1);
+
+       MemoryContextSwitchTo(state->sortcontext);
+
+       if (!state->sortKeys->abbrev_converter || stup.isnull1)
+       {
+               /*
+                * Store ordinary Datum representation, or NULL value.  If there is a
+                * converter it won't expect NULL values, and cost model is not
+                * required to account for NULL, so in that case we avoid calling
+                * converter and just set datum1 to zeroed representation (to be
+                * consistent, and to support cheap inequality tests for NULL
+                * abbreviated keys).
+                */
+               stup.datum1 = original;
+       }
+       else if (!consider_abort_common(state))
+       {
+               /* Store abbreviated key representation */
+               stup.datum1 = state->sortKeys->abbrev_converter(original,
+                                                                                                               state->sortKeys);
+       }
+       else
+       {
+               /* Abort abbreviation */
+               int                     i;
+
+               stup.datum1 = original;
+
+               /*
+                * Set state to be consistent with never trying abbreviation.
+                *
+                * Alter datum1 representation in already-copied tuples, so as to
+                * ensure a consistent representation (current tuple was just
+                * handled).  It does not matter if some dumped tuples are already
+                * sorted on tape, since serialized tuples lack abbreviated keys
+                * (TSS_BUILDRUNS state prevents control reaching here in any case).
+                */
+               for (i = 0; i < state->memtupcount; i++)
+               {
+                       SortTuple  *mtup = &state->memtuples[i];
+
+                       htup.t_len = ((MinimalTuple) mtup->tuple)->t_len +
+                               MINIMAL_TUPLE_OFFSET;
+                       htup.t_data = (HeapTupleHeader) ((char *) mtup->tuple -
+                                                                                        MINIMAL_TUPLE_OFFSET);
+
+                       mtup->datum1 = heap_getattr(&htup,
+                                                                               state->sortKeys[0].ssup_attno,
+                                                                               state->tupDesc,
+                                                                               &mtup->isnull1);
+               }
+       }
 
        puttuple_common(state, &stup);
 
@@ -1861,14 +1900,75 @@ tuplesort_puttupleslot(Tuplesortstate *state, TupleTableSlot *slot)
 void
 tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup)
 {
-       MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
        SortTuple       stup;
+       Datum           original;
+       MemoryContext oldcontext = MemoryContextSwitchTo(state->tuplecontext);
+
+       /* copy the tuple into sort storage */
+       tup = heap_copytuple(tup);
+       stup.tuple = (void *) tup;
+       USEMEM(state, GetMemoryChunkSpace(tup));
+
+       MemoryContextSwitchTo(state->sortcontext);
 
        /*
-        * Copy the given tuple into memory we control, and decrease availMem.
-        * Then call the common code.
+        * set up first-column key value, and potentially abbreviate, if it's a
+        * simple column
         */
-       COPYTUP(state, &stup, (void *) tup);
+       if (state->haveDatum1)
+       {
+               original = heap_getattr(tup,
+                                                               state->indexInfo->ii_IndexAttrNumbers[0],
+                                                               state->tupDesc,
+                                                               &stup.isnull1);
+
+               if (!state->sortKeys->abbrev_converter || stup.isnull1)
+               {
+                       /*
+                        * Store ordinary Datum representation, or NULL value.  If there
+                        * is a converter it won't expect NULL values, and cost model is
+                        * not required to account for NULL, so in that case we avoid
+                        * calling converter and just set datum1 to zeroed representation
+                        * (to be consistent, and to support cheap inequality tests for
+                        * NULL abbreviated keys).
+                        */
+                       stup.datum1 = original;
+               }
+               else if (!consider_abort_common(state))
+               {
+                       /* Store abbreviated key representation */
+                       stup.datum1 = state->sortKeys->abbrev_converter(original,
+                                                                                                                       state->sortKeys);
+               }
+               else
+               {
+                       /* Abort abbreviation */
+                       int                     i;
+
+                       stup.datum1 = original;
+
+                       /*
+                        * Set state to be consistent with never trying abbreviation.
+                        *
+                        * Alter datum1 representation in already-copied tuples, so as to
+                        * ensure a consistent representation (current tuple was just
+                        * handled).  It does not matter if some dumped tuples are already
+                        * sorted on tape, since serialized tuples lack abbreviated keys
+                        * (TSS_BUILDRUNS state prevents control reaching here in any
+                        * case).
+                        */
+                       for (i = 0; i < state->memtupcount; i++)
+                       {
+                               SortTuple  *mtup = &state->memtuples[i];
+
+                               tup = (HeapTuple) mtup->tuple;
+                               mtup->datum1 = heap_getattr(tup,
+                                                                                       state->indexInfo->ii_IndexAttrNumbers[0],
+                                                                                       state->tupDesc,
+                                                                                       &mtup->isnull1);
+                       }
+               }
+       }
 
        puttuple_common(state, &stup);
 
@@ -3947,84 +4047,6 @@ comparetup_heap(const SortTuple *a, const SortTuple *b, Tuplesortstate *state)
        return 0;
 }
 
-static void
-copytup_heap(Tuplesortstate *state, SortTuple *stup, void *tup)
-{
-       /*
-        * We expect the passed "tup" to be a TupleTableSlot, and form a
-        * MinimalTuple using the exported interface for that.
-        */
-       TupleTableSlot *slot = (TupleTableSlot *) tup;
-       Datum           original;
-       MinimalTuple tuple;
-       HeapTupleData htup;
-       MemoryContext oldcontext = MemoryContextSwitchTo(state->tuplecontext);
-
-       /* copy the tuple into sort storage */
-       tuple = ExecCopySlotMinimalTuple(slot);
-       stup->tuple = (void *) tuple;
-       USEMEM(state, GetMemoryChunkSpace(tuple));
-       /* set up first-column key value */
-       htup.t_len = tuple->t_len + MINIMAL_TUPLE_OFFSET;
-       htup.t_data = (HeapTupleHeader) ((char *) tuple - MINIMAL_TUPLE_OFFSET);
-       original = heap_getattr(&htup,
-                                                       state->sortKeys[0].ssup_attno,
-                                                       state->tupDesc,
-                                                       &stup->isnull1);
-
-       MemoryContextSwitchTo(oldcontext);
-
-       if (!state->sortKeys->abbrev_converter || stup->isnull1)
-       {
-               /*
-                * Store ordinary Datum representation, or NULL value.  If there is a
-                * converter it won't expect NULL values, and cost model is not
-                * required to account for NULL, so in that case we avoid calling
-                * converter and just set datum1 to zeroed representation (to be
-                * consistent, and to support cheap inequality tests for NULL
-                * abbreviated keys).
-                */
-               stup->datum1 = original;
-       }
-       else if (!consider_abort_common(state))
-       {
-               /* Store abbreviated key representation */
-               stup->datum1 = state->sortKeys->abbrev_converter(original,
-                                                                                                                state->sortKeys);
-       }
-       else
-       {
-               /* Abort abbreviation */
-               int                     i;
-
-               stup->datum1 = original;
-
-               /*
-                * Set state to be consistent with never trying abbreviation.
-                *
-                * Alter datum1 representation in already-copied tuples, so as to
-                * ensure a consistent representation (current tuple was just
-                * handled).  It does not matter if some dumped tuples are already
-                * sorted on tape, since serialized tuples lack abbreviated keys
-                * (TSS_BUILDRUNS state prevents control reaching here in any case).
-                */
-               for (i = 0; i < state->memtupcount; i++)
-               {
-                       SortTuple  *mtup = &state->memtuples[i];
-
-                       htup.t_len = ((MinimalTuple) mtup->tuple)->t_len +
-                               MINIMAL_TUPLE_OFFSET;
-                       htup.t_data = (HeapTupleHeader) ((char *) mtup->tuple -
-                                                                                        MINIMAL_TUPLE_OFFSET);
-
-                       mtup->datum1 = heap_getattr(&htup,
-                                                                               state->sortKeys[0].ssup_attno,
-                                                                               state->tupDesc,
-                                                                               &mtup->isnull1);
-               }
-       }
-}
-
 static void
 writetup_heap(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
 {
@@ -4193,79 +4215,6 @@ comparetup_cluster(const SortTuple *a, const SortTuple *b,
        return 0;
 }
 
-static void
-copytup_cluster(Tuplesortstate *state, SortTuple *stup, void *tup)
-{
-       HeapTuple       tuple = (HeapTuple) tup;
-       Datum           original;
-       MemoryContext oldcontext = MemoryContextSwitchTo(state->tuplecontext);
-
-       /* copy the tuple into sort storage */
-       tuple = heap_copytuple(tuple);
-       stup->tuple = (void *) tuple;
-       USEMEM(state, GetMemoryChunkSpace(tuple));
-
-       MemoryContextSwitchTo(oldcontext);
-
-       /*
-        * set up first-column key value, and potentially abbreviate, if it's a
-        * simple column
-        */
-       if (!state->haveDatum1)
-               return;
-
-       original = heap_getattr(tuple,
-                                                       state->indexInfo->ii_IndexAttrNumbers[0],
-                                                       state->tupDesc,
-                                                       &stup->isnull1);
-
-       if (!state->sortKeys->abbrev_converter || stup->isnull1)
-       {
-               /*
-                * Store ordinary Datum representation, or NULL value.  If there is a
-                * converter it won't expect NULL values, and cost model is not
-                * required to account for NULL, so in that case we avoid calling
-                * converter and just set datum1 to zeroed representation (to be
-                * consistent, and to support cheap inequality tests for NULL
-                * abbreviated keys).
-                */
-               stup->datum1 = original;
-       }
-       else if (!consider_abort_common(state))
-       {
-               /* Store abbreviated key representation */
-               stup->datum1 = state->sortKeys->abbrev_converter(original,
-                                                                                                                state->sortKeys);
-       }
-       else
-       {
-               /* Abort abbreviation */
-               int                     i;
-
-               stup->datum1 = original;
-
-               /*
-                * Set state to be consistent with never trying abbreviation.
-                *
-                * Alter datum1 representation in already-copied tuples, so as to
-                * ensure a consistent representation (current tuple was just
-                * handled).  It does not matter if some dumped tuples are already
-                * sorted on tape, since serialized tuples lack abbreviated keys
-                * (TSS_BUILDRUNS state prevents control reaching here in any case).
-                */
-               for (i = 0; i < state->memtupcount; i++)
-               {
-                       SortTuple  *mtup = &state->memtuples[i];
-
-                       tuple = (HeapTuple) mtup->tuple;
-                       mtup->datum1 = heap_getattr(tuple,
-                                                                               state->indexInfo->ii_IndexAttrNumbers[0],
-                                                                               state->tupDesc,
-                                                                               &mtup->isnull1);
-               }
-       }
-}
-
 static void
 writetup_cluster(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
 {
@@ -4512,13 +4461,6 @@ comparetup_index_hash(const SortTuple *a, const SortTuple *b,
        return 0;
 }
 
-static void
-copytup_index(Tuplesortstate *state, SortTuple *stup, void *tup)
-{
-       /* Not currently needed */
-       elog(ERROR, "copytup_index() should not be called");
-}
-
 static void
 writetup_index(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
 {
@@ -4583,13 +4525,6 @@ comparetup_datum(const SortTuple *a, const SortTuple *b, Tuplesortstate *state)
        return compare;
 }
 
-static void
-copytup_datum(Tuplesortstate *state, SortTuple *stup, void *tup)
-{
-       /* Not currently needed */
-       elog(ERROR, "copytup_datum() should not be called");
-}
-
 static void
 writetup_datum(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
 {