Lists: | pgsql-hackers |
---|
From: | Xi Wang <xi(dot)wang(at)gmail(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Cc: | Xi Wang <xi(dot)wang(at)gmail(dot)com> |
Subject: | [PATCH] Fix INT_MIN % -1 overflow in int8mod(). |
Date: | 2012-11-14 20:44:12 |
Message-ID: | 1352925852-14714-1-git-send-email-xi.wang@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Return 0 for INT_MIN % -1 (64-bit) instead of throwing an exception.
This patch complements commit f9ac414c that fixed int4mod().
---
src/backend/utils/adt/int8.c | 4 ++++
src/include/c.h | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 0e59956..9da651b 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -649,6 +649,10 @@ int8mod(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
}
+ /* SELECT ((-9223372036854775808)::int8) % (-1); causes a floating point exception */
+ if (arg1 == INT64_MIN && arg2 == -1)
+ PG_RETURN_INT64(0);
+
/* No overflow is possible */
PG_RETURN_INT64(arg1 % arg2);
diff --git a/src/include/c.h b/src/include/c.h
index a6c0e6e..d20ba8c 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -294,6 +294,13 @@ typedef unsigned long long int uint64;
#define UINT64CONST(x) ((uint64) x)
#endif
+#ifndef INT64_MAX
+#define INT64_MAX INT64CONST(9223372036854775807)
+#endif
+
+#ifndef INT64_MIN
+#define INT64_MIN (-INT64_MAX-1)
+#endif
/* Select timestamp representation (float8 or int64) */
#ifdef USE_INTEGER_DATETIMES
--
1.7.10.4
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Xi Wang <xi(dot)wang(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Fix INT_MIN % -1 overflow in int8mod(). |
Date: | 2012-11-14 21:46:38 |
Message-ID: | 26733.1352929598@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Xi Wang <xi(dot)wang(at)gmail(dot)com> writes:
> Return 0 for INT_MIN % -1 (64-bit) instead of throwing an exception.
> This patch complements commit f9ac414c that fixed int4mod().
Meh. I didn't care for the explicit dependency on INT_MIN in the
previous patch, and I like introducing INT64_MIN even less. I think
we should be able to reduce the test to just
if (arg2 == -1)
return 0;
since zero is the correct result for any value of arg1, not only
INT_MIN.
regards, tom lane
From: | Xi Wang <xi(dot)wang(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Fix INT_MIN % -1 overflow in int8mod(). |
Date: | 2012-11-14 21:54:46 |
Message-ID: | 50A41326.6040309@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 11/14/12 4:46 PM, Tom Lane wrote:
> Meh. I didn't care for the explicit dependency on INT_MIN in the
> previous patch, and I like introducing INT64_MIN even less. I think
> we should be able to reduce the test to just
>
> if (arg2 == -1)
> return 0;
>
> since zero is the correct result for any value of arg1, not only
> INT_MIN.
I agree. Will send a v2. Thanks. :)
- xi
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Xi Wang <xi(dot)wang(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Fix INT_MIN % -1 overflow in int8mod(). |
Date: | 2012-11-14 22:03:07 |
Message-ID: | 27071.1352930587@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Xi Wang <xi(dot)wang(at)gmail(dot)com> writes:
> I agree. Will send a v2. Thanks. :)
No need, I'm already patching it.
regards, tom lane
From: | Xi Wang <xi(dot)wang(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | [PATCH 1/2] Fix INT_MIN % -1 overflow in int8mod(). |
Date: | 2012-11-14 22:05:28 |
Message-ID: | 50A415A8.2060105@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Return 0 for x % -1 instead of throwing an exception (e.g., when x
is INT_MIN).
Suggested by Tom Lane.
---
src/backend/utils/adt/int8.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 0e59956..a30ab36 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -649,6 +649,10 @@ int8mod(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
}
+ /* SELECT ((-9223372036854775808)::int8) % (-1); causes a floating point exception */
+ if (arg2 == -1)
+ PG_RETURN_INT64(0);
+
/* No overflow is possible */
PG_RETURN_INT64(arg1 % arg2);
--
1.7.10.4
From: | Xi Wang <xi(dot)wang(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | [PATCH 2/2] Clean up INT_MIN % -1 overflow in int4mod(). |
Date: | 2012-11-14 22:07:28 |
Message-ID: | 50A41620.5040606@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Since x % -1 returns 0 for any x, we don't need the check x == INT_MIN.
Suggested by Tom Lane.
---
src/backend/utils/adt/int.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c
index 4be3901..3e423fe 100644
--- a/src/backend/utils/adt/int.c
+++ b/src/backend/utils/adt/int.c
@@ -1096,7 +1096,7 @@ int4mod(PG_FUNCTION_ARGS)
}
/* SELECT ((-2147483648)::int4) % (-1); causes a floating point exception */
- if (arg1 == INT_MIN && arg2 == -1)
+ if (arg2 == -1)
PG_RETURN_INT32(0);
/* No overflow is possible */
--
1.7.10.4
From: | Xi Wang <xi(dot)wang(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Fix INT_MIN % -1 overflow in int8mod(). |
Date: | 2012-11-14 22:08:52 |
Message-ID: | 50A41674.2010800@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 11/14/12 5:03 PM, Tom Lane wrote:
> No need, I'm already patching it.
Oops. Sorry. Ignore my patches.
- xi