On Thu, Feb 8, 2024 at 3:56 AM Nathan Bossart <nathandboss...@gmail.com>
wrote:
> On Thu, Feb 08, 2024 at 03:49:03PM +1300, Thomas Munro wrote:
> > On Thu, Feb 8, 2024 at 3:38 PM Thomas Munro <thomas.mu...@gmail.com>
> wrote:
> >> Perhaps you could wrap it in a branch-free sign() function so you get
> >> a narrow answer?
> >>
> >> https://stackoverflow.com/questions/14579920/fast-sign-of-integer-in-c
> >
> > Ah, strike that, it is much like the suggested (a > b) - (a < b) but
> > with extra steps...
>
> Yeah, https://godbolt.org/ indicates that the sign approach compiles to
>
> movsx rsi, esi
> movsx rdi, edi
> xor eax, eax
> sub rdi, rsi
> test rdi, rdi
> setg al
> shr rdi, 63
> sub eax, edi
> ret
>
> while the approach Andres suggested compiles to
>
> xor eax, eax
> cmp edi, esi
> setl dl
> setg al
> movzx edx, dl
> sub eax, edx
> ret
>
Here is a patch that fixes existing cases and introduces a macro for this
comparison (it uses the (a > b) - (a < b) approach). Not sure where to
place the macro nor what a suitable name should be, so feel free to suggest
anything.
I also noted that some functions are duplicated and it might be an idea to
introduce a few standard functions like pg_qsort_strcmp for, e.g., integers
and other common types.
Also noted it is quite common to have this pattern in various places to do
lexicographic sort of multiple values and continue the comparison if they
are equal. Not sure if that is something we should look at.
Best wishes,
Mats Kindahl
>
> --
> Nathan Bossart
> Amazon Web Services: https://aws.amazon.com
>
From 8c183806a6b0f95ab53db4b029bc82823785c363 Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@timescale.com>
Date: Tue, 6 Feb 2024 14:53:53 +0100
Subject: Standardize integer comparison for qsort()
Introduce a macro INT_CMP() that will standardize integer comparison
for implementing comparison function for qsort(). The macro will ensure
that the function returns -1, 0, or 1 without risking overflow as a
result of subtracting the two integers, which is otherwise a common
pattern for implementing this.
---
contrib/hstore/hstore_gist.c | 2 +-
contrib/intarray/_intbig_gist.c | 2 +-
contrib/pg_stat_statements/pg_stat_statements.c | 7 +------
contrib/pg_trgm/trgm_op.c | 7 +------
contrib/seg/seg.c | 7 +------
src/backend/access/nbtree/nbtinsert.c | 7 +------
src/backend/access/nbtree/nbtpage.c | 9 ++-------
src/backend/access/nbtree/nbtsplitloc.c | 7 +------
src/backend/access/spgist/spgdoinsert.c | 4 +---
src/backend/access/spgist/spgkdtreeproc.c | 8 ++------
src/backend/access/spgist/spgtextproc.c | 2 +-
src/backend/backup/basebackup_incremental.c | 7 +------
src/backend/backup/walsummary.c | 6 +-----
src/backend/catalog/heap.c | 7 +------
src/backend/nodes/list.c | 12 ++----------
src/backend/nodes/tidbitmap.c | 6 +-----
src/backend/optimizer/geqo/geqo_pool.c | 7 +------
src/backend/parser/parse_agg.c | 2 +-
src/backend/postmaster/autovacuum.c | 5 +----
src/backend/replication/logical/reorderbuffer.c | 6 +-----
src/backend/replication/syncrep.c | 7 +------
src/backend/utils/adt/oid.c | 6 +-----
src/backend/utils/adt/tsgistidx.c | 9 ++-------
src/backend/utils/adt/tsquery_gist.c | 5 +----
src/backend/utils/adt/tsvector.c | 4 +---
src/backend/utils/adt/tsvector_op.c | 4 +---
src/backend/utils/adt/xid.c | 6 +-----
src/backend/utils/cache/relcache.c | 2 +-
src/backend/utils/cache/syscache.c | 4 +---
src/backend/utils/cache/typcache.c | 7 +------
src/backend/utils/resowner/resowner.c | 9 +--------
src/bin/pg_dump/pg_dump_sort.c | 6 +-----
src/bin/pg_upgrade/function.c | 8 ++++----
src/bin/pg_walsummary/pg_walsummary.c | 7 +------
src/bin/psql/crosstabview.c | 2 +-
src/include/access/gin_private.h | 7 +------
src/include/c.h | 8 ++++++++
37 files changed, 51 insertions(+), 170 deletions(-)
diff --git a/contrib/hstore/hstore_gist.c b/contrib/hstore/hstore_gist.c
index fe343739eb..e125b76290 100644
--- a/contrib/hstore/hstore_gist.c
+++ b/contrib/hstore/hstore_gist.c
@@ -356,7 +356,7 @@ typedef struct
static int
comparecost(const void *a, const void *b)
{
- return ((const SPLITCOST *) a)->cost - ((const SPLITCOST *) b)->cost;
+ return INT_CMP(((const SPLITCOST *) a)->cost, ((const SPLITCOST *) b)->cost);
}
diff --git a/contrib/intarray/_intbig_gist.c b/contrib/intarray/_intbig_gist.c
index 8c6c4b5d05..7f0deda34d 100644
--- a/contrib/intarray/_intbig_gist.c
+++ b/contrib/intarray/_intbig_gist.c
@@ -312,7 +312,7 @@ typedef struct
static int
comparecost(const void *a, const void *b)
{
- return ((const SPLITCOST *) a)->cost - ((const SPLITCOST *) b)->cost;
+ return INT_CMP(((const SPLITCOST *) a)->cost, ((const SPLITCOST *) b)->cost);
}
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 8c6a3a2d08..f32e6868ed 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -3007,10 +3007,5 @@ comp_location(const void *a, const void *b)
int l = ((const LocationLen *) a)->location;
int r = ((const LocationLen *) b)->location;
- if (l < r)
- return -1;
- else if (l > r)
- return +1;
- else
- return 0;
+ return INT_CMP(l, r);
}
diff --git a/contrib/pg_trgm/trgm_op.c b/contrib/pg_trgm/trgm_op.c
index 49d4497b4f..f6020141a2 100644
--- a/contrib/pg_trgm/trgm_op.c
+++ b/contrib/pg_trgm/trgm_op.c
@@ -433,12 +433,7 @@ comp_ptrgm(const void *v1, const void *v2)
if (cmp != 0)
return cmp;
- if (p1->index < p2->index)
- return -1;
- else if (p1->index == p2->index)
- return 0;
- else
- return 1;
+ return INT_CMP(p1->index, p2->index);
}
/*
diff --git a/contrib/seg/seg.c b/contrib/seg/seg.c
index 7f9fc24eb4..ff2ffaf8c7 100644
--- a/contrib/seg/seg.c
+++ b/contrib/seg/seg.c
@@ -300,12 +300,7 @@ gseg_picksplit_item_cmp(const void *a, const void *b)
const gseg_picksplit_item *i1 = (const gseg_picksplit_item *) a;
const gseg_picksplit_item *i2 = (const gseg_picksplit_item *) b;
- if (i1->center < i2->center)
- return -1;
- else if (i1->center == i2->center)
- return 0;
- else
- return 1;
+ return INT_CMP(i1->center, i2->center);
}
/*
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 709edd1c17..1aeaf19efd 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -3013,10 +3013,5 @@ _bt_blk_cmp(const void *arg1, const void *arg2)
BlockNumber b1 = *((BlockNumber *) arg1);
BlockNumber b2 = *((BlockNumber *) arg2);
- if (b1 < b2)
- return -1;
- else if (b1 > b2)
- return 1;
-
- return 0;
+ return INT_CMP(b1, b2);
}
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 567bade9f4..bb31cda542 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -1466,14 +1466,9 @@ _bt_delitems_cmp(const void *a, const void *b)
TM_IndexDelete *indexdelete1 = (TM_IndexDelete *) a;
TM_IndexDelete *indexdelete2 = (TM_IndexDelete *) b;
- if (indexdelete1->id > indexdelete2->id)
- return 1;
- if (indexdelete1->id < indexdelete2->id)
- return -1;
+ Assert(INT_CMP(indexdelete1->id, indexdelete2->id) != 0);
- Assert(false);
-
- return 0;
+ return INT_CMP(indexdelete1->id, indexdelete2->id);
}
/*
diff --git a/src/backend/access/nbtree/nbtsplitloc.c b/src/backend/access/nbtree/nbtsplitloc.c
index d0b1d82578..f33f1f9d51 100644
--- a/src/backend/access/nbtree/nbtsplitloc.c
+++ b/src/backend/access/nbtree/nbtsplitloc.c
@@ -596,12 +596,7 @@ _bt_splitcmp(const void *arg1, const void *arg2)
SplitPoint *split1 = (SplitPoint *) arg1;
SplitPoint *split2 = (SplitPoint *) arg2;
- if (split1->curdelta > split2->curdelta)
- return 1;
- if (split1->curdelta < split2->curdelta)
- return -1;
-
- return 0;
+ return INT_CMP(split1->curdelta, split2->curdelta);
}
/*
diff --git a/src/backend/access/spgist/spgdoinsert.c b/src/backend/access/spgist/spgdoinsert.c
index bb063c858d..995392d480 100644
--- a/src/backend/access/spgist/spgdoinsert.c
+++ b/src/backend/access/spgist/spgdoinsert.c
@@ -110,9 +110,7 @@ addNode(SpGistState *state, SpGistInnerTuple tuple, Datum label, int offset)
static int
cmpOffsetNumbers(const void *a, const void *b)
{
- if (*(const OffsetNumber *) a == *(const OffsetNumber *) b)
- return 0;
- return (*(const OffsetNumber *) a > *(const OffsetNumber *) b) ? 1 : -1;
+ return INT_CMP(*(const OffsetNumber *) a, *(const OffsetNumber *) b);
}
/*
diff --git a/src/backend/access/spgist/spgkdtreeproc.c b/src/backend/access/spgist/spgkdtreeproc.c
index 900fe0d2af..a1a2598a3f 100644
--- a/src/backend/access/spgist/spgkdtreeproc.c
+++ b/src/backend/access/spgist/spgkdtreeproc.c
@@ -87,9 +87,7 @@ x_cmp(const void *a, const void *b)
SortedPoint *pa = (SortedPoint *) a;
SortedPoint *pb = (SortedPoint *) b;
- if (pa->p->x == pb->p->x)
- return 0;
- return (pa->p->x > pb->p->x) ? 1 : -1;
+ return INT_CMP(pa->p->x, pb->p->x);
}
static int
@@ -98,9 +96,7 @@ y_cmp(const void *a, const void *b)
SortedPoint *pa = (SortedPoint *) a;
SortedPoint *pb = (SortedPoint *) b;
- if (pa->p->y == pb->p->y)
- return 0;
- return (pa->p->y > pb->p->y) ? 1 : -1;
+ return INT_CMP(pa->p->y, pb->p->y);
}
diff --git a/src/backend/access/spgist/spgtextproc.c b/src/backend/access/spgist/spgtextproc.c
index b8fd0c2ad8..4855f1f1de 100644
--- a/src/backend/access/spgist/spgtextproc.c
+++ b/src/backend/access/spgist/spgtextproc.c
@@ -325,7 +325,7 @@ cmpNodePtr(const void *a, const void *b)
const spgNodePtr *aa = (const spgNodePtr *) a;
const spgNodePtr *bb = (const spgNodePtr *) b;
- return aa->c - bb->c;
+ return INT_CMP(aa->c, bb->c);
}
Datum
diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c
index 0504c465db..c45c015f3c 100644
--- a/src/backend/backup/basebackup_incremental.c
+++ b/src/backend/backup/basebackup_incremental.c
@@ -994,10 +994,5 @@ compare_block_numbers(const void *a, const void *b)
BlockNumber aa = *(BlockNumber *) a;
BlockNumber bb = *(BlockNumber *) b;
- if (aa > bb)
- return 1;
- else if (aa == bb)
- return 0;
- else
- return -1;
+ return INT_CMP(aa,bb);
}
diff --git a/src/backend/backup/walsummary.c b/src/backend/backup/walsummary.c
index 867870aaad..41e34fcb8b 100644
--- a/src/backend/backup/walsummary.c
+++ b/src/backend/backup/walsummary.c
@@ -355,9 +355,5 @@ ListComparatorForWalSummaryFiles(const ListCell *a, const ListCell *b)
WalSummaryFile *ws1 = lfirst(a);
WalSummaryFile *ws2 = lfirst(b);
- if (ws1->start_lsn < ws2->start_lsn)
- return -1;
- if (ws1->start_lsn > ws2->start_lsn)
- return 1;
- return 0;
+ return INT_CMP(ws1->start_lsn, ws2->start_lsn);
}
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index c73f7bcd01..b26d2bf04a 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2758,12 +2758,7 @@ list_cookedconstr_attnum_cmp(const ListCell *p1, const ListCell *p2)
{
AttrNumber v1 = ((CookedConstraint *) lfirst(p1))->attnum;
AttrNumber v2 = ((CookedConstraint *) lfirst(p2))->attnum;
-
- if (v1 < v2)
- return -1;
- if (v1 > v2)
- return 1;
- return 0;
+ return INT_CMP(v1, v2);
}
/*
diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c
index 957186bef5..769c97ee83 100644
--- a/src/backend/nodes/list.c
+++ b/src/backend/nodes/list.c
@@ -1692,11 +1692,7 @@ list_int_cmp(const ListCell *p1, const ListCell *p2)
int v1 = lfirst_int(p1);
int v2 = lfirst_int(p2);
- if (v1 < v2)
- return -1;
- if (v1 > v2)
- return 1;
- return 0;
+ return INT_CMP(v1, v2);
}
/*
@@ -1708,9 +1704,5 @@ list_oid_cmp(const ListCell *p1, const ListCell *p2)
Oid v1 = lfirst_oid(p1);
Oid v2 = lfirst_oid(p2);
- if (v1 < v2)
- return -1;
- if (v1 > v2)
- return 1;
- return 0;
+ return INT_CMP(v1, v2);
}
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index 0f4850065f..944f261e38 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -1425,11 +1425,7 @@ tbm_comparator(const void *left, const void *right)
BlockNumber l = (*((PagetableEntry *const *) left))->blockno;
BlockNumber r = (*((PagetableEntry *const *) right))->blockno;
- if (l < r)
- return -1;
- else if (l > r)
- return 1;
- return 0;
+ return INT_CMP(l,r);
}
/*
diff --git a/src/backend/optimizer/geqo/geqo_pool.c b/src/backend/optimizer/geqo/geqo_pool.c
index 0ec97d5a3f..165ff7a380 100644
--- a/src/backend/optimizer/geqo/geqo_pool.c
+++ b/src/backend/optimizer/geqo/geqo_pool.c
@@ -147,12 +147,7 @@ compare(const void *arg1, const void *arg2)
const Chromosome *chromo1 = (const Chromosome *) arg1;
const Chromosome *chromo2 = (const Chromosome *) arg2;
- if (chromo1->worth == chromo2->worth)
- return 0;
- else if (chromo1->worth > chromo2->worth)
- return 1;
- else
- return -1;
+ return INT_CMP(chromo1->worth, chromo2->worth);
}
/* alloc_chromo
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 7b211a7743..af3c4da463 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -1760,7 +1760,7 @@ cmp_list_len_asc(const ListCell *a, const ListCell *b)
int la = list_length((const List *) lfirst(a));
int lb = list_length((const List *) lfirst(b));
- return (la > lb) ? 1 : (la == lb) ? 0 : -1;
+ return INT_CMP(la, lb);
}
/* list_sort comparator to sort sub-lists by length and contents */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 2c3099f76f..82a6f4a2f0 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -1119,10 +1119,7 @@ rebuild_database_list(Oid newdb)
static int
db_comparator(const void *a, const void *b)
{
- if (((const avl_dbase *) a)->adl_score == ((const avl_dbase *) b)->adl_score)
- return 0;
- else
- return (((const avl_dbase *) a)->adl_score < ((const avl_dbase *) b)->adl_score) ? 1 : -1;
+ return INT_CMP(((const avl_dbase *) a)->adl_score, ((const avl_dbase *) b)->adl_score);
}
/*
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index bbf0966182..5d61a2227e 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -5119,11 +5119,7 @@ file_sort_by_lsn(const ListCell *a_p, const ListCell *b_p)
RewriteMappingFile *a = (RewriteMappingFile *) lfirst(a_p);
RewriteMappingFile *b = (RewriteMappingFile *) lfirst(b_p);
- if (a->lsn < b->lsn)
- return -1;
- else if (a->lsn > b->lsn)
- return 1;
- return 0;
+ return INT_CMP(a->lsn, b->lsn);
}
/*
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 2e6493aaaa..6aecce074d 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -698,12 +698,7 @@ cmp_lsn(const void *a, const void *b)
XLogRecPtr lsn1 = *((const XLogRecPtr *) a);
XLogRecPtr lsn2 = *((const XLogRecPtr *) b);
- if (lsn1 > lsn2)
- return -1;
- else if (lsn1 == lsn2)
- return 0;
- else
- return 1;
+ return INT_CMP(lsn2, lsn1);
}
/*
diff --git a/src/backend/utils/adt/oid.c b/src/backend/utils/adt/oid.c
index 62bcfc5b56..a46adfe764 100644
--- a/src/backend/utils/adt/oid.c
+++ b/src/backend/utils/adt/oid.c
@@ -259,11 +259,7 @@ oid_cmp(const void *p1, const void *p2)
Oid v1 = *((const Oid *) p1);
Oid v2 = *((const Oid *) p2);
- if (v1 < v2)
- return -1;
- if (v1 > v2)
- return 1;
- return 0;
+ return INT_CMP(v1, v2);
}
diff --git a/src/backend/utils/adt/tsgistidx.c b/src/backend/utils/adt/tsgistidx.c
index a62b285365..bd6eac3ea4 100644
--- a/src/backend/utils/adt/tsgistidx.c
+++ b/src/backend/utils/adt/tsgistidx.c
@@ -136,9 +136,7 @@ compareint(const void *va, const void *vb)
int32 a = *((const int32 *) va);
int32 b = *((const int32 *) vb);
- if (a == b)
- return 0;
- return (a > b) ? 1 : -1;
+ return INT_CMP(a,b);
}
static void
@@ -598,10 +596,7 @@ comparecost(const void *va, const void *vb)
const SPLITCOST *a = (const SPLITCOST *) va;
const SPLITCOST *b = (const SPLITCOST *) vb;
- if (a->cost == b->cost)
- return 0;
- else
- return (a->cost > b->cost) ? 1 : -1;
+ return INT_CMP(a->cost, b->cost);
}
diff --git a/src/backend/utils/adt/tsquery_gist.c b/src/backend/utils/adt/tsquery_gist.c
index f0b1c81c81..29d49f45dc 100644
--- a/src/backend/utils/adt/tsquery_gist.c
+++ b/src/backend/utils/adt/tsquery_gist.c
@@ -156,10 +156,7 @@ typedef struct
static int
comparecost(const void *a, const void *b)
{
- if (((const SPLITCOST *) a)->cost == ((const SPLITCOST *) b)->cost)
- return 0;
- else
- return (((const SPLITCOST *) a)->cost > ((const SPLITCOST *) b)->cost) ? 1 : -1;
+ return INT_CMP(((const SPLITCOST *) a)->cost, ((const SPLITCOST *) b)->cost);
}
#define WISH_F(a,b,c) (double)( -(double)(((a)-(b))*((a)-(b))*((a)-(b)))*(c) )
diff --git a/src/backend/utils/adt/tsvector.c b/src/backend/utils/adt/tsvector.c
index fb7b7c712a..1875612019 100644
--- a/src/backend/utils/adt/tsvector.c
+++ b/src/backend/utils/adt/tsvector.c
@@ -37,9 +37,7 @@ compareWordEntryPos(const void *a, const void *b)
int apos = WEP_GETPOS(*(const WordEntryPos *) a);
int bpos = WEP_GETPOS(*(const WordEntryPos *) b);
- if (apos == bpos)
- return 0;
- return (apos > bpos) ? 1 : -1;
+ return INT_CMP(apos, bpos);
}
/*
diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index 71386d0a1f..c17d0599e3 100644
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -435,9 +435,7 @@ compare_int(const void *va, const void *vb)
int a = *((const int *) va);
int b = *((const int *) vb);
- if (a == b)
- return 0;
- return (a > b) ? 1 : -1;
+ return INT_CMP(a,b);
}
static int
diff --git a/src/backend/utils/adt/xid.c b/src/backend/utils/adt/xid.c
index 63adf5668b..79b35cbd7b 100644
--- a/src/backend/utils/adt/xid.c
+++ b/src/backend/utils/adt/xid.c
@@ -140,11 +140,7 @@ xidComparator(const void *arg1, const void *arg2)
TransactionId xid1 = *(const TransactionId *) arg1;
TransactionId xid2 = *(const TransactionId *) arg2;
- if (xid1 > xid2)
- return 1;
- if (xid1 < xid2)
- return -1;
- return 0;
+ return INT_CMP(xid1, xid2);
}
/*
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index ac106b40e3..fd3a56349e 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4520,7 +4520,7 @@ AttrDefaultCmp(const void *a, const void *b)
const AttrDefault *ada = (const AttrDefault *) a;
const AttrDefault *adb = (const AttrDefault *) b;
- return ada->adnum - adb->adnum;
+ return INT_CMP(ada->adnum, adb->adnum);
}
/*
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index 162855b158..2bfc7e7fe0 100644
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -676,7 +676,5 @@ oid_compare(const void *a, const void *b)
Oid oa = *((const Oid *) a);
Oid ob = *((const Oid *) b);
- if (oa == ob)
- return 0;
- return (oa > ob) ? 1 : -1;
+ return INT_CMP(oa, ob);
}
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 84fc83cb11..b689471e53 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -2722,12 +2722,7 @@ enum_oid_cmp(const void *left, const void *right)
const EnumItem *l = (const EnumItem *) left;
const EnumItem *r = (const EnumItem *) right;
- if (l->enum_oid < r->enum_oid)
- return -1;
- else if (l->enum_oid > r->enum_oid)
- return 1;
- else
- return 0;
+ return INT_CMP(l->enum_oid, r->enum_oid);
}
/*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index aa199b23ff..4cda8dde7f 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -264,14 +264,7 @@ resource_priority_cmp(const void *a, const void *b)
/* Note: reverse order */
if (ra->kind->release_phase == rb->kind->release_phase)
- {
- if (ra->kind->release_priority == rb->kind->release_priority)
- return 0;
- else if (ra->kind->release_priority > rb->kind->release_priority)
- return -1;
- else
- return 1;
- }
+ return INT_CMP(ra->kind->release_priority, rb->kind->release_priority);
else if (ra->kind->release_phase > rb->kind->release_phase)
return -1;
else
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index f358dd22b9..72937622ac 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -1504,9 +1504,5 @@ int_cmp(void *a, void *b, void *arg)
int ai = (int) (intptr_t) a;
int bi = (int) (intptr_t) b;
- if (ai < bi)
- return -1;
- if (ai > bi)
- return 1;
- return 0;
+ return INT_CMP(ai, bi);
}
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index af998f74d3..d21c534d7c 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -32,14 +32,14 @@ library_name_compare(const void *p1, const void *p2)
int slen1 = strlen(str1);
int slen2 = strlen(str2);
int cmp = strcmp(str1, str2);
+ int lhsdb = ((const LibraryInfo *) p1)->dbnum;
+ int rhsdb = ((const LibraryInfo *) p2)->dbnum;
if (slen1 != slen2)
- return slen1 - slen2;
+ return INT_CMP(slen1, slen2);
if (cmp != 0)
return cmp;
- else
- return ((const LibraryInfo *) p1)->dbnum -
- ((const LibraryInfo *) p2)->dbnum;
+ return INT_CMP(lhsdb, rhsdb);
}
diff --git a/src/bin/pg_walsummary/pg_walsummary.c b/src/bin/pg_walsummary/pg_walsummary.c
index 1341c83c69..2de4547279 100644
--- a/src/bin/pg_walsummary/pg_walsummary.c
+++ b/src/bin/pg_walsummary/pg_walsummary.c
@@ -219,12 +219,7 @@ compare_block_numbers(const void *a, const void *b)
BlockNumber aa = *(BlockNumber *) a;
BlockNumber bb = *(BlockNumber *) b;
- if (aa > bb)
- return 1;
- else if (aa == bb)
- return 0;
- else
- return -1;
+ return INT_CMP(aa, bb);
}
/*
diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c
index c6116b7238..50f24055b6 100644
--- a/src/bin/psql/crosstabview.c
+++ b/src/bin/psql/crosstabview.c
@@ -709,5 +709,5 @@ pivotFieldCompare(const void *a, const void *b)
static int
rankCompare(const void *a, const void *b)
{
- return *((const int *) a) - *((const int *) b);
+ return INT_CMP(*(const int *) a, *(const int *) b);
}
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 51d0c74a6b..da5646ab41 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -489,12 +489,7 @@ ginCompareItemPointers(ItemPointer a, ItemPointer b)
uint64 ia = (uint64) GinItemPointerGetBlockNumber(a) << 32 | GinItemPointerGetOffsetNumber(a);
uint64 ib = (uint64) GinItemPointerGetBlockNumber(b) << 32 | GinItemPointerGetOffsetNumber(b);
- if (ia == ib)
- return 0;
- else if (ia > ib)
- return 1;
- else
- return -1;
+ return INT_CMP(ia, ib);
}
extern int ginTraverseLock(Buffer buffer, bool searchMode);
diff --git a/src/include/c.h b/src/include/c.h
index 2e3ea206e1..114c3373ca 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1090,6 +1090,14 @@ extern void ExceptionalCondition(const char *conditionName,
* ----------------------------------------------------------------
*/
+/*
+ * Compare two integers and return -1, 0, or 1 without risking overflow.
+ *
+ * This macro is used to avoid running into overflow issues because a simple
+ * subtraction of the two values when implementing a cmp function for qsort().
+*/
+#define INT_CMP(lhs,rhs) (((lhs) > (rhs)) - ((lhs) < (rhs)))
+
/*
* Invert the sign of a qsort-style comparison result, ie, exchange negative
* and positive integer values, being careful not to get the wrong answer
--
2.34.1
