stanyao opened a new pull request, #57732: URL: https://github.com/apache/spark/pull/57732
### What changes were proposed in this pull request? When a `Sample` has no user-specified seed, Spark generates one with `(math.random() * 1000).toLong`. This PR replaces both call sites with a shared `Sample.resolveSeed` helper that returns a non-negative 63-bit seed via `Utils.random.nextLong() & Long.MaxValue`. The seed is generated in two places -- `SampleExec.resolvedSeed` and `V2ScanRelationPushDown.pushDownSample` -- which SPARK-56392 duplicated when it moved the expression out of `AstBuilder`. 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 SYSTEM` rejects the `REPEATABLE` clause, so block-sampling users cannot pin a seed and always take this path, making the generated seed their only source of variation. `TABLESAMPLE` was 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.resolvedSeed` is a non-constructor `val`, so structurally identical `SampleExec` nodes 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 SYSTEM` is unreleased, so no released behavior changes there. ### How was this patch tested? New `SampleSuite` covering 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`, `AnalysisErrorSuite` and others -- 2302 tests passing. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
