pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Adrien Nayrat <adrien.nayrat@anayrat.info>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Vik Fearing <vik.fearing@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: David Rowley <david.rowley@2ndquadrant.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: random() (was Re: New GUC to sample log queries)
Date: Fri, 28 Dec 2018 16:42:20 -0500
Message-ID: <22401.1546033340@sss.pgh.pa.us> (raw)
In-Reply-To: <10518.1545940836@sss.pgh.pa.us>
References: <20181129214850.kiq6mvofsgplqjdr@alvherre.pgsql>
	<3859.1545849900@sss.pgh.pa.us>
	<20181227013458.GF2106@paquier.xyz>
	<32696.1545875185@sss.pgh.pa.us>
	<CAH2-Wz=xJiy+BgU7M49L6EHSkMbJWbzbprge=AqyOC8aDXRj8A@mail.gmail.com>
	<2820.1545878340@sss.pgh.pa.us>
	<CAH2-WznuXsWmjnvbt=odKMVxpj1zutgzRb7s6Vu3VPXCjejS5g@mail.gmail.com>
	<9585.1545939214@sss.pgh.pa.us>
	<10518.1545940836@sss.pgh.pa.us>

I wrote:
> On further reflection, it seems likely that in most installations a lot
> of processes never invoke drandom()/setseed() at all, making such work
> in InitProcessGlobals a waste of cycles.  Probably a better idea is to
> have drandom() initialize the seed on first use, if it wasn't already
> set by setseed().

Here's a proposed patch for that.

It occurs to me that there's still another good reason to decouple
drandom from everything else: the point of setseed(), if it has any
at all, is to allow a repeatable sequence of drandom values to be
generated.  Without this patch, no such guarantee exists because of
the possibility of the backend making additional internal calls of
libc random().

			regards, tom lane

Attachments:

  [text/x-diff] decouple-drandom-from-other-random-calls-1.patch (4.2K, ../22401.1546033340@sss.pgh.pa.us/2-decouple-drandom-from-other-random-calls-1.patch)
  download | inline diff:
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 3786099..1df4356 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***************
*** 1136,1146 ****
     </table>
  
    <para>
!    The characteristics of the values returned by
!    <literal><function>random()</function></literal> depend
!    on the system implementation. It is not suitable for cryptographic
!    applications; see <xref linkend="pgcrypto"/> module for an alternative.
!    </para>
  
    <para>
     Finally, <xref linkend="functions-math-trig-table"/> shows the
--- 1136,1150 ----
     </table>
  
    <para>
!    The <function>random()</function> function uses a simple linear
!    congruential algorithm.  It is fast but not suitable for cryptographic
!    applications; see the <xref linkend="pgcrypto"/> module for an
!    alternative.
!    If <function>setseed()</function> is called, the results of
!    subsequent <function>random()</function> calls in the current session are
!    repeatable by re-issuing <function>setseed()</function> with the same
!    argument.
!   </para>
  
    <para>
     Finally, <xref linkend="functions-math-trig-table"/> shows the
diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index cf9327f..add099e 100644
*** a/src/backend/utils/adt/float.c
--- b/src/backend/utils/adt/float.c
***************
*** 22,31 ****
--- 22,34 ----
  #include "catalog/pg_type.h"
  #include "common/int.h"
  #include "libpq/pqformat.h"
+ #include "miscadmin.h"
  #include "utils/array.h"
+ #include "utils/backend_random.h"
  #include "utils/float.h"
  #include "utils/fmgrprotos.h"
  #include "utils/sortsupport.h"
+ #include "utils/timestamp.h"
  
  
  /* Configurable GUC parameter */
*************** float8		degree_c_sixty = 60.0;
*** 53,58 ****
--- 56,65 ----
  float8		degree_c_one_half = 0.5;
  float8		degree_c_one = 1.0;
  
+ /* State for drandom() and setseed() */
+ static bool drandom_seed_set = false;
+ static unsigned short drandom_seed[3] = {0, 0, 0};
+ 
  /* Local function prototypes */
  static double sind_q1(double x);
  static double cosd_q1(double x);
*************** drandom(PG_FUNCTION_ARGS)
*** 2378,2385 ****
  {
  	float8		result;
  
! 	/* result [0.0 - 1.0) */
! 	result = (double) random() / ((double) MAX_RANDOM_VALUE + 1);
  
  	PG_RETURN_FLOAT8(result);
  }
--- 2385,2414 ----
  {
  	float8		result;
  
! 	/* Initialize random seed, if not done yet in this process */
! 	if (unlikely(!drandom_seed_set))
! 	{
! 		/*
! 		 * If possible, initialize the seed using high-quality random bits.
! 		 * Should that fail for some reason, we fall back on a lower-quality
! 		 * seed based on current time and PID.
! 		 */
! 		if (!pg_backend_random((char *) drandom_seed, sizeof(drandom_seed)))
! 		{
! 			TimestampTz now = GetCurrentTimestamp();
! 			uint64		iseed;
! 
! 			/* Mix the PID with the most predictable bits of the timestamp */
! 			iseed = (uint64) now ^ ((uint64) MyProcPid << 32);
! 			drandom_seed[0] = (unsigned short) iseed;
! 			drandom_seed[1] = (unsigned short) (iseed >> 16);
! 			drandom_seed[2] = (unsigned short) (iseed >> 32);
! 		}
! 		drandom_seed_set = true;
! 	}
! 
! 	/* pg_erand48 produces desired result range [0.0 - 1.0) */
! 	result = pg_erand48(drandom_seed);
  
  	PG_RETURN_FLOAT8(result);
  }
*************** Datum
*** 2392,2404 ****
  setseed(PG_FUNCTION_ARGS)
  {
  	float8		seed = PG_GETARG_FLOAT8(0);
! 	int			iseed;
  
! 	if (seed < -1 || seed > 1)
! 		elog(ERROR, "setseed parameter %f out of range [-1,1]", seed);
  
! 	iseed = (int) (seed * MAX_RANDOM_VALUE);
! 	srandom((unsigned int) iseed);
  
  	PG_RETURN_VOID();
  }
--- 2421,2440 ----
  setseed(PG_FUNCTION_ARGS)
  {
  	float8		seed = PG_GETARG_FLOAT8(0);
! 	uint64		iseed;
  
! 	if (seed < -1 || seed > 1 || isnan(seed))
! 		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! 				 errmsg("setseed parameter %g is out of allowed range [-1,1]",
! 						seed)));
  
! 	/* Use sign bit + 47 fractional bits to fill drandom_seed[] */
! 	iseed = (int64) (seed * (float8) UINT64CONST(0x7FFFFFFFFFFF));
! 	drandom_seed[0] = (unsigned short) iseed;
! 	drandom_seed[1] = (unsigned short) (iseed >> 16);
! 	drandom_seed[2] = (unsigned short) (iseed >> 32);
! 	drandom_seed_set = true;
  
  	PG_RETURN_VOID();
  }

view thread (33+ messages)  latest in thread

Message-ID: <22401.1546033340@sss.pgh.pa.us>
Permalink:  ../22401.1546033340@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/22401.1546033340@sss.pgh.pa.us

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pg@bowt.ie, michael@paquier.xyz, alvherre@2ndquadrant.com, adrien.nayrat@anayrat.info, thomas.munro@enterprisedb.com, 9erthalion6@gmail.com, tomas.vondra@2ndquadrant.com, vik.fearing@2ndquadrant.com, robertmhaas@gmail.com, david.rowley@2ndquadrant.com
  Subject: Re: random() (was Re: New GUC to sample log queries)
  In-Reply-To: <22401.1546033340@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox