Rejection of the smallest int8

Started by Nonameabout 24 years ago7 messages
#1Noname
sugita@sra.co.jp
1 attachment(s)

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=#

Attachments:

smallest-int8.patchtext/plain; charset=us-asciiDownload
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);
 }
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noname (#1)
Re: Rejection of the smallest int8

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
Re: Rejection of the smallest int8

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
Re: Rejection of the smallest int8

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

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#2)
Re: Rejection of the smallest int8

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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#5)
Re: Rejection of the smallest int8

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

#7Noname
sugita@sra.co.jp
In reply to: Tom Lane (#4)
1 attachment(s)
Re: Rejection of the smallest int8

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

Attachments:

smallest-int8.patchtext/plain; charset=us-asciiDownload
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);
 }