Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2792374

Browse files
committed
Ensure that btree sort ordering functions and boolean comparison operators
give consistent results for all datatypes. Types float4, float8, and numeric were broken for NaN values; abstime, timestamp, and interval were broken for INVALID values; timetz was just plain broken (some possible pairs of values were neither < nor = nor >). Also clean up text, bpchar, varchar, and bit/varbit to eliminate duplicate code and thereby reduce the probability of similar inconsistencies arising in the future.
1 parent 77fe28f commit 2792374

File tree

13 files changed

+440
-651
lines changed

13 files changed

+440
-651
lines changed

src/backend/access/nbtree/nbtcompare.c

Lines changed: 16 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtcompare.c,v 1.41 2001/03/22 03:59:14 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtcompare.c,v 1.42 2001/05/03 19:00:36 tgl Exp $
1212
*
1313
* NOTES
1414
*
@@ -25,20 +25,32 @@
2525
* NOTE: although any negative int32 is acceptable for reporting "<",
2626
* 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.
28+
* That could overflow and give the wrong answer. Also, one should not
29+
* return INT_MIN to report "<", since some callers will negate the result.
30+
*
31+
* NOTE: it is critical that the comparison function impose a total order
32+
* on all non-NULL values of the data type, and that the datatype's
33+
* boolean comparison operators (= < >= etc) yield results consistent
34+
* with the comparison routine. Otherwise bad behavior may ensue.
35+
* (For example, the comparison operators must NOT punt when faced with
36+
* NAN or other funny values; you must devise some collation sequence for
37+
* all such values.) If the datatype is not trivial, this is most
38+
* reliably done by having the boolean operators invoke the same
39+
* three-way comparison code that the btree function does. Therefore,
40+
* this file contains only btree support for "trivial" datatypes ---
41+
* all others are in the /utils/adt/ files that implement their datatypes.
2942
*
3043
* NOTE: these routines must not leak memory, since memory allocated
3144
* during an index access won't be recovered till end of query. This
3245
* primarily affects comparison routines for toastable datatypes;
3346
* they have to be careful to free any detoasted copy of an input datum.
3447
*-------------------------------------------------------------------------
3548
*/
36-
3749
#include "postgres.h"
3850

39-
#include "utils/nabstime.h"
4051
#include "utils/builtins.h"
4152

53+
4254
Datum
4355
btboolcmp(PG_FUNCTION_ARGS)
4456
{
@@ -85,34 +97,6 @@ btint8cmp(PG_FUNCTION_ARGS)
8597
PG_RETURN_INT32(-1);
8698
}
8799

88-
Datum
89-
btfloat4cmp(PG_FUNCTION_ARGS)
90-
{
91-
float4 a = PG_GETARG_FLOAT4(0);
92-
float4 b = PG_GETARG_FLOAT4(1);
93-
94-
if (a > b)
95-
PG_RETURN_INT32(1);
96-
else if (a == b)
97-
PG_RETURN_INT32(0);
98-
else
99-
PG_RETURN_INT32(-1);
100-
}
101-
102-
Datum
103-
btfloat8cmp(PG_FUNCTION_ARGS)
104-
{
105-
float8 a = PG_GETARG_FLOAT8(0);
106-
float8 b = PG_GETARG_FLOAT8(1);
107-
108-
if (a > b)
109-
PG_RETURN_INT32(1);
110-
else if (a == b)
111-
PG_RETURN_INT32(0);
112-
else
113-
PG_RETURN_INT32(-1);
114-
}
115-
116100
Datum
117101
btoidcmp(PG_FUNCTION_ARGS)
118102
{
@@ -147,20 +131,6 @@ btoidvectorcmp(PG_FUNCTION_ARGS)
147131
PG_RETURN_INT32(0);
148132
}
149133

150-
Datum
151-
btabstimecmp(PG_FUNCTION_ARGS)
152-
{
153-
AbsoluteTime a = PG_GETARG_ABSOLUTETIME(0);
154-
AbsoluteTime b = PG_GETARG_ABSOLUTETIME(1);
155-
156-
if (AbsoluteTimeIsBefore(a, b))
157-
PG_RETURN_INT32(-1);
158-
else if (AbsoluteTimeIsBefore(b, a))
159-
PG_RETURN_INT32(1);
160-
else
161-
PG_RETURN_INT32(0);
162-
}
163-
164134
Datum
165135
btcharcmp(PG_FUNCTION_ARGS)
166136
{
@@ -179,79 +149,3 @@ btnamecmp(PG_FUNCTION_ARGS)
179149

180150
PG_RETURN_INT32(strncmp(NameStr(*a), NameStr(*b), NAMEDATALEN));
181151
}
182-
183-
Datum
184-
bttextcmp(PG_FUNCTION_ARGS)
185-
{
186-
text *a = PG_GETARG_TEXT_P(0);
187-
text *b = PG_GETARG_TEXT_P(1);
188-
int res;
189-
unsigned char *ap,
190-
*bp;
191-
192-
#ifdef USE_LOCALE
193-
int la = VARSIZE(a) - VARHDRSZ;
194-
int lb = VARSIZE(b) - VARHDRSZ;
195-
196-
ap = (unsigned char *) palloc(la + 1);
197-
bp = (unsigned char *) palloc(lb + 1);
198-
199-
memcpy(ap, VARDATA(a), la);
200-
*(ap + la) = '\0';
201-
memcpy(bp, VARDATA(b), lb);
202-
*(bp + lb) = '\0';
203-
204-
res = strcoll(ap, bp);
205-
206-
pfree(ap);
207-
pfree(bp);
208-
209-
#else
210-
int len = VARSIZE(a);
211-
212-
/* len is the length of the shorter of the two strings */
213-
if (len > VARSIZE(b))
214-
len = VARSIZE(b);
215-
216-
len -= VARHDRSZ;
217-
218-
ap = (unsigned char *) VARDATA(a);
219-
bp = (unsigned char *) VARDATA(b);
220-
221-
/*
222-
* If the two strings differ in the first len bytes, or if they're the
223-
* same in the first len bytes and they're both len bytes long, we're
224-
* done.
225-
*/
226-
227-
res = 0;
228-
if (len > 0)
229-
{
230-
do
231-
{
232-
res = (int) *ap++ - (int) *bp++;
233-
len--;
234-
} while (res == 0 && len != 0);
235-
}
236-
237-
if (res == 0 && VARSIZE(a) != VARSIZE(b))
238-
{
239-
240-
/*
241-
* The two strings are the same in the first len bytes, and they
242-
* are of different lengths.
243-
*/
244-
if (VARSIZE(a) < VARSIZE(b))
245-
res = -1;
246-
else
247-
res = 1;
248-
}
249-
250-
#endif
251-
252-
/* Avoid leaking memory when handed toasted input. */
253-
PG_FREE_IF_COPY(a, 0);
254-
PG_FREE_IF_COPY(b, 1);
255-
256-
PG_RETURN_INT32(res);
257-
}

src/backend/utils/adt/date.c

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/utils/adt/date.c,v 1.56 2001/03/22 03:59:49 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/date.c,v 1.57 2001/05/03 19:00:36 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -899,13 +899,35 @@ timetz_out(PG_FUNCTION_ARGS)
899899
}
900900

901901

902+
static int
903+
timetz_cmp_internal(TimeTzADT *time1, TimeTzADT *time2)
904+
{
905+
double t1,
906+
t2;
907+
908+
/* Primary sort is by true (GMT-equivalent) time */
909+
t1 = time1->time + time1->zone;
910+
t2 = time2->time + time2->zone;
911+
912+
if (t1 > t2)
913+
return 1;
914+
if (t1 < t2)
915+
return -1;
916+
917+
/*
918+
* If same GMT time, sort by timezone; we only want to say that two
919+
* timetz's are equal if both the time and zone parts are equal.
920+
*/
921+
return time1->zone - time2->zone;
922+
}
923+
902924
Datum
903925
timetz_eq(PG_FUNCTION_ARGS)
904926
{
905927
TimeTzADT *time1 = PG_GETARG_TIMETZADT_P(0);
906928
TimeTzADT *time2 = PG_GETARG_TIMETZADT_P(1);
907929

908-
PG_RETURN_BOOL(((time1->time + time1->zone) == (time2->time + time2->zone)));
930+
PG_RETURN_BOOL(timetz_cmp_internal(time1, time2) == 0);
909931
}
910932

911933
Datum
@@ -914,7 +936,7 @@ timetz_ne(PG_FUNCTION_ARGS)
914936
TimeTzADT *time1 = PG_GETARG_TIMETZADT_P(0);
915937
TimeTzADT *time2 = PG_GETARG_TIMETZADT_P(1);
916938

917-
PG_RETURN_BOOL(((time1->time + time1->zone) != (time2->time + time2->zone)));
939+
PG_RETURN_BOOL(timetz_cmp_internal(time1, time2) != 0);
918940
}
919941

920942
Datum
@@ -923,7 +945,7 @@ timetz_lt(PG_FUNCTION_ARGS)
923945
TimeTzADT *time1 = PG_GETARG_TIMETZADT_P(0);
924946
TimeTzADT *time2 = PG_GETARG_TIMETZADT_P(1);
925947

926-
PG_RETURN_BOOL(((time1->time + time1->zone) < (time2->time + time2->zone)));
948+
PG_RETURN_BOOL(timetz_cmp_internal(time1, time2) < 0);
927949
}
928950

929951
Datum
@@ -932,7 +954,7 @@ timetz_le(PG_FUNCTION_ARGS)
932954
TimeTzADT *time1 = PG_GETARG_TIMETZADT_P(0);
933955
TimeTzADT *time2 = PG_GETARG_TIMETZADT_P(1);
934956

935-
PG_RETURN_BOOL(((time1->time + time1->zone) <= (time2->time + time2->zone)));
957+
PG_RETURN_BOOL(timetz_cmp_internal(time1, time2) <= 0);
936958
}
937959

938960
Datum
@@ -941,7 +963,7 @@ timetz_gt(PG_FUNCTION_ARGS)
941963
TimeTzADT *time1 = PG_GETARG_TIMETZADT_P(0);
942964
TimeTzADT *time2 = PG_GETARG_TIMETZADT_P(1);
943965

944-
PG_RETURN_BOOL(((time1->time + time1->zone) > (time2->time + time2->zone)));
966+
PG_RETURN_BOOL(timetz_cmp_internal(time1, time2) > 0);
945967
}
946968

947969
Datum
@@ -950,7 +972,7 @@ timetz_ge(PG_FUNCTION_ARGS)
950972
TimeTzADT *time1 = PG_GETARG_TIMETZADT_P(0);
951973
TimeTzADT *time2 = PG_GETARG_TIMETZADT_P(1);
952974

953-
PG_RETURN_BOOL(((time1->time + time1->zone) >= (time2->time + time2->zone)));
975+
PG_RETURN_BOOL(timetz_cmp_internal(time1, time2) >= 0);
954976
}
955977

956978
Datum
@@ -959,15 +981,7 @@ timetz_cmp(PG_FUNCTION_ARGS)
959981
TimeTzADT *time1 = PG_GETARG_TIMETZADT_P(0);
960982
TimeTzADT *time2 = PG_GETARG_TIMETZADT_P(1);
961983

962-
if (DatumGetBool(DirectFunctionCall2(timetz_lt,
963-
TimeTzADTPGetDatum(time1),
964-
TimeTzADTPGetDatum(time2))))
965-
PG_RETURN_INT32(-1);
966-
if (DatumGetBool(DirectFunctionCall2(timetz_gt,
967-
TimeTzADTPGetDatum(time1),
968-
TimeTzADTPGetDatum(time2))))
969-
PG_RETURN_INT32(1);
970-
PG_RETURN_INT32(0);
984+
PG_RETURN_INT32(timetz_cmp_internal(time1, time2));
971985
}
972986

973987
/*

0 commit comments

Comments
 (0)