On Tue, Feb 2, 2016 at 1:24 PM, Robert Haas <robertmh...@gmail.com> wrote:
> On Mon, Feb 1, 2016 at 9:46 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: > > On Mon, Feb 1, 2016 at 10:34 PM, Robert Haas <robertmh...@gmail.com> > wrote: > >> On Sat, Jan 30, 2016 at 7:36 AM, Michael Paquier > >> <michael.paqu...@gmail.com> wrote: > >>> 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. > >> > >> 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. > > Oh, gosh, I should have said more clearly that I didn't really see a > need to fix this all the way back. But I guess we could. > And now there are patches. Well, nobody has complained about that until now except me... So we could live without patching back-branches, but it don't think it hurts much to fix those holes. -- Michael
