Fix some incorrect preprocessor tests in tuplesort specializations
authorDavid Rowley <[email protected]>
Tue, 10 May 2022 23:38:13 +0000 (11:38 +1200)
committerDavid Rowley <[email protected]>
Tue, 10 May 2022 23:38:13 +0000 (11:38 +1200)
697492434 added 3 new quicksort specialization functions for common
datatypes.

That commit was not very consistent in how it would determine if we're
compiling for 32-bit or 64-bit machines.  It would sometimes use
USE_FLOAT8_BYVAL and at other times check if SIZEOF_DATUM == 8.  This
could cause theoretical problems due to the way USE_FLOAT8_BYVAL is now
defined based on SIZEOF_VOID_P >= 8.  If pointers for some reason were
ever larger than 8-bytes then we'd end up doing 32-bit comparisons
mistakenly.  Let's just always check SIZEOF_DATUM >= 8.

It also seems that ssup_datum_signed_cmp is just never used on 32-bit
builds, so let's just ifdef that out to make sure we never accidentally
use that comparison function on such machines.  This also allows us to
ifdef out 1 of the 3 new specialization quicksort functions in 32-bit
builds which seems to shrink down the binary by over 4KB on my machine.

In passing, also add the missing DatumGetInt32() / DatumGetInt64() macros
in the comparison functions.

Discussion: https://postgr.es/m/CAApHDvqcQExRhtRa9hJrJB_5egs3SUfOcutP3m+3HO8A+fZTPA@mail.gmail.com
Reviewed-by: John Naylor
src/backend/access/nbtree/nbtcompare.c
src/backend/utils/adt/timestamp.c
src/backend/utils/sort/tuplesort.c
src/include/utils/sortsupport.h

index 40de3878fe5c88469f3c67eef437ebcb0ab1cd64..7e18e2fc62fc4d90ecee26837def03b155d1ddf0 100644 (file)
@@ -142,7 +142,7 @@ btint8cmp(PG_FUNCTION_ARGS)
        PG_RETURN_INT32(A_LESS_THAN_B);
 }
 
-#ifndef USE_FLOAT8_BYVAL
+#if SIZEOF_DATUM < 8
 static int
 btint8fastcmp(Datum x, Datum y, SortSupport ssup)
 {
@@ -163,7 +163,7 @@ btint8sortsupport(PG_FUNCTION_ARGS)
 {
    SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);
 
-#ifdef USE_FLOAT8_BYVAL
+#if SIZEOF_DATUM >= 8
    ssup->comparator = ssup_datum_signed_cmp;
 #else
    ssup->comparator = btint8fastcmp;
index 552b631ba784e04a6f608b9c8406475e7930785d..8acb725bc8f17de6fb0ca9ea4fd23cc4a1bc94a5 100644 (file)
@@ -2176,7 +2176,7 @@ timestamp_cmp(PG_FUNCTION_ARGS)
    PG_RETURN_INT32(timestamp_cmp_internal(dt1, dt2));
 }
 
-#ifndef USE_FLOAT8_BYVAL
+#if SIZEOF_DATUM < 8
 /* note: this is used for timestamptz also */
 static int
 timestamp_fastcmp(Datum x, Datum y, SortSupport ssup)
@@ -2193,7 +2193,7 @@ timestamp_sortsupport(PG_FUNCTION_ARGS)
 {
    SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);
 
-#ifdef USE_FLOAT8_BYVAL
+#if SIZEOF_DATUM >= 8
    /*
     * If this build has pass-by-value timestamps, then we can use a standard
     * comparator function.
index e8da988a73c2154120ebd1830f90744365622556..a4c3b736678295fa4ada18384e7de9beb81c70e5 100644 (file)
@@ -715,6 +715,7 @@ qsort_tuple_unsigned_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
    return state->comparetup(a, b, state);
 }
 
+#if SIZEOF_DATUM >= 8
 /* Used if first key's comparator is ssup_datum_signed_compare */
 static pg_attribute_always_inline int
 qsort_tuple_signed_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
@@ -737,6 +738,7 @@ qsort_tuple_signed_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
 
    return state->comparetup(a, b, state);
 }
+#endif
 
 /* Used if first key's comparator is ssup_datum_int32_compare */
 static pg_attribute_always_inline int
@@ -779,6 +781,7 @@ qsort_tuple_int32_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
 #define ST_DEFINE
 #include "lib/sort_template.h"
 
+#if SIZEOF_DATUM >= 8
 #define ST_SORT qsort_tuple_signed
 #define ST_ELEMENT_TYPE SortTuple
 #define ST_COMPARE(a, b, state) qsort_tuple_signed_compare(a, b, state)
@@ -787,6 +790,7 @@ qsort_tuple_int32_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
 #define ST_SCOPE static
 #define ST_DEFINE
 #include "lib/sort_template.h"
+#endif
 
 #define ST_SORT qsort_tuple_int32
 #define ST_ELEMENT_TYPE SortTuple
@@ -3666,6 +3670,7 @@ tuplesort_sort_memtuples(Tuplesortstate *state)
                                     state);
                return;
            }
+#if SIZEOF_DATUM >= 8
            else if (state->sortKeys[0].comparator == ssup_datum_signed_cmp)
            {
                elog(DEBUG1, "qsort_tuple_signed");
@@ -3674,6 +3679,7 @@ tuplesort_sort_memtuples(Tuplesortstate *state)
                                   state);
                return;
            }
+#endif
            else if (state->sortKeys[0].comparator == ssup_datum_int32_cmp)
            {
                elog(DEBUG1, "qsort_tuple_int32");
@@ -4905,16 +4911,12 @@ ssup_datum_unsigned_cmp(Datum x, Datum y, SortSupport ssup)
        return 0;
 }
 
+#if SIZEOF_DATUM >= 8
 int
 ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup)
 {
-#if SIZEOF_DATUM == 8
-   int64       xx = (int64) x;
-   int64       yy = (int64) y;
-#else
-   int32       xx = (int32) x;
-   int32       yy = (int32) y;
-#endif
+   int64       xx = DatumGetInt64(x);
+   int64       yy = DatumGetInt64(y);
 
    if (xx < yy)
        return -1;
@@ -4923,12 +4925,13 @@ ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup)
    else
        return 0;
 }
+#endif
 
 int
 ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup)
 {
-   int32       xx = (int32) x;
-   int32       yy = (int32) y;
+   int32       xx = DatumGetInt32(x);
+   int32       yy = DatumGetInt32(y);
 
    if (xx < yy)
        return -1;
index 4f7c73f0aacf740abde715fd27991184f87172bb..ae8f4852a8bdfb988a4f1ca39e3a2b3243537b7b 100644 (file)
@@ -379,7 +379,9 @@ ApplySortAbbrevFullComparator(Datum datum1, bool isNull1,
  * are eligible for faster sorting.
  */
 extern int ssup_datum_unsigned_cmp(Datum x, Datum y, SortSupport ssup);
+#if SIZEOF_DATUM >= 8
 extern int ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup);
+#endif
 extern int ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup);
 
 /* Other functions in utils/sort/sortsupport.c */