Thread: Rejection of the smallest int8
Attached is a patch to accept the smallest value of int8.
The smallest value -9223372036854775808 is rejected as follows:
test=# create table test_int8 (val int8);
CREATE
test=# insert into test_int8 values (-9223372036854775807);
INSERT 4026531936 1
test=# insert into test_int8 values (-9223372036854775808);
ERROR: int8 value out of range: "-9223372036854775808"
test=#
Index: src/backend/utils/adt/int8.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/int8.c,v
retrieving revision 1.35
diff -u -3 -p -r1.35 int8.c
--- src/backend/utils/adt/int8.c 2001/10/25 14:10:06 1.35
+++ src/backend/utils/adt/int8.c 2001/11/21 05:35:25
@@ -28,6 +28,12 @@
#define MAXINT8LEN 25
+#ifndef INT64_MAX
+#define INT64_MAX (0x7FFFFFFFFFFFFFFFLL)
+#endif
+#ifndef INT64_MIN
+#define INT64_MIN (-INT64_MAX-1)
+#endif
#ifndef INT_MAX
#define INT_MAX (0x7FFFFFFFL)
#endif
@@ -77,7 +83,7 @@ int8in(PG_FUNCTION_ARGS)
elog(ERROR, "Bad int8 external representation \"%s\"", str);
while (*ptr && isdigit((unsigned char) *ptr)) /* process digits */
{
- int64 newtmp = tmp * 10 + (*ptr++ - '0');
+ int64 newtmp = tmp * 10 - (*ptr++ - '0');
if ((newtmp / 10) != tmp) /* overflow? */
elog(ERROR, "int8 value out of range: \"%s\"", str);
@@ -86,7 +92,13 @@ int8in(PG_FUNCTION_ARGS)
if (*ptr) /* trailing junk? */
elog(ERROR, "Bad int8 external representation \"%s\"", str);
- result = (sign < 0) ? -tmp : tmp;
+ if (sign < 0) {
+ result = tmp;
+ } else {
+ if (tmp == INT64_MIN)
+ elog(ERROR, "int8 value out of range: \"%s\"", str);
+ result = -tmp;
+ }
PG_RETURN_INT64(result);
}
sugita@sra.co.jp writes:
> Attached is a patch to accept the smallest value of int8.
This has been proposed before. The problem with it is that it's
not portable: the C standard does not specify the direction of rounding
of integer division when the dividend is negative. So the test
inside the loop that tries to detect overflow would be likely to fail
on some machines.
If you can see a way around that, we're all ears ...
regards, tom lane
I said:
>> Attached is a patch to accept the smallest value of int8.
> This has been proposed before. The problem with it is that it's
> not portable: the C standard does not specify the direction of rounding
> of integer division when the dividend is negative.
BTW, does anyone have a copy of the ANSI C standard to check this?
I have a draft of C99, which says that truncation is towards 0
regardless of the sign, but I think that this is something that was
tightened up in C99; we can't rely on older compilers to follow it.
regards, tom lane
I said:
> If you can see a way around that, we're all ears ...
Of course there's always the brute-force solution:
if (strcmp(ptr, "-9223372036854775808") == 0)
return -9223372036854775808;
else
<<proceed with int8in>>
(modulo some #ifdef hacking to attach the correct L or LL suffix to the
constant, but you get the idea)
This qualifies as pretty durn ugly, but might indeed be more portable
than any other alternative. Comments?
regards, tom lane
Peter Eisentraut <peter_e@gmx.net> writes:
> Use strtoll/strtoull if available. They should be on "most" systems
> anyway.
Mph. The reason int8in is coded the way it is is to avoid having to
deal with strtoll configuration (does it exist? Is it the right thing?
Don't forget Alphas, where int8 == long). We'd still need a fallback
if it doesn't exist, so I'm not that excited about this answer.
regards, tom lane
Tom Lane writes: > This has been proposed before. The problem with it is that it's > not portable: the C standard does not specify the direction of rounding > of integer division when the dividend is negative. So the test > inside the loop that tries to detect overflow would be likely to fail > on some machines. > > If you can see a way around that, we're all ears ... Use strtoll/strtoull if available. They should be on "most" systems anyway. -- Peter Eisentraut peter_e@gmx.net
From: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: [PATCHES] Rejection of the smallest int8
Date: Wed, 21 Nov 2001 12:54:31 -0500
;;; I said:
;;; > If you can see a way around that, we're all ears ...
;;;
;;; Of course there's always the brute-force solution:
;;;
;;; if (strcmp(ptr, "-9223372036854775808") == 0)
;;; return -9223372036854775808;
;;; else
;;; <<proceed with int8in>>
;;;
;;; (modulo some #ifdef hacking to attach the correct L or LL suffix to the
;;; constant, but you get the idea)
;;;
;;; This qualifies as pretty durn ugly, but might indeed be more portable
;;; than any other alternative. Comments?
I made a new patch. Toward zero fault is fixed.
Kind regards,
Kenji Sugita
sugita@sra.co.jp
Index: int8.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/int8.c,v
retrieving revision 1.35
diff -u -3 -p -r1.35 int8.c
--- int8.c 2001/10/25 14:10:06 1.35
+++ int8.c 2001/11/22 08:48:09
@@ -28,6 +28,12 @@
#define MAXINT8LEN 25
+#ifndef INT64_MAX
+#define INT64_MAX (0x7FFFFFFFFFFFFFFFLL)
+#endif
+#ifndef INT64_MIN
+#define INT64_MIN (-INT64_MAX-1)
+#endif
#ifndef INT_MAX
#define INT_MAX (0x7FFFFFFFL)
#endif
@@ -62,6 +68,7 @@ int8in(PG_FUNCTION_ARGS)
char *ptr = str;
int64 tmp = 0;
int sign = 1;
+ int64 limit;
/*
* Do our own scan, rather than relying on sscanf which might be
@@ -75,18 +82,26 @@ int8in(PG_FUNCTION_ARGS)
ptr++;
if (!isdigit((unsigned char) *ptr)) /* require at least one digit */
elog(ERROR, "Bad int8 external representation \"%s\"", str);
+ if (sign < 0)
+ limit = INT64_MIN;
+ else
+ limit = -INT64_MAX;
while (*ptr && isdigit((unsigned char) *ptr)) /* process digits */
{
- int64 newtmp = tmp * 10 + (*ptr++ - '0');
+ int digit;
- if ((newtmp / 10) != tmp) /* overflow? */
+ if (tmp < -(INT64_MAX/10)) /* overflow? */
+ elog(ERROR, "int8 value out of range: \"%s\"", str);
+ digit = *ptr++ - '0';
+ tmp *= 10;
+ if (tmp < limit + digit)
elog(ERROR, "int8 value out of range: \"%s\"", str);
- tmp = newtmp;
+ tmp -= digit;
}
if (*ptr) /* trailing junk? */
elog(ERROR, "Bad int8 external representation \"%s\"", str);
- result = (sign < 0) ? -tmp : tmp;
+ result = (sign < 0) ? tmp : -tmp;
PG_RETURN_INT64(result);
}