Adjust tuplesort.c based on the fact that we never use the OS's qsort().

Our own qsort_arg() implementation doesn't have the defect previously
observed to affect only QNX 4, so it seems sufficiently to assert that
it isn't broken rather than retesting.  Also, update a few comments to
clarify why it's valuable to retain a tie-break rule based on CTID
during index builds.

Peter Geoghegan, with slight tweaks by me.

diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 4c2fe6979939670c2765f8809adaed5d399e6957..1452e8c7cfcb94919ae914c3bc4912f3de5a0028 100644 (file)
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -3047,17 +3047,19 @@ comparetup_index_btree(const SortTuple *a, const SortTuple *b,
         * sort algorithm wouldn't have checked whether one must appear before the
         * other.
         *
-        * Some rather brain-dead implementations of qsort will sometimes call the
-        * comparison routine to compare a value to itself.  (At this writing only
-        * QNX 4 is known to do such silly things; we don't support QNX anymore,
-        * but perhaps the behavior still exists elsewhere.)  Don't raise a bogus
-        * error in that case.
         */
-       if (state->enforceUnique && !equal_hasnull && tuple1 != tuple2)
+       if (state->enforceUnique && !equal_hasnull)
        {
                Datum           values[INDEX_MAX_KEYS];
                bool            isnull[INDEX_MAX_KEYS];
 
+               /*
+                * Some rather brain-dead implementations of qsort (such as the one in QNX 4)
+                * will sometimes call the comparison routine to compare a value to itself,
+                * but we always use our own implementation, which does not.
+                */
+               Assert(tuple1 != tuple2);
+
                index_deform_tuple(tuple1, tupDes, values, isnull);
                ereport(ERROR,
                                (errcode(ERRCODE_UNIQUE_VIOLATION),
@@ -3070,9 +3072,8 @@ comparetup_index_btree(const SortTuple *a, const SortTuple *b,
 
        /*
         * If key values are equal, we sort on ItemPointer.  This does not affect
-        * validity of the finished index, but it offers cheap insurance against
-        * performance problems with bad qsort implementations that have trouble
-        * with large numbers of equal keys.
+        * validity of the finished index, but it may be useful to have index scans
+        * in physical order.
         */
        {
                BlockNumber blk1 = ItemPointerGetBlockNumber(&tuple1->t_tid);
@@ -3120,9 +3121,8 @@ comparetup_index_hash(const SortTuple *a, const SortTuple *b,
 
        /*
         * If hash values are equal, we sort on ItemPointer.  This does not affect
-        * validity of the finished index, but it offers cheap insurance against
-        * performance problems with bad qsort implementations that have trouble
-        * with large numbers of equal keys.
+        * validity of the finished index, but it may be useful to have index scans
+        * in physical order.
         */
        tuple1 = (IndexTuple) a->tuple;
        tuple2 = (IndexTuple) b->tuple;