Skip to content

Conversation

Mytherin
Copy link
Collaborator

Fixes #13867

Previously we were generating 32-bit random numbers for random(), and using that to return a double. This PR modifies this behavior so that we instead generate a 64-bit random number. This ensures we don't run into repeated numbers when generating a low amount of random numbers. Note that this changes the random numbers that are generated with specific seeds, hence this requires changing a number of tests.

@Mytherin Mytherin merged commit aaad212 into duckdb:feature Sep 13, 2024
39 checks passed
@Mytherin Mytherin deleted the morerandombits branch December 8, 2024 06:53
carlopi added a commit to carlopi/duckdb that referenced this pull request Dec 27, 2024
This is connected to raising RandomEngine to use 64bit state, see duckdb#13920, but
initialization need to be done on full range otherwise entropy is limited when many repeated statement are executed.

Note that here we only solve the problem for RandomLocalState.
A wider rework of the RandomEngine API migth be also handy.

Test case provided by @taniabogatsch

Fixes duckdb/duckdb-rs#331
Fixes marcboeker/go-duckdb#339

Note that impe
carlopi added a commit to carlopi/duckdb that referenced this pull request Dec 28, 2024
This is connected to raising RandomEngine to use 64bit state, see duckdb#13920, but
initialization need to be done on full range otherwise entropy is limited when many repeated statement are executed.

Note that here we only solve the problem for RandomLocalState.
A wider rework of the RandomEngine API migth be also handy.

Test case provided by @taniabogatsch

Fixes duckdb/duckdb-rs#331
Fixes marcboeker/go-duckdb#339
hannes added a commit that referenced this pull request Dec 30, 2024
This is connected to raising RandomEngine to use 64bit state, see
#13920, but initialization need to
be done on full range otherwise entropy is limited when many repeated
statement are executed.

Note that here we only solve the problem for RandomLocalState.
A wider rework of the RandomEngine API would also be also handy, since
currently it takes a int64_t that can be either invalid (-1) or valid
(>=0), but out of scope here given interactions with sampling PRs make
it a larger change.

Test case by @taniabogatsch

Fixes duckdb/duckdb-rs#331
Fixes marcboeker/go-duckdb#339
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants