On Mon, Feb 1, 2016 at 10:34 PM, Robert Haas <[email protected]> wrote:
> On Sat, Jan 30, 2016 at 7:36 AM, Michael Paquier
> <[email protected]> wrote:
>> On Fri, Jan 29, 2016 at 11:21 PM, Fabien COELHO <[email protected]> wrote:
>>> + /* overflow check (needed for INT64_MIN) */
>>> + if (lval != 0 && (*retval < 0 == lval < 0))
>>>
>>> Why not use "if (lval == INT64_MIN)" instead of this complicated condition?
>>> If it is really needed for some reason, I think that a comment could help.
>>
>> Checking for PG_INT64_MIN only would be fine as well, so let's do so.
>> I thought honestly that we had better check if the result and the left
>> argument are not of the same sign, but well.
>
> Committed and back-patched to 9.5. Doesn't apply further back.
OK, here are patches for 9.1~9.4. The main differences are that in
9.3/9.4 int64 is used for the division operations, and in 9.2/9.1
that's int32. In the latter case pgbench blows up the same way with
that:
\set i -2147483648
\set i :i / -1
select :i;
In those patches INT32_MIN/INT64_MIN need to be explicitly set as well
at the top of pgbench.c. I thing that's fine.
--
Michael
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 2111f16..84b6303 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -56,6 +56,10 @@
#ifndef INT64_MAX
#define INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#endif
+#ifndef INT32_MIN
+#define INT32_MIN (-0x7FFFFFFF-1)
+#endif
+
/*
* Multi-platform pthread implementations
@@ -1152,13 +1156,37 @@ top:
snprintf(res, sizeof(res), "%d", ope1 * ope2);
else if (strcmp(argv[3], "/") == 0)
{
+ int operes;
+
if (ope2 == 0)
{
fprintf(stderr, "%s: division by zero\n", argv[0]);
st->ecnt++;
return true;
}
- snprintf(res, sizeof(res), "%d", ope1 / ope2);
+ /*
+ * INT32_MIN / -1 is problematic, since the result can't
+ * be represented on a two's-complement machine. Some
+ * machines produce INT32_MIN, some produce zero, some
+ * throw an exception. We can dodge the problem by
+ * recognizing that division by -1 is the same as
+ * negation.
+ */
+ if (ope2 == -1)
+ {
+ operes = -ope1;
+
+ /* overflow check (needed for INT32_MIN) */
+ if (ope1 == INT32_MIN)
+ {
+ fprintf(stderr, "integer out of range\n");
+ st->ecnt++;
+ return true;
+ }
+ }
+ else
+ operes = ope1 / ope2;
+ snprintf(res, sizeof(res), "%d", operes);
}
else
{
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 4e22695..062a32f 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -57,6 +57,10 @@
#ifndef INT64_MAX
#define INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#endif
+#ifndef INT64_MIN
+#define INT64_MIN (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
+#endif
+
/*
* Multi-platform pthread implementations
@@ -1331,13 +1335,37 @@ top:
snprintf(res, sizeof(res), INT64_FORMAT, ope1 * ope2);
else if (strcmp(argv[3], "/") == 0)
{
+ int64 operes;
+
if (ope2 == 0)
{
fprintf(stderr, "%s: division by zero\n", argv[0]);
st->ecnt++;
return true;
}
- snprintf(res, sizeof(res), INT64_FORMAT, ope1 / ope2);
+ /*
+ * INT64_MIN / -1 is problematic, since the result can't
+ * be represented on a two's-complement machine. Some
+ * machines produce INT64_MIN, some produce zero, some
+ * throw an exception. We can dodge the problem by
+ * recognizing that division by -1 is the same as
+ * negation.
+ */
+ if (ope2 == -1)
+ {
+ operes = -ope1;
+
+ /* overflow check (needed for INT64_MIN) */
+ if (ope1 == INT64_MIN)
+ {
+ fprintf(stderr, "bigint out of range\n");
+ st->ecnt++;
+ return true;
+ }
+ }
+ else
+ operes = ope1 / ope2;
+ snprintf(res, sizeof(res), INT64_FORMAT, operes);
}
else
{
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
