[SPARK-56573][SQL] Use a full-range non-negative random seed for unseeded sampling - #57732
Open
stanyao wants to merge 1 commit into
Open
[SPARK-56573][SQL] Use a full-range non-negative random seed for unseeded sampling#57732stanyao wants to merge 1 commit into
stanyao wants to merge 1 commit into
Conversation
…eded sampling Sample seeds generated when the user did not specify one were drawn from `(math.random() * 1000).toLong`, limiting Spark to at most 1000 distinct samples of any table. Replace both call sites with a shared `Sample.resolveSeed` helper that draws a non-negative 63-bit seed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
When a
Samplehas no user-specified seed, Spark generates one with(math.random() * 1000).toLong. This PR replaces both call sites with a sharedSample.resolveSeedhelper that returns a non-negative 63-bit seed viaUtils.random.nextLong() & Long.MaxValue.The seed is generated in two places --
SampleExec.resolvedSeedandV2ScanRelationPushDown.pushDownSample-- which SPARK-56392 duplicated when it moved the expression out ofAstBuilder. Both now delegate to one helper so they cannot drift apart.Generated seeds must be non-negative: a pushed-down sample renders its seed into SQL as
REPEATABLE (<seed>), and the seed in that grammar accepts no sign. A user-specified seed passes through unchanged, negative values included.Why are the changes needed?
A 1000-value seed space means Spark can produce at most 1000 distinct samples of a table regardless of its size. Two unseeded samples are identical 0.1% of the time; among ~37 samples the odds of some pair colliding exceed 50%; 1000 bootstrap resamples yield only ~632 distinct samples.
This is not a regression -- the expression dates to SPARK-1251 (2014), and SPARK-56392 relocated it verbatim. But
TABLESAMPLE SYSTEMrejects theREPEATABLEclause, so block-sampling users cannot pin a seed and always take this path, making the generated seed their only source of variation.TABLESAMPLEwas the last sampling path in Spark still using a narrow seed. PySpark (random.randint(0, sys.maxsize)) and the RDD APIs (Utils.random.nextLong) already draw from the full range, so this removes an outlier rather than introducing a new convention.Separately considered and intentionally left out of scope:
SampleExec.resolvedSeedis a non-constructorval, so structurally identicalSampleExecnodes canonicalize equal while holding different seeds. Whether plan reuse can collapse two independent samples into one is an independence question that seed width does not address, and it warrants its own JIRA.Does this PR introduce any user-facing change?
No behavior change users can depend on. Unseeded sampling was already nondeterministic; it now draws from a much larger seed space. Explicitly seeded sampling (
REPEATABLE(n),sample(fraction, seed)) is unaffected.TABLESAMPLE SYSTEMis unreleased, so no released behavior changes there.How was this patch tested?
New
SampleSuitecovering seed passthrough (including negative user-specified seeds), non-negativity of generated seeds, and distinctness across 10000 draws.Existing suites were run across catalyst, sql/core and connect:
PlanParserSuite,DataSourceV2TableSampleSuite,JDBCV2Suite,JDBCSuite,BasicStatsEstimationSuite,SparkConnectProtoSuite,DataFrameSuite,DatasetSuite,SQLQuerySuite,ColumnPruningSuite,CollapseProjectSuite,NestedColumnAliasingSuite,UnsupportedOperationsSuite,AnalysisErrorSuiteand others -- 2302 tests passing.Was this patch authored or co-authored using generative AI tooling?
Co-authored-by: Claude Code (Claude Opus 5) with thorough human review and iterations