On Fri, Jan 29, 2016 at 11:21 PM, Fabien COELHO <coe...@cri.ensmp.fr> 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.
--
Michael
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d5f242c..6a17990 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -967,7 +967,28 @@ evaluateExpr(CState *st, PgBenchExpr *expr, int64 *retval)
fprintf(stderr, "division by zero\n");
return false;
}
- *retval = lval / rval;
+ /*
+ * 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 (rval == -1)
+ {
+ *retval = -lval;
+
+ /* overflow check (needed for INT64_MIN) */
+ if (lval == PG_INT64_MIN)
+ {
+ fprintf(stderr, "bigint out of range\n");
+ return false;
+ }
+ }
+ else
+ *retval = lval / rval;
+
return true;
case '%':
@@ -976,7 +997,15 @@ evaluateExpr(CState *st, PgBenchExpr *expr, int64 *retval)
fprintf(stderr, "division by zero\n");
return false;
}
- *retval = lval % rval;
+ /*
+ * Some machines throw a floating-point exception
+ * for INT64_MIN % -1, the correct answer being
+ * zero in any case.
+ */
+ if (rval == -1)
+ *retval = 0;
+ else
+ *retval = lval % rval;
return true;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
