Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit60cc241

Browse files
committed
Allow btree comparison functions to return INT_MIN.
Historically we forbade datatype-specific comparison functions fromreturning INT_MIN, so that it would be safe to invert the sort orderjust by negating the comparison result. However, this was neverreally safe for comparison functions that directly return the resultof memcmp(), strcmp(), etc, as POSIX doesn't place any such restrictionon those library functions. Buildfarm results show that at least onrecent Linux on s390x, memcmp() actually does return INT_MIN sometimes,causing sort failures.The agreed-on answer is to remove this restriction and fix relevantcall sites to not make such an assumption; code such as "res = -res"should be replaced by "INVERT_COMPARE_RESULT(res)". The same is neededin a few places that just directly negated the result of memcmp orstrcmp.To help find places having this problem, I've also added a compile optionto nbtcompare.c that causes some of the commonly used comparators toreturn INT_MIN/INT_MAX instead of their usual -1/+1. It'd likely bea 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 supportedbranches.Discussion:https://postgr.es/m/20180928185215.ffoq2xrq5d3pafna@alap3.anarazel.de
1 parentdca44d0 commit60cc241

File tree

11 files changed

+86
-55
lines changed

11 files changed

+86
-55
lines changed

‎contrib/ltree/ltree_op.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,24 @@ ltree_compare(const ltree *a, const ltree *b)
4545
ltree_level*bl=LTREE_FIRST(b);
4646
intan=a->numlevel;
4747
intbn=b->numlevel;
48-
intres=0;
4948

5049
while (an>0&&bn>0)
5150
{
51+
intres;
52+
5253
if ((res=memcmp(al->name,bl->name,Min(al->len,bl->len)))==0)
5354
{
5455
if (al->len!=bl->len)
5556
return (al->len-bl->len)*10* (an+1);
5657
}
5758
else
59+
{
60+
if (res<0)
61+
res=-1;
62+
else
63+
res=1;
5864
returnres*10* (an+1);
65+
}
5966

6067
an--;
6168
bn--;
@@ -146,7 +153,7 @@ inner_isparent(const ltree *c, const ltree *p)
146153
{
147154
if (cl->len!=pl->len)
148155
return false;
149-
if (memcmp(cl->name,pl->name,cl->len))
156+
if (memcmp(cl->name,pl->name,cl->len)!=0)
150157
return false;
151158

152159
pn--;

‎contrib/pgcrypto/imath.c

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,11 +1254,9 @@ mp_int_compare(mp_int a, mp_int b)
12541254
* If they're both zero or positive, the normal comparison applies; if
12551255
* both negative, the sense is reversed.
12561256
*/
1257-
if (sa==MP_ZPOS)
1258-
returncmp;
1259-
else
1260-
return-cmp;
1261-
1257+
if (sa!=MP_ZPOS)
1258+
INVERT_COMPARE_RESULT(cmp);
1259+
returncmp;
12621260
}
12631261
else
12641262
{
@@ -1314,10 +1312,9 @@ mp_int_compare_value(mp_int z, int value)
13141312
{
13151313
cmp=s_vcmp(z,value);
13161314

1317-
if (vsign==MP_ZPOS)
1318-
returncmp;
1319-
else
1320-
return-cmp;
1315+
if (vsign!=MP_ZPOS)
1316+
INVERT_COMPARE_RESULT(cmp);
1317+
returncmp;
13211318
}
13221319
else
13231320
{

‎src/backend/access/nbtree/nbtcompare.c

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@
2222
*
2323
*The result is always an int32 regardless of the input datatype.
2424
*
25-
*Although any negative int32(except INT_MIN)is acceptable for reporting
26-
*"<",and any positive int32 is acceptable for reporting ">", routines
25+
*Although any negative int32 is acceptable for reporting "<",
26+
*and any positive int32 is acceptable for reporting ">", routines
2727
*that work on 32-bit or wider datatypes can't just return "a - b".
28-
*That could overflow and give the wrong answer. Also, one must not
29-
*return INT_MIN to report "<", since some callers will negate the result.
28+
*That could overflow and give the wrong answer.
3029
*
3130
*NOTE: it is critical that the comparison function impose a total order
3231
*on all non-NULL values of the data type, and that the datatype's
@@ -44,13 +43,31 @@
4443
*during an index access won't be recovered till end of query. This
4544
*primarily affects comparison routines for toastable datatypes;
4645
*they have to be careful to free any detoasted copy of an input datum.
46+
*
47+
*NOTE: we used to forbid comparison functions from returning INT_MIN,
48+
*but that proves to be too error-prone because some platforms' versions
49+
*of memcmp() etc can return INT_MIN. As a means of stress-testing
50+
*callers, this file can be compiled with STRESS_SORT_INT_MIN defined
51+
*to cause many of these functions to return INT_MIN or INT_MAX instead of
52+
*their customary -1/+1. For production, though, that's not a good idea
53+
*since users or third-party code might expect the traditional results.
4754
*-------------------------------------------------------------------------
4855
*/
4956
#include"postgres.h"
5057

58+
#include<limits.h>
59+
5160
#include"utils/builtins.h"
5261
#include"utils/sortsupport.h"
5362

63+
#ifdefSTRESS_SORT_INT_MIN
64+
#defineA_LESS_THAN_BINT_MIN
65+
#defineA_GREATER_THAN_BINT_MAX
66+
#else
67+
#defineA_LESS_THAN_B(-1)
68+
#defineA_GREATER_THAN_B1
69+
#endif
70+
5471

5572
Datum
5673
btboolcmp(PG_FUNCTION_ARGS)
@@ -95,11 +112,11 @@ btint4cmp(PG_FUNCTION_ARGS)
95112
int32b=PG_GETARG_INT32(1);
96113

97114
if (a>b)
98-
PG_RETURN_INT32(1);
115+
PG_RETURN_INT32(A_GREATER_THAN_B);
99116
elseif (a==b)
100117
PG_RETURN_INT32(0);
101118
else
102-
PG_RETURN_INT32(-1);
119+
PG_RETURN_INT32(A_LESS_THAN_B);
103120
}
104121

105122
staticint
@@ -109,11 +126,11 @@ btint4fastcmp(Datum x, Datum y, SortSupport ssup)
109126
int32b=DatumGetInt32(y);
110127

111128
if (a>b)
112-
return1;
129+
returnA_GREATER_THAN_B;
113130
elseif (a==b)
114131
return0;
115132
else
116-
return-1;
133+
returnA_LESS_THAN_B;
117134
}
118135

119136
Datum
@@ -132,11 +149,11 @@ btint8cmp(PG_FUNCTION_ARGS)
132149
int64b=PG_GETARG_INT64(1);
133150

134151
if (a>b)
135-
PG_RETURN_INT32(1);
152+
PG_RETURN_INT32(A_GREATER_THAN_B);
136153
elseif (a==b)
137154
PG_RETURN_INT32(0);
138155
else
139-
PG_RETURN_INT32(-1);
156+
PG_RETURN_INT32(A_LESS_THAN_B);
140157
}
141158

142159
staticint
@@ -146,11 +163,11 @@ btint8fastcmp(Datum x, Datum y, SortSupport ssup)
146163
int64b=DatumGetInt64(y);
147164

148165
if (a>b)
149-
return1;
166+
returnA_GREATER_THAN_B;
150167
elseif (a==b)
151168
return0;
152169
else
153-
return-1;
170+
returnA_LESS_THAN_B;
154171
}
155172

156173
Datum
@@ -169,11 +186,11 @@ btint48cmp(PG_FUNCTION_ARGS)
169186
int64b=PG_GETARG_INT64(1);
170187

171188
if (a>b)
172-
PG_RETURN_INT32(1);
189+
PG_RETURN_INT32(A_GREATER_THAN_B);
173190
elseif (a==b)
174191
PG_RETURN_INT32(0);
175192
else
176-
PG_RETURN_INT32(-1);
193+
PG_RETURN_INT32(A_LESS_THAN_B);
177194
}
178195

179196
Datum
@@ -183,11 +200,11 @@ btint84cmp(PG_FUNCTION_ARGS)
183200
int32b=PG_GETARG_INT32(1);
184201

185202
if (a>b)
186-
PG_RETURN_INT32(1);
203+
PG_RETURN_INT32(A_GREATER_THAN_B);
187204
elseif (a==b)
188205
PG_RETURN_INT32(0);
189206
else
190-
PG_RETURN_INT32(-1);
207+
PG_RETURN_INT32(A_LESS_THAN_B);
191208
}
192209

193210
Datum
@@ -197,11 +214,11 @@ btint24cmp(PG_FUNCTION_ARGS)
197214
int32b=PG_GETARG_INT32(1);
198215

199216
if (a>b)
200-
PG_RETURN_INT32(1);
217+
PG_RETURN_INT32(A_GREATER_THAN_B);
201218
elseif (a==b)
202219
PG_RETURN_INT32(0);
203220
else
204-
PG_RETURN_INT32(-1);
221+
PG_RETURN_INT32(A_LESS_THAN_B);
205222
}
206223

207224
Datum
@@ -211,11 +228,11 @@ btint42cmp(PG_FUNCTION_ARGS)
211228
int16b=PG_GETARG_INT16(1);
212229

213230
if (a>b)
214-
PG_RETURN_INT32(1);
231+
PG_RETURN_INT32(A_GREATER_THAN_B);
215232
elseif (a==b)
216233
PG_RETURN_INT32(0);
217234
else
218-
PG_RETURN_INT32(-1);
235+
PG_RETURN_INT32(A_LESS_THAN_B);
219236
}
220237

221238
Datum
@@ -225,11 +242,11 @@ btint28cmp(PG_FUNCTION_ARGS)
225242
int64b=PG_GETARG_INT64(1);
226243

227244
if (a>b)
228-
PG_RETURN_INT32(1);
245+
PG_RETURN_INT32(A_GREATER_THAN_B);
229246
elseif (a==b)
230247
PG_RETURN_INT32(0);
231248
else
232-
PG_RETURN_INT32(-1);
249+
PG_RETURN_INT32(A_LESS_THAN_B);
233250
}
234251

235252
Datum
@@ -239,11 +256,11 @@ btint82cmp(PG_FUNCTION_ARGS)
239256
int16b=PG_GETARG_INT16(1);
240257

241258
if (a>b)
242-
PG_RETURN_INT32(1);
259+
PG_RETURN_INT32(A_GREATER_THAN_B);
243260
elseif (a==b)
244261
PG_RETURN_INT32(0);
245262
else
246-
PG_RETURN_INT32(-1);
263+
PG_RETURN_INT32(A_LESS_THAN_B);
247264
}
248265

249266
Datum
@@ -253,11 +270,11 @@ btoidcmp(PG_FUNCTION_ARGS)
253270
Oidb=PG_GETARG_OID(1);
254271

255272
if (a>b)
256-
PG_RETURN_INT32(1);
273+
PG_RETURN_INT32(A_GREATER_THAN_B);
257274
elseif (a==b)
258275
PG_RETURN_INT32(0);
259276
else
260-
PG_RETURN_INT32(-1);
277+
PG_RETURN_INT32(A_LESS_THAN_B);
261278
}
262279

263280
staticint
@@ -267,11 +284,11 @@ btoidfastcmp(Datum x, Datum y, SortSupport ssup)
267284
Oidb=DatumGetObjectId(y);
268285

269286
if (a>b)
270-
return1;
287+
returnA_GREATER_THAN_B;
271288
elseif (a==b)
272289
return0;
273290
else
274-
return-1;
291+
returnA_LESS_THAN_B;
275292
}
276293

277294
Datum
@@ -299,9 +316,9 @@ btoidvectorcmp(PG_FUNCTION_ARGS)
299316
if (a->values[i]!=b->values[i])
300317
{
301318
if (a->values[i]>b->values[i])
302-
PG_RETURN_INT32(1);
319+
PG_RETURN_INT32(A_GREATER_THAN_B);
303320
else
304-
PG_RETURN_INT32(-1);
321+
PG_RETURN_INT32(A_LESS_THAN_B);
305322
}
306323
}
307324
PG_RETURN_INT32(0);

‎src/backend/access/nbtree/nbtsearch.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ _bt_compare(Relation rel,
494494
scankey->sk_argument));
495495

496496
if (!(scankey->sk_flags&SK_BT_DESC))
497-
result=-result;
497+
INVERT_COMPARE_RESULT(result);
498498
}
499499

500500
/* if the keys are unequal, return the difference */

‎src/backend/access/nbtree/nbtutils.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ _bt_compare_array_elements(const void *a, const void *b, void *arg)
507507
cxt->collation,
508508
da,db));
509509
if (cxt->reverse)
510-
compare=-compare;
510+
INVERT_COMPARE_RESULT(compare);
511511
returncompare;
512512
}
513513

@@ -1636,7 +1636,7 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, TupleDesc tupdesc,
16361636
subkey->sk_argument));
16371637

16381638
if (subkey->sk_flags&SK_BT_DESC)
1639-
cmpresult=-cmpresult;
1639+
INVERT_COMPARE_RESULT(cmpresult);
16401640

16411641
/* Done comparing if unequal, else advance to next column */
16421642
if (cmpresult!=0)

‎src/backend/executor/nodeIndexscan.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,9 +413,10 @@ reorderqueue_cmp(const pairingheap_node *a, const pairingheap_node *b,
413413
ReorderTuple*rtb= (ReorderTuple*)b;
414414
IndexScanState*node= (IndexScanState*)arg;
415415

416-
return-cmp_orderbyvals(rta->orderbyvals,rta->orderbynulls,
417-
rtb->orderbyvals,rtb->orderbynulls,
418-
node);
416+
/* exchange argument order to invert the sort order */
417+
returncmp_orderbyvals(rtb->orderbyvals,rtb->orderbynulls,
418+
rta->orderbyvals,rta->orderbynulls,
419+
node);
419420
}
420421

421422
/*

‎src/backend/executor/nodeMergeAppend.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,10 @@ heap_compare_slots(Datum a, Datum b, void *arg)
250250
datum2,isNull2,
251251
sortKey);
252252
if (compare!=0)
253-
return-compare;
253+
{
254+
INVERT_COMPARE_RESULT(compare);
255+
returncompare;
256+
}
254257
}
255258
return0;
256259
}

‎src/bin/pg_rewind/filemap.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ final_filemap_cmp(const void *a, const void *b)
684684
return-1;
685685

686686
if (fa->action==FILE_ACTION_REMOVE)
687-
return-strcmp(fa->path,fb->path);
687+
returnstrcmp(fb->path,fa->path);
688688
else
689689
returnstrcmp(fa->path,fb->path);
690690
}

‎src/include/access/nbtree.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,8 +444,7 @@ typedef struct xl_btree_newroot
444444
*When a new operator class is declared, we require that the user
445445
*supply us with an amproc procedure (BTORDER_PROC) for determining
446446
*whether, for two keys a and b, a < b, a = b, or a > b. This routine
447-
*must return < 0, 0, > 0, respectively, in these three cases. (It must
448-
*not return INT_MIN, since we may negate the result before using it.)
447+
*must return < 0, 0, > 0, respectively, in these three cases.
449448
*
450449
*To facilitate accelerated sorting, an operator class may choose to
451450
*offer a second procedure (BTSORTSUPPORT_PROC). For full details, see

‎src/include/c.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -949,6 +949,14 @@ typedef NameData *Name;
949949
* ----------------------------------------------------------------
950950
*/
951951

952+
/*
953+
* Invert the sign of a qsort-style comparison result, ie, exchange negative
954+
* and positive integer values, being careful not to get the wrong answer
955+
* for INT_MIN. The argument should be an integral variable.
956+
*/
957+
#defineINVERT_COMPARE_RESULT(var) \
958+
((var) = ((var) < 0) ? 1 : -(var))
959+
952960
/*
953961
* Use this, not "char buf[BLCKSZ]", to declare a field or local variable
954962
* holding a page buffer, if that page might be accessed as a page and not

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp