Comment improvements for abbreviated keys.

Peter Geoghegan and Robert Haas

diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 6572af875a5ee42d63287bb7773476913b2e05b3..cf1cdcbeab58ee2c14783c5c7158cdb988802de3 100644 (file)
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -930,7 +930,14 @@ tuplesort_begin_datum(Oid datumType, Oid sortOperator, Oid sortCollation,
    state->sortKeys->ssup_collation = sortCollation;
    state->sortKeys->ssup_nulls_first = nullsFirstFlag;
 
-   /* abbreviation is possible here only for by-reference types */
+   /*
+    * Abbreviation is possible here only for by-reference types.  In theory,
+    * a pass-by-value datatype could have an abbreviated form that is cheaper
+    * to compare.  In a tuple sort, we could support that, because we can
+    * always extract the original datum from the tuple is needed.  Here, we
+    * can't, because a datum sort only stores a single copy of the datum;
+    * the "tuple" field of each sortTuple is NULL.
+    */
    state->sortKeys->abbreviate = !typbyval;
 
    PrepareSortSupportFromOrderingOp(sortOperator, state->sortKeys);
@@ -1271,7 +1278,7 @@ tuplesort_putindextuplevalues(Tuplesortstate *state, Relation rel,
         * 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 "void" representation (to be
+        * converter and just set datum1 to zeroed representation (to be
         * consistent).
         */
        stup.datum1 = original;
@@ -3155,7 +3162,7 @@ copytup_heap(Tuplesortstate *state, SortTuple *stup, void *tup)
         * 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 "void" representation (to be
+        * converter and just set datum1 to zeroed representation (to be
         * consistent).
         */
        stup->datum1 = original;
@@ -3397,7 +3404,7 @@ copytup_cluster(Tuplesortstate *state, SortTuple *stup, void *tup)
         * 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 "void" representation (to be
+        * converter and just set datum1 to zeroed representation (to be
         * consistent).
         */
        stup->datum1 = original;
@@ -3701,7 +3708,7 @@ copytup_index(Tuplesortstate *state, SortTuple *stup, void *tup)
         * 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 "void" representation (to be
+        * converter and just set datum1 to zeroed representation (to be
         * consistent).
         */
        stup->datum1 = original;

diff --git a/src/include/utils/sortsupport.h b/src/include/utils/sortsupport.h
index 425863055631208bc4b48c8dc0ec95e025876d3b..e890503d31fa259597d0d132567951f316f40537 100644 (file)
--- a/src/include/utils/sortsupport.h
+++ b/src/include/utils/sortsupport.h
@@ -116,24 +116,25 @@ typedef struct SortSupportData
     *
     * This allows opclass authors to supply a conversion routine, used to
     * create an alternative representation of the underlying type (an
-    * "abbreviated key").  Typically, this representation is an ad-hoc,
-    * pass-by-value Datum format that only the opclass has knowledge of.  An
-    * alternative comparator, used only with this alternative representation
-    * must also be provided (which is assigned to "comparator").  This
-    * representation is a simple approximation of the original Datum.  It
-    * must be possible to compare datums of this representation with each
-    * other using the supplied alternative comparator, and have any non-zero
-    * return value be a reliable proxy for what a proper comparison would
-    * indicate. Returning zero from the alternative comparator does not
-    * indicate equality, as with a conventional support routine 1, though --
-    * it indicates that it wasn't possible to determine how the two
-    * abbreviated values compared.  A proper comparison, using
-    * "abbrev_full_comparator"/ ApplySortAbbrevFullComparator() is therefore
-    * required.  In many cases this results in most or all comparisons only
-    * using the cheap alternative comparison func, which is typically
-    * implemented as code that compiles to just a few CPU instructions.  CPU
-    * cache miss penalties are expensive; to get good overall performance,
-    * sort infrastructure must heavily weigh cache performance.
+    * "abbreviated key").  This representation must be pass-by-value and
+    * typically will use some ad-hoc format that only the opclass has
+    * knowledge of.  An alternative comparator, used only with this
+    * alternative representation must also be provided (which is assigned to
+    * "comparator").  This representation is a simple approximation of the
+    * original Datum.  It must be possible to compare datums of this
+    * representation with each other using the supplied alternative
+    * comparator, and have any non-zero return value be a reliable proxy for
+    * what a proper comparison would indicate. Returning zero from the
+    * alternative comparator does not indicate equality, as with a
+    * conventional support routine 1, though -- it indicates that it wasn't
+    * possible to determine how the two abbreviated values compared.  A
+    * proper comparison, using "abbrev_full_comparator"/
+    * ApplySortAbbrevFullComparator() is therefore required.  In many cases
+    * this results in most or all comparisons only using the cheap
+    * alternative comparison func, which is typically implemented as code
+    * that compiles to just a few CPU instructions.  CPU cache miss penalties
+    * are expensive; to get good overall performance, sort infrastructure
+    * must heavily weigh cache performance.
     *
     * Opclass authors must consider the final cardinality of abbreviated keys
     * when devising an encoding scheme.  It's possible for a strategy to work