Fix the seed of RandomLocalState to be 64bit instead of 32bits

This is connected to raising RandomEngine to use 64bit state, but initialization need to be done on full range otherwise entropy is limited when many repeated statement are executed. Test case provided by @taniabogatsch Fixes duckdb/duckdb-rs#331 Fixes marcboeker/go-duckdb#339
carlopi · Dec 27, 2024 · 6a5cd79 · 6a5cd79
1 parent ab8c909
commit 6a5cd79
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 11 deletions.
diff --git a/extension/core_functions/scalar/random/random.cpp b/extension/core_functions/scalar/random/random.cpp
@@ -9,7 +9,7 @@
 namespace duckdb {
 
 struct RandomLocalState : public FunctionLocalState {
-	explicit RandomLocalState(uint32_t seed) : random_engine(seed) {
+	explicit RandomLocalState(uint64_t seed) : random_engine(seed) {
 	}
 
 	RandomEngine random_engine;
@@ -30,7 +30,7 @@ static unique_ptr<FunctionLocalState> RandomInitLocalState(ExpressionState &stat
                                                            FunctionData *bind_data) {
 	auto &random_engine = RandomEngine::Get(state.GetContext());
 	lock_guard<mutex> guard(random_engine.lock);
-	return make_uniq<RandomLocalState>(random_engine.NextRandomInteger());
+	return make_uniq<RandomLocalState>(random_engine.NextRandomInteger64());
 }
 
 ScalarFunction RandomFun::GetFunction() {

diff --git a/src/common/random_engine.cpp b/src/common/random_engine.cpp
@@ -12,12 +12,12 @@ struct RandomState {
 	pcg32 pcg;
 };
 
-RandomEngine::RandomEngine(int64_t seed) : random_state(make_uniq<RandomState>()) {
-	if (seed < 0) {
-		random_state->pcg.seed(pcg_extras::seed_seq_from<std::random_device>());
-	} else {
-		random_state->pcg.seed(NumericCast<uint64_t>(seed));
-	}
+RandomEngine::RandomEngine() : random_state(make_uniq<RandomState>()) {
+	random_state->pcg.seed(pcg_extras::seed_seq_from<std::random_device>());
+}
+
+RandomEngine::RandomEngine(uint64_t seed) : random_state(make_uniq<RandomState>()) {
+	random_state->pcg.seed(seed);
 }
 
 RandomEngine::~RandomEngine() {
@@ -59,7 +59,7 @@ uint32_t RandomEngine::NextRandomInteger32(uint32_t min, uint32_t max) {
 	return min + static_cast<uint32_t>(NextRandom32() * double(max - min));
 }
 
-void RandomEngine::SetSeed(uint32_t seed) {
+void RandomEngine::SetSeed(uint64_t seed) {
 	random_state->pcg.seed(seed);
 }
 

diff --git a/src/include/duckdb/common/random_engine.hpp b/src/include/duckdb/common/random_engine.hpp
@@ -20,7 +20,8 @@ struct RandomState;
 
 class RandomEngine {
 public:
-	explicit RandomEngine(int64_t seed = -1);
+	explicit RandomEngine(uint64_t seed);
+	explicit RandomEngine();
 	~RandomEngine();
 
 	//! Generate a random number between min and max
@@ -36,7 +37,7 @@ class RandomEngine {
 	uint32_t NextRandomInteger(uint32_t min, uint32_t max);
 	uint64_t NextRandomInteger64();
 
-	void SetSeed(uint32_t seed);
+	void SetSeed(uint64_t seed);
 
 	static RandomEngine &Get(ClientContext &context);
 

diff --git a/test/sql/function/numeric/test_random_loop.test_slow b/test/sql/function/numeric/test_random_loop.test_slow
@@ -0,0 +1,38 @@
+# name: test/sql/function/numeric/test_random_loop.test_slow
+# description: Test random via repeated inserts
+# group: [numeric]
+
+statement ok
+CREATE TABLE test (id UUID DEFAULT gen_random_uuid());
+
+concurrentloop i 0 20
+
+loop j 0 10000
+
+statement ok
+INSERT INTO test DEFAULT VALUES;
+
+endloop
+
+endloop
+
+query I
+SELECT COUNT(DISTINCT id) FROM test;
+----
+200000
+
+statement ok
+CREATE TABLE test_pk (id UUID PRIMARY KEY DEFAULT gen_random_uuid());
+
+concurrentloop i 0 20
+
+loop j 0 10000
+
+statement ok
+INSERT INTO test_pk DEFAULT VALUES;
+
+endloop
+
+endloop
+
+