aboutsummaryrefslogtreecommitdiff
path: root/src/backend/executor
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2018-10-05 16:01:29 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2018-10-05 16:01:29 -0400
commit142cfd3cd82efd4d661f8a8d88960602a439a744 (patch)
tree6c20c105aeeaac1404a7ecb9534158e20cd3287e /src/backend/executor
parent9718c93f532c3cfb6c39679452149dba711d12af (diff)
downloadpostgresql-142cfd3cd82efd4d661f8a8d88960602a439a744.tar.gz
postgresql-142cfd3cd82efd4d661f8a8d88960602a439a744.zip
Allow btree comparison functions to return INT_MIN.
Historically we forbade datatype-specific comparison functions from returning INT_MIN, so that it would be safe to invert the sort order just by negating the comparison result. However, this was never really safe for comparison functions that directly return the result of memcmp(), strcmp(), etc, as POSIX doesn't place any such restriction on those library functions. Buildfarm results show that at least on recent Linux on s390x, memcmp() actually does return INT_MIN sometimes, causing sort failures. The agreed-on answer is to remove this restriction and fix relevant call sites to not make such an assumption; code such as "res = -res" should be replaced by "INVERT_COMPARE_RESULT(res)". The same is needed in a few places that just directly negated the result of memcmp or strcmp. To help find places having this problem, I've also added a compile option to nbtcompare.c that causes some of the commonly used comparators to return INT_MIN/INT_MAX instead of their usual -1/+1. It'd likely be a good idea to have at least one buildfarm member running with "-DSTRESS_SORT_INT_MIN". That's far from a complete test of course, but it should help to prevent fresh introductions of such bugs. This is a longstanding portability hazard, so back-patch to all supported branches. Discussion: https://postgr.es/m/20180928185215.ffoq2xrq5d3pafna@alap3.anarazel.de
Diffstat (limited to 'src/backend/executor')
-rw-r--r--src/backend/executor/nodeGatherMerge.c5
-rw-r--r--src/backend/executor/nodeIndexscan.c7
-rw-r--r--src/backend/executor/nodeMergeAppend.c5
3 files changed, 12 insertions, 5 deletions
diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c
index ee98f4cf30c..9b4d12b90ef 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -756,7 +756,10 @@ heap_compare_slots(Datum a, Datum b, void *arg)
datum2, isNull2,
sortKey);
if (compare != 0)
- return -compare;
+ {
+ INVERT_COMPARE_RESULT(compare);
+ return compare;
+ }
}
return 0;
}
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 700d7635f3c..9ccdab6d071 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -474,9 +474,10 @@ reorderqueue_cmp(const pairingheap_node *a, const pairingheap_node *b,
ReorderTuple *rtb = (ReorderTuple *) b;
IndexScanState *node = (IndexScanState *) arg;
- return -cmp_orderbyvals(rta->orderbyvals, rta->orderbynulls,
- rtb->orderbyvals, rtb->orderbynulls,
- node);
+ /* exchange argument order to invert the sort order */
+ return cmp_orderbyvals(rtb->orderbyvals, rtb->orderbynulls,
+ rta->orderbyvals, rta->orderbynulls,
+ node);
}
/*
diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c
index 6bf490bd700..391e157727b 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -261,7 +261,10 @@ heap_compare_slots(Datum a, Datum b, void *arg)
datum2, isNull2,
sortKey);
if (compare != 0)
- return -compare;
+ {
+ INVERT_COMPARE_RESULT(compare);
+ return compare;
+ }
}
return 0;
}