Thread: Silly bug in pgbench's random number generator

Silly bug in pgbench's random number generator

From

Gregory Stark

Date:

14 June 2007, 16:51:59

pgbench's random number generator was only generating the first and last value
in the specified range half as often as other values in the range. Not that it
actually matters but it may as well do what it claims. This line has a pretty
long and sordid history with various people tweaking it one way and another.

cvs diff: Diffing contrib/pgbench
Index: contrib/pgbench/pgbench.c
===================================================================
RCS file: /home/stark/src/REPOSITORY/pgsql/contrib/pgbench/pgbench.c,v
retrieving revision 1.66
diff -u -r1.66 pgbench.c
--- contrib/pgbench/pgbench.c    24 May 2007 18:54:10 -0000    1.66
+++ contrib/pgbench/pgbench.c    14 Jun 2007 16:22:19 -0000
@@ -191,7 +191,7 @@
 static int
 getrand(int min, int max)
 {
-    return min + (int) (((max - min) * (double) random()) / MAX_RANDOM_VALUE + 0.5);
+    return min + (int) (((max - min + 1) * (double) random()) / MAX_RANDOM_VALUE);
 }

 /* call PQexec() and exit() on failure */


--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com

Re: Silly bug in pgbench's random number generator

From

Alexey Klyukin

Date:

14 June 2007, 19:19:17

Gregory Stark wrote:
>
> pgbench's random number generator was only generating the first and last value
> in the specified range half as often as other values in the range. Not that it
> actually matters but it may as well do what it claims. This line has a pretty
> long and sordid history with various people tweaking it one way and another.
>
> cvs diff: Diffing contrib/pgbench
> Index: contrib/pgbench/pgbench.c
> ===================================================================
> RCS file: /home/stark/src/REPOSITORY/pgsql/contrib/pgbench/pgbench.c,v
> retrieving revision 1.66
> diff -u -r1.66 pgbench.c
> --- contrib/pgbench/pgbench.c    24 May 2007 18:54:10 -0000    1.66
> +++ contrib/pgbench/pgbench.c    14 Jun 2007 16:22:19 -0000
> @@ -191,7 +191,7 @@
>  static int
>  getrand(int min, int max)
>  {
> -    return min + (int) (((max - min) * (double) random()) / MAX_RANDOM_VALUE + 0.5);
> +    return min + (int) (((max - min + 1) * (double) random()) / MAX_RANDOM_VALUE);
>  }
>
>  /* call PQexec() and exit() on failure */

I think this line should be altered this way:
return min + (int) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE + 1.0));
eliminating the result of max + 1 in a corner case when random() equals to
MAX_RANDOM_VALUE.

--
Alexey Klyukin                         http://www.commandprompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Re: Silly bug in pgbench's random number generator

From

Bruce Momjian

Date:

17 July 2007, 04:50:24

This has been saved for the 8.4 release:

    http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Gregory Stark wrote:
>
> pgbench's random number generator was only generating the first and last value
> in the specified range half as often as other values in the range. Not that it
> actually matters but it may as well do what it claims. This line has a pretty
> long and sordid history with various people tweaking it one way and another.
>
> cvs diff: Diffing contrib/pgbench
> Index: contrib/pgbench/pgbench.c
> ===================================================================
> RCS file: /home/stark/src/REPOSITORY/pgsql/contrib/pgbench/pgbench.c,v
> retrieving revision 1.66
> diff -u -r1.66 pgbench.c
> --- contrib/pgbench/pgbench.c    24 May 2007 18:54:10 -0000    1.66
> +++ contrib/pgbench/pgbench.c    14 Jun 2007 16:22:19 -0000
> @@ -191,7 +191,7 @@
>  static int
>  getrand(int min, int max)
>  {
> -    return min + (int) (((max - min) * (double) random()) / MAX_RANDOM_VALUE + 0.5);
> +    return min + (int) (((max - min + 1) * (double) random()) / MAX_RANDOM_VALUE);
>  }
>
>  /* call PQexec() and exit() on failure */
>
>
> --
>   Gregory Stark
>   EnterpriseDB          http://www.enterprisedb.com
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: You can help support the PostgreSQL project by donating at
>
>                 http://www.postgresql.org/about/donate

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: Silly bug in pgbench's random number generator

From

Bruce Momjian

Date:

17 July 2007, 05:15:41

Gregory Stark wrote:
>
> Uhm, this is a one-line *bug fix*.

There wasn't agreement on how it should be fixed, it is a minor bug, and
as you said, we should be focusing on the significant outstanding
patches submitted before feature freeze.

---------------------------------------------------------------------------


>
>
> "Bruce Momjian" <bruce@momjian.us> writes:
>
> > This has been saved for the 8.4 release:
> >
> >     http://momjian.postgresql.org/cgi-bin/pgpatches_hold
> >
> > ---------------------------------------------------------------------------
> >
> > Gregory Stark wrote:
> >>
> >> pgbench's random number generator was only generating the first and last value
> >> in the specified range half as often as other values in the range. Not that it
> >> actually matters but it may as well do what it claims. This line has a pretty
> >> long and sordid history with various people tweaking it one way and another.
> >>
> >> cvs diff: Diffing contrib/pgbench
> >> Index: contrib/pgbench/pgbench.c
> >> ===================================================================
> >> RCS file: /home/stark/src/REPOSITORY/pgsql/contrib/pgbench/pgbench.c,v
> >> retrieving revision 1.66
> >> diff -u -r1.66 pgbench.c
> >> --- contrib/pgbench/pgbench.c    24 May 2007 18:54:10 -0000    1.66
> >> +++ contrib/pgbench/pgbench.c    14 Jun 2007 16:22:19 -0000
> >> @@ -191,7 +191,7 @@
> >>  static int
> >>  getrand(int min, int max)
> >>  {
> >> -    return min + (int) (((max - min) * (double) random()) / MAX_RANDOM_VALUE + 0.5);
> >> +    return min + (int) (((max - min + 1) * (double) random()) / MAX_RANDOM_VALUE);
> >>  }
> >>
> >>  /* call PQexec() and exit() on failure */
> >>
> >>
> >> --
> >>   Gregory Stark
> >>   EnterpriseDB          http://www.enterprisedb.com
> >>
> >>
> >> ---------------------------(end of broadcast)---------------------------
> >> TIP 7: You can help support the PostgreSQL project by donating at
> >>
> >>                 http://www.postgresql.org/about/donate
> >
> > --
> >   Bruce Momjian  <bruce@momjian.us>          http://momjian.us
> >   EnterpriseDB                               http://www.enterprisedb.com
> >
> >   + If your life is a hard drive, Christ can be your backup. +
>
> --
>   Gregory Stark
>   EnterpriseDB          http://www.enterprisedb.com

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: Silly bug in pgbench's random number generator

From

Gregory Stark

Date:

17 July 2007, 05:19:08

Uhm, this is a one-line *bug fix*.


"Bruce Momjian" <bruce@momjian.us> writes:

> This has been saved for the 8.4 release:
>
>     http://momjian.postgresql.org/cgi-bin/pgpatches_hold
>
> ---------------------------------------------------------------------------
>
> Gregory Stark wrote:
>>
>> pgbench's random number generator was only generating the first and last value
>> in the specified range half as often as other values in the range. Not that it
>> actually matters but it may as well do what it claims. This line has a pretty
>> long and sordid history with various people tweaking it one way and another.
>>
>> cvs diff: Diffing contrib/pgbench
>> Index: contrib/pgbench/pgbench.c
>> ===================================================================
>> RCS file: /home/stark/src/REPOSITORY/pgsql/contrib/pgbench/pgbench.c,v
>> retrieving revision 1.66
>> diff -u -r1.66 pgbench.c
>> --- contrib/pgbench/pgbench.c    24 May 2007 18:54:10 -0000    1.66
>> +++ contrib/pgbench/pgbench.c    14 Jun 2007 16:22:19 -0000
>> @@ -191,7 +191,7 @@
>>  static int
>>  getrand(int min, int max)
>>  {
>> -    return min + (int) (((max - min) * (double) random()) / MAX_RANDOM_VALUE + 0.5);
>> +    return min + (int) (((max - min + 1) * (double) random()) / MAX_RANDOM_VALUE);
>>  }
>>
>>  /* call PQexec() and exit() on failure */
>>
>>
>> --
>>   Gregory Stark
>>   EnterpriseDB          http://www.enterprisedb.com
>>
>>
>> ---------------------------(end of broadcast)---------------------------
>> TIP 7: You can help support the PostgreSQL project by donating at
>>
>>                 http://www.postgresql.org/about/donate
>
> --
>   Bruce Momjian  <bruce@momjian.us>          http://momjian.us
>   EnterpriseDB                               http://www.enterprisedb.com
>
>   + If your life is a hard drive, Christ can be your backup. +

--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com

Re: Silly bug in pgbench's random number generator

From

Tom Lane

Date:

10 March 2008, 01:23:55

Alexey Klyukin <alexk@commandprompt.com> writes:
> Gregory Stark wrote:
>> pgbench's random number generator was only generating the first and last value
>> in the specified range half as often as other values in the range.

> I think this line should be altered this way:
> return min + (int) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE + 1.0));
> eliminating the result of max + 1 in a corner case when random() equals to
> MAX_RANDOM_VALUE.

Yeah, that looks more correct.  Applied.

            regards, tom lane