[SPARK-55978][SQL] Add TABLESAMPLE SYSTEM block sampling with DSv2 pushdown by stanyao · Pull Request #54972 · apache/spark

stanyao · 2026-03-24T00:29:16Z

What changes were proposed in this pull request?

This PR adds support for ANSI SQL TABLESAMPLE SYSTEM (block-level sampling) alongside the existing TABLESAMPLE BERNOULLI (row-level sampling). Key changes:

SQL grammar: Extended TABLESAMPLE to accept an optional SYSTEM or BERNOULLI qualifier before the sample method. Added both as non-reserved keywords.
Logical plan: Introduced SampleMethod sealed trait (Bernoulli/System) and added it to the Sample node. Default is Bernoulli for backward compatibility.
Parser: TABLESAMPLE SYSTEM only supports PERCENT sampling and does not support REPEATABLE. Other methods (ROWS, BUCKET, BYTES) are rejected with clear error messages.
DSv2 pushdown: TABLESAMPLE SYSTEM is pushed down to data sources via an extended SupportsPushDownTableSample.pushTableSample() overload with isSystemSampling flag. Sources that don't override the new method reject SYSTEM sampling by default.
Physical planning: SYSTEM samples that aren't pushed down to a DSv2 source raise an AnalysisException — there is no row-level fallback since block sampling is data-source dependent.

Why are the changes needed?

ANSI SQL defines two sampling methods: BERNOULLI (row-level) and SYSTEM (implementation-dependent, typically block/split-level). Block sampling is significantly faster for large tables since it avoids per-row evaluation, making it useful for approximate queries and data exploration. Many databases (PostgreSQL, Hive, Trino) support this distinction.

Does this PR introduce any user-facing change?

Yes. New SQL syntax TABLESAMPLE SYSTEM (x PERCENT) and TABLESAMPLE BERNOULLI (x PERCENT). BERNOULLI and SYSTEM are added as non-reserved keywords. Existing queries without these keywords behave identically to before.

How was this patch tested?

9 new test cases in PlanParserSuite covering: basic parsing, case insensitivity, boundary fractions, unsupported methods (ROWS/BUCKET with SYSTEM), REPEATABLE rejection, fraction validation, identifier preservation, and subquery contexts.
Existing SQLQuerySuite tests pass.
Scalastyle passes with 0 errors/warnings.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (thoroughly refined, reviewed, and tested by human)

dbtsai · 2026-03-26T19:01:14Z

cc @szehon-ho

stanyao · 2026-03-28T17:18:54Z

Rebased and resolved the new merge conflict above.

szehon-ho

Thanks, the idea seems reasonable to me in DSV2 API. Added some comments

Summary: CONTEXT: Review feedback on PR apache#54972 for TABLESAMPLE SYSTEM block sampling. WHAT: - Add DSv2 SampleMethod Java enum to replace boolean isSystemSampling parameter - Update SupportsPushDownTableSample Javadoc to clarify BERNOULLI vs SYSTEM - Replace _LEGACY_ERROR_TEMP_0035 with UNSUPPORTED_FEATURE.TABLESAMPLE_SYSTEM - Restrict SYSTEM sample pushdown to direct table scans only (no subqueries/views) - Remove findScanBuilderHolder that incorrectly traversed through Filter/Project Test Plan: Existing tests. Error class added to error-conditions.json.

szehon-ho

Thanks, leaving a few more comments

Summary: CONTEXT: Addressing code review feedback on PR apache#54972. WHAT: - Use PhysicalOperation extractor in pushDownSample for System sampling, consistent with other pushdown rules - Remove unnecessary SubqueryAlias match (eliminated before this rule runs) - Guard pushDownJoin against composing with pushed samples to prevent silent sample loss - Use user-friendly error message for TABLESAMPLE_SYSTEM - Include sample method in EXPLAIN pushed sample metadata - Add 'as x' alias to BERNOULLI REPEATABLE parser test for consistency Test Plan: Existing tests. No golden file or EXPLAIN string assertions affected.

Summary: CONTEXT: Review feedback on PR apache#54972 for TABLESAMPLE SYSTEM block sampling. WHAT: - Add DSv2 SampleMethod Java enum to replace boolean isSystemSampling parameter - Update SupportsPushDownTableSample Javadoc to clarify BERNOULLI vs SYSTEM - Replace _LEGACY_ERROR_TEMP_0035 with UNSUPPORTED_FEATURE.TABLESAMPLE_SYSTEM - Restrict SYSTEM sample pushdown to direct table scans only (no subqueries/views) - Remove findScanBuilderHolder that incorrectly traversed through Filter/Project Test Plan: Existing tests. Error class added to error-conditions.json.

Summary: CONTEXT: Addressing code review feedback on PR apache#54972. WHAT: - Use PhysicalOperation extractor in pushDownSample for System sampling, consistent with other pushdown rules - Remove unnecessary SubqueryAlias match (eliminated before this rule runs) - Guard pushDownJoin against composing with pushed samples to prevent silent sample loss - Use user-friendly error message for TABLESAMPLE_SYSTEM - Include sample method in EXPLAIN pushed sample metadata - Add 'as x' alias to BERNOULLI REPEATABLE parser test for consistency Test Plan: Existing tests. No golden file or EXPLAIN string assertions affected.

Summary: CONTEXT: Review feedback on PR apache#54972 for TABLESAMPLE SYSTEM block sampling. WHAT: - Add DSv2 SampleMethod Java enum to replace boolean isSystemSampling parameter - Update SupportsPushDownTableSample Javadoc to clarify BERNOULLI vs SYSTEM - Replace _LEGACY_ERROR_TEMP_0035 with UNSUPPORTED_FEATURE.TABLESAMPLE_SYSTEM - Restrict SYSTEM sample pushdown to direct table scans only (no subqueries/views) - Remove findScanBuilderHolder that incorrectly traversed through Filter/Project Test Plan: Existing tests. Error class added to error-conditions.json.

Summary: CONTEXT: Addressing code review feedback on PR apache#54972. WHAT: - Use PhysicalOperation extractor in pushDownSample for System sampling, consistent with other pushdown rules - Remove unnecessary SubqueryAlias match (eliminated before this rule runs) - Guard pushDownJoin against composing with pushed samples to prevent silent sample loss - Use user-friendly error message for TABLESAMPLE_SYSTEM - Include sample method in EXPLAIN pushed sample metadata - Add 'as x' alias to BERNOULLI REPEATABLE parser test for consistency Test Plan: Existing tests. No golden file or EXPLAIN string assertions affected.

stanyao · 2026-04-09T05:11:52Z

Squashed multiple commits into a single one to make code review easier.

szehon-ho

@stanyao took a look, it looks coming together.

Do we have any plan to add test coverage using DSV2 InMemory data source? I think we should add it.

stanyao · 2026-04-16T04:04:39Z

@stanyao took a look, it looks coming together.

Do we have any plan to add test coverage using DSV2 InMemory data source? I think we should add it.

I added the following plan level test cases and harness:

InMemoryTableWithTableSample.scala — in-memory DSv2 table with scan builder implementing SupportsPushDownTableSample (BERNOULLI + SYSTEM).
InMemoryTableWithTableSampleCatalog.scala — catalog creating those tables.
DataSourceV2TableSampleSuite.scala — 7 end-to-end tests: SYSTEM/BERNOULLI/default pushdown, 0%/100% boundary, projection composition, and non-pushdown fallback.

szehon-ho

A few minor follow-ups (continuing from an earlier pass)

szehon-ho · 2026-04-25T06:48:19Z

+    // > 100 PERCENT
+    checkError(
+      exception = parseException(s"$sql tablesample system (150 percent) as x"),
+      condition = "_LEGACY_ERROR_TEMP_0064",


Legacy error class: This test still expects _LEGACY_ERROR_TEMP_0064 for invalid SYSTEM percentages. Consider migrating to a stable UNSUPPORTED_FEATURE / INVALID_* error class for consistency with the other new TABLESAMPLE_SYSTEM* conditions.

This is due to the call chain here -> AnalysisTest.scala -> AbstractSqlParser.scala -> AstBuilder.scala -> validate() function in ParserUtils.scala that is shared by both the existing Bernoulli sampling and the new System sampling. Fixing this will need code changes to 10 call sites and 20 test cases that are not related to this PR. It's best to be a dedicated follow up PR to keep this PR focused. I logged a JIRA item to track SPARK-56638.

szehon-ho · 2026-04-25T06:48:19Z

      // scalastyle:off line.size.limit
      // CURRENT_PATH is excluded: getSQLKeywords drops SQL:2003 reserved words (see companion).
-      assert(metadata.getSQLKeywords === "ADD,AFTER,AGGREGATE,ALWAYS,ANALYZE,ANTI,ANY_VALUE,ARCHIVE,ASC,BINDING,BUCKET,BUCKETS,BYTE,CACHE,CASCADE,CATALOG,CATALOGS,CHANGE,CHANGES,CLEAR,CLUSTER,CLUSTERED,CODEGEN,COLLATION,COLLATIONS,COLLECTION,COLUMNS,COMMENT,COMPACT,COMPACTIONS,COMPENSATION,COMPUTE,CONCATENATE,CONTAINS,CONTINUE,COST,CURRENT_DATABASE,CURRENT_SCHEMA,DATA,DATABASE,DATABASES,DATEADD,DATEDIFF,DATE_ADD,DATE_DIFF,DAYOFYEAR,DAYS,DBPROPERTIES,DEFAULT_PATH,DEFINED,DEFINER,DELAY,DELIMITED,DESC,DFS,DIRECTORIES,DIRECTORY,DISTRIBUTE,DIV,DO,ELSEIF,ENFORCED,ESCAPED,EVOLUTION,EXCHANGE,EXCLUDE,EXCLUSIVE,EXIT,EXPLAIN,EXPORT,EXTEND,EXTENDED,FIELDS,FILEFORMAT,FIRST,FLOW,FOLLOWING,FORMAT,FORMATTED,FOUND,FUNCTIONS,GENERATED,GEOGRAPHY,GEOMETRY,HANDLER,HOURS,IDENTIFIED,IDENTIFIER,IF,IGNORE,ILIKE,IMMEDIATE,INCLUDE,INCLUSIVE,INCREMENT,INDEX,INDEXES,INPATH,INPUT,INPUTFORMAT,INVOKER,ITEMS,ITERATE,JSON,KEY,KEYS,LAST,LAZY,LEAVE,LEVEL,LIMIT,LINES,LIST,LOAD,LOCATION,LOCK,LOCKS,LOGICAL,LONG,LOOP,MACRO,MAP,MATCHED,MATERIALIZED,MEASURE,METRICS,MICROSECOND,MICROSECONDS,MILLISECOND,MILLISECONDS,MINUS,MINUTES,MONTHS,MSCK,NAME,NAMESPACE,NAMESPACES,NANOSECOND,NANOSECONDS,NORELY,NULLS,OFFSET,OPTION,OPTIONS,OUTPUTFORMAT,OVERWRITE,PARTITIONED,PARTITIONS,PATH,PERCENT,PIVOT,PLACING,PRECEDING,PRINCIPALS,PROCEDURES,PROPERTIES,PURGE,QUALIFY,QUARTER,QUERY,RECORDREADER,RECORDWRITER,RECOVER,RECURSION,REDUCE,REFRESH,RELY,RENAME,REPAIR,REPEAT,REPEATABLE,REPLACE,RESET,RESPECT,RESTRICT,ROLE,ROLES,SCHEMA,SCHEMAS,SECONDS,SECURITY,SEMI,SEPARATED,SERDE,SERDEPROPERTIES,SETS,SHORT,SHOW,SINGLE,SKEWED,SORT,SORTED,SOURCE,STATISTICS,STORED,STRATIFY,STREAM,STREAMING,STRING,STRUCT,SUBSTR,SYNC,SYSTEM_PATH,SYSTEM_TIME,SYSTEM_VERSION,TABLES,TARGET,TBLPROPERTIES,TERMINATED,TIMEDIFF,TIMESTAMPADD,TIMESTAMPDIFF,TIMESTAMP_LTZ,TIMESTAMP_NTZ,TINYINT,TOUCH,TRANSACTION,TRANSACTIONS,TRANSFORM,TRUNCATE,TRY_CAST,TYPE,UNARCHIVE,UNBOUNDED,UNCACHE,UNLOCK,UNPIVOT,UNSET,UNTIL,USE,VAR,VARIABLE,VARIANT,VERSION,VIEW,VIEWS,VOID,WATERMARK,WEEK,WEEKS,WHILE,X,YEARS,ZONE")
+      assert(metadata.getSQLKeywords === "ADD,AFTER,AGGREGATE,ALWAYS,ANALYZE,ANTI,ANY_VALUE,ARCHIVE,ASC,BERNOULLI,BINDING,BUCKET,BUCKETS,BYTE,CACHE,CASCADE,CATALOG,CATALOGS,CHANGE,CHANGES,CLEAR,CLUSTER,CLUSTERED,CODEGEN,COLLATION,COLLATIONS,COLLECTION,COLUMNS,COMMENT,COMPACT,COMPACTIONS,COMPENSATION,COMPUTE,CONCATENATE,CONTAINS,CONTINUE,COST,CURRENT_DATABASE,CURRENT_SCHEMA,DATA,DATABASE,DATABASES,DATEADD,DATEDIFF,DATE_ADD,DATE_DIFF,DAYOFYEAR,DAYS,DBPROPERTIES,DEFAULT_PATH,DEFINED,DEFINER,DELAY,DELIMITED,DESC,DFS,DIRECTORIES,DIRECTORY,DISTRIBUTE,DIV,DO,ELSEIF,ENFORCED,ESCAPED,EVOLUTION,EXCHANGE,EXCLUDE,EXCLUSIVE,EXIT,EXPLAIN,EXPORT,EXTEND,EXTENDED,FIELDS,FILEFORMAT,FIRST,FLOW,FOLLOWING,FORMAT,FORMATTED,FOUND,FUNCTIONS,GENERATED,GEOGRAPHY,GEOMETRY,HANDLER,HOURS,IDENTIFIED,IDENTIFIER,IF,IGNORE,ILIKE,IMMEDIATE,INCLUDE,INCLUSIVE,INCREMENT,INDEX,INDEXES,INPATH,INPUT,INPUTFORMAT,INVOKER,ITEMS,ITERATE,JSON,KEY,KEYS,LAST,LAZY,LEAVE,LEVEL,LIMIT,LINES,LIST,LOAD,LOCATION,LOCK,LOCKS,LOGICAL,LONG,LOOP,MACRO,MAP,MATCHED,MATERIALIZED,MEASURE,METRICS,MICROSECOND,MICROSECONDS,MILLISECOND,MILLISECONDS,MINUS,MINUTES,MONTHS,MSCK,NAME,NAMESPACE,NAMESPACES,NANOSECOND,NANOSECONDS,NORELY,NULLS,OFFSET,OPTION,OPTIONS,OUTPUTFORMAT,OVERWRITE,PARTITIONED,PARTITIONS,PATH,PERCENT,PIVOT,PLACING,PRECEDING,PRINCIPALS,PROCEDURES,PROPERTIES,PURGE,QUALIFY,QUARTER,QUERY,RECORDREADER,RECORDWRITER,RECOVER,RECURSION,REDUCE,REFRESH,RELY,RENAME,REPAIR,REPEAT,REPEATABLE,REPLACE,RESET,RESPECT,RESTRICT,ROLE,ROLES,SCHEMA,SCHEMAS,SECONDS,SECURITY,SEMI,SEPARATED,SERDE,SERDEPROPERTIES,SETS,SHORT,SHOW,SINGLE,SKEWED,SORT,SORTED,SOURCE,STATISTICS,STORED,STRATIFY,STREAM,STREAMING,STRING,STRUCT,SUBSTR,SYNC,SYSTEM_PATH,SYSTEM_TIME,SYSTEM_VERSION,TABLES,TARGET,TBLPROPERTIES,TERMINATED,TIMEDIFF,TIMESTAMPADD,TIMESTAMPDIFF,TIMESTAMP_LTZ,TIMESTAMP_NTZ,TINYINT,TOUCH,TRANSACTION,TRANSACTIONS,TRANSFORM,TRUNCATE,TRY_CAST,TYPE,UNARCHIVE,UNBOUNDED,UNCACHE,UNLOCK,UNPIVOT,UNSET,UNTIL,USE,VAR,VARIABLE,VARIANT,VERSION,VIEW,VIEWS,VOID,WATERMARK,WEEK,WEEKS,WHILE,X,YEARS,ZONE")


Keywords: BERNOULLI was added to the expected getSQLKeywords string, but the substring still runs SYNC,SYSTEM_PATH,... (no standalone SYSTEM), while ThriftServerWithSparkContextSuite includes both BERNOULLI and SYSTEM in CLI_ODBC_KEYWORDS. If Connect intentionally omits SYSTEM (e.g. SQL:2003 reserved-word filtering per the comment above), a brief note here would help future contributors avoid “fixing” it incorrectly.

Added "SYSTEM" in line 212, so it reads:
// CURRENT_PATH and SYSTEM are excluded: getSQLKeywords drops SQL:2003 reserved words (see companion).

szehon-ho · 2026-04-25T06:48:19Z

        // style 1: values (1), (2), (3)  -- multiple columns are supported
        // style 2: values 1, 2, 3  -- only a single column is supported here
-        // Strip Alias wrappers from row values — CreateStruct.apply preserves them for
+        // Strip Alias wrappers from row values - CreateStruct.apply preserves them for


Unrelated change: The em dash → hyphen change in this comment is unrelated to TABLESAMPLE; harmless but slightly widens the diff.

This actually breaks linter (NonASCIICharacterChecker enabled at scalastyle-config.xml:119). Since it is just one character in the comment, I'd recommend keeping it.

szehon-ho · 2026-04-25T06:48:19Z

 |SUBSTR|non-reserved|non-reserved|non-reserved|
 |SUBSTRING|non-reserved|non-reserved|non-reserved|
 |SYNC|non-reserved|non-reserved|non-reserved|
+|SYSTEM|non-reserved|non-reserved|reserved|


Keyword table: The new SYSTEM row shows reserved in the last column while BERNOULLI is non-reserved across the board. Worth double-checking this row matches the same convention as neighboring keywords so the table stays internally consistent.

Double checked. The SQL-2016 column is correct for both keywords. SYSTEM is a reserved word in the SQL:2016 standard while BERNOULLI is not. The reasoning for that is that SYSTEM is reserved broadly because of its wide use in other syntax like SYSTEM_USER, SYSTEM VERSIONING, etc.

szehon-ho

I think it looks good. My only concern was starting with small default seed.

Ill leave to @cloud-fan

cloud-fan

Overall LGTM. The end-to-end wiring — parser → Sample.sampleMethod → V2ScanRelationPushDown.pushDownSample → new SupportsPushDownTableSample default 5-arg overload → pushedSampleMetadataString — is consistent with the existing BERNOULLI plumbing, the backward-compat path for legacy 4-arg connectors is correctly enforced, and the test coverage in PlanParserSuite and DataSourceV2TableSampleSuite is thorough (including the join+sample guard and the DSv1 / subquery / WHERE-filter error paths).

On the parser changes specifically (you'd asked me to take a look): the grammar change TABLESAMPLE (sampleType=(SYSTEM | BERNOULLI))? LEFT_PAREN ... is safe. ANTLR LL(*) lookahead deterministically classifies the token after TABLESAMPLE. Both keywords are added to ansiNonReserved/nonReserved so they remain usable as columns/tables/aliases. SYSTEM_TIME / SYSTEM_VERSION are distinct lexer tokens, so the existing temporalClause paths are unaffected. The dedicated identifier-conflict tests (SYSTEM as column/table/alias, schema-qualified mydb.system, cross-keyword mixes) cover the cases I cared about.

A few non-blocking suggestions inline — the most substantive is the defensive SparkStrategies throw, which is unreachable in normal flow (V2ScanRelationPushDown is non-excludable and always either removes the SYSTEM Sample node or throws). Worth converting to SparkException.internalError or removing; the rest are nits.

cloud-fan · 2026-05-11T18:34:36Z

        execution.ExpandExec(e.projections, e.output, planLater(child)) :: Nil
-      case logical.Sample(lb, ub, withReplacement, seed, child) =>
+      case logical.Sample(lb, ub, withReplacement, seed, child, sampleMethod) =>
+        if (sampleMethod == logical.SampleMethod.System) {


This branch is unreachable in normal flow — V2ScanRelationPushDown is non-excludable and always handles SYSTEM samples (either removes the Sample node or throws). Reaching it would indicate an internal invariant violation (e.g. a custom Optimizer subclass that skipped V2ScanRelationPushDown), not a user error, so a SparkException.internalError would be more accurate than a user-facing AnalysisException.

The comment is also a bit misleading: it says "fell through to row-based sampling" but the actual cause if this is ever reached is "the SYSTEM-handling optimizer rule didn't run as expected".

Very good point. Yeah, SYSTEM sample nodes should have been fully handled by V2ScanRelationPushDown (including throwing AnalysisException for user error like doing system sampling on DSV1 source); reaching this point indicates an internal error. Fixed both the error type and comment, while still keeping this defensive check.

cloud-fan · 2026-05-11T18:34:37Z

  }

  def pushDownSample(plan: LogicalPlan): LogicalPlan = plan.transform {
+    case sample: Sample if sample.sampleMethod == SampleMethod.System =>


The SYSTEM and BERNOULLI branches share most of the structure — TableSampleInfo construction, the PhysicalOperation(_, Nil, sHolder) pattern, and the PushDownUtils.pushTableSample call — and differ only in failure handling. Worth consolidating into a single case sample: Sample => sample.child match { ... } that branches on sampleMethod only for the throw vs. fall-back decision.

As a side effect, the two paths have already started to drift: the seed-TODO comment lives only on the SYSTEM branch even though both branches use the same * 1000 multiplier.

Good catch! The new case for SYSTEM started differently and evolved to be very similar to BERNOULLI case. I have merged them to reduce duplicate code.

cloud-fan · 2026-05-11T18:34:37Z

   * - TABLESAMPLE(x ROWS): Sample the table down to the given number of rows.
   * - TABLESAMPLE(x PERCENT) [REPEATABLE (y)]: Sample the table down to the given percentage with
   * seed 'y'. Note that percentages are defined as a number between 0 and 100.
+   * - TABLESAMPLE SYSTEM(x PERCENT): Sample by data source dependent blocks or file splits.


Compound modifier should be hyphenated.

Suggested change

* - TABLESAMPLE SYSTEM(x PERCENT): Sample by data source dependent blocks or file splits.

* - TABLESAMPLE SYSTEM(x PERCENT): Sample by data-source-dependent blocks or file splits.

cloud-fan · 2026-05-11T18:34:37Z

+      long seed,
+      SampleMethod sampleMethod) {
+    if (sampleMethod == SampleMethod.SYSTEM) {
+      // If the data source hasn't overridden this method, it must have not added support


Word order — "must not have" reads more naturally than "must have not".

Suggested change

// If the data source hasn't overridden this method, it must have not added support

// If the data source hasn't overridden this method, it must not have added support

…shdown

stanyao · 2026-05-11T23:40:57Z

This has been approved and CI is green. Could you please merge when ready? Thanks! /cc: @cloud-fan , @szehon-ho

cloud-fan · 2026-05-12T01:06:29Z

thanks, merging to master/4.x/4.2!

…shdown ### What changes were proposed in this pull request? This PR adds support for ANSI SQL `TABLESAMPLE SYSTEM` (block-level sampling) alongside the existing `TABLESAMPLE BERNOULLI` (row-level sampling). Key changes: - **SQL grammar**: Extended `TABLESAMPLE` to accept an optional `SYSTEM` or `BERNOULLI` qualifier before the sample method. Added both as non-reserved keywords. - **Logical plan**: Introduced `SampleMethod` sealed trait (`Bernoulli`/`System`) and added it to the `Sample` node. Default is `Bernoulli` for backward compatibility. - **Parser**: `TABLESAMPLE SYSTEM` only supports `PERCENT` sampling and does not support `REPEATABLE`. Other methods (ROWS, BUCKET, BYTES) are rejected with clear error messages. - **DSv2 pushdown**: `TABLESAMPLE SYSTEM` is pushed down to data sources via an extended `SupportsPushDownTableSample.pushTableSample()` overload with `isSystemSampling` flag. Sources that don't override the new method reject SYSTEM sampling by default. - **Physical planning**: SYSTEM samples that aren't pushed down to a DSv2 source raise an `AnalysisException` — there is no row-level fallback since block sampling is data-source dependent. ### Why are the changes needed? ANSI SQL defines two sampling methods: `BERNOULLI` (row-level) and `SYSTEM` (implementation-dependent, typically block/split-level). Block sampling is significantly faster for large tables since it avoids per-row evaluation, making it useful for approximate queries and data exploration. Many databases (PostgreSQL, Hive, Trino) support this distinction. ### Does this PR introduce _any_ user-facing change? Yes. New SQL syntax `TABLESAMPLE SYSTEM (x PERCENT)` and `TABLESAMPLE BERNOULLI (x PERCENT)`. `BERNOULLI` and `SYSTEM` are added as non-reserved keywords. Existing queries without these keywords behave identically to before. ### How was this patch tested? - 9 new test cases in `PlanParserSuite` covering: basic parsing, case insensitivity, boundary fractions, unsupported methods (ROWS/BUCKET with SYSTEM), REPEATABLE rejection, fraction validation, identifier preservation, and subquery contexts. - Existing `SQLQuerySuite` tests pass. - Scalastyle passes with 0 errors/warnings. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (thoroughly refined, reviewed, and tested by human) Closes #54972 from stanyao/spark-55978-tablesample-system. Lead-authored-by: Stanley Yao <592623+stanyao@users.noreply.github.com> Co-authored-by: Stanley Yao <stanleyy@fb.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 5fe5451) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

… Sample has fraction=1 without replacement ### What changes were proposed in this pull request? Followup to #54972. Narrow the `V2ScanRelationPushDown` join-pushdown guard so it only blocks pushdown when at least one side has a pushed `Sample` that can actually change the result set. A pushed sample is treated as a no-op when **both** `withReplacement = false` **and** `upperBound - lowerBound >= 1.0` (fraction = 1). In that case dropping the sample inside the merged scan builder is safe. ### Why are the changes needed? The guard added in SPARK-55978 exists because the merged scan builder for `SupportsPushDownJoin` cannot carry a pushed sample and would silently discard it. The hazard is *silent result change*. For a `Sample` without replacement at fraction = 1, no rows are excluded, so dropping the sample changes nothing observable. The current guard is therefore stricter than its rationale requires, and unnecessarily skips join pushdown for queries that land at `TABLESAMPLE SYSTEM (100 PERCENT)` (parameterized queries, query generators, environment-tuned fractions). With replacement is a different story: `SampleExec` uses `PoissonSampler`, and even at rate 1.0 each input row can be emitted 0, 1, 2, … times, so it is **not** a no-op. SQL `TABLESAMPLE` always sets `withReplacement = false`, but `DataFrame.sample(withReplacement = true, fraction = 1.0)` can be pushed to DSv2, so the guard must keep blocking that case. ### Does this PR introduce _any_ user-facing change? No behavior change for queries with fraction < 1, or for `withReplacement = true` at any fraction. For queries where the pushed sample has `withReplacement = false` and fraction = 1, join pushdown now proceeds — same result set, faster plan. ### How was this patch tested? - Existing `"join pushdown is skipped when a side has a pushed sample"` test moved from `100 PERCENT` to `50 PERCENT` so it keeps exercising the (still-active) fraction < 1 branch of the guard. - New `"100% SYSTEM sample does not block join pushdown"` test asserts the new fraction = 1, without-replacement short-circuit. - New `"with-replacement sample blocks join pushdown even at fraction 1"` test uses the DataFrame API to assert that Poisson sampling at fraction 1 keeps blocking join pushdown. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #56031 from cloud-fan/SPARK-55978-followup. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

… Sample has fraction=1 without replacement ### What changes were proposed in this pull request? Followup to #54972. Narrow the `V2ScanRelationPushDown` join-pushdown guard so it only blocks pushdown when at least one side has a pushed `Sample` that can actually change the result set. A pushed sample is treated as a no-op when **both** `withReplacement = false` **and** `upperBound - lowerBound >= 1.0` (fraction = 1). In that case dropping the sample inside the merged scan builder is safe. ### Why are the changes needed? The guard added in SPARK-55978 exists because the merged scan builder for `SupportsPushDownJoin` cannot carry a pushed sample and would silently discard it. The hazard is *silent result change*. For a `Sample` without replacement at fraction = 1, no rows are excluded, so dropping the sample changes nothing observable. The current guard is therefore stricter than its rationale requires, and unnecessarily skips join pushdown for queries that land at `TABLESAMPLE SYSTEM (100 PERCENT)` (parameterized queries, query generators, environment-tuned fractions). With replacement is a different story: `SampleExec` uses `PoissonSampler`, and even at rate 1.0 each input row can be emitted 0, 1, 2, … times, so it is **not** a no-op. SQL `TABLESAMPLE` always sets `withReplacement = false`, but `DataFrame.sample(withReplacement = true, fraction = 1.0)` can be pushed to DSv2, so the guard must keep blocking that case. ### Does this PR introduce _any_ user-facing change? No behavior change for queries with fraction < 1, or for `withReplacement = true` at any fraction. For queries where the pushed sample has `withReplacement = false` and fraction = 1, join pushdown now proceeds — same result set, faster plan. ### How was this patch tested? - Existing `"join pushdown is skipped when a side has a pushed sample"` test moved from `100 PERCENT` to `50 PERCENT` so it keeps exercising the (still-active) fraction < 1 branch of the guard. - New `"100% SYSTEM sample does not block join pushdown"` test asserts the new fraction = 1, without-replacement short-circuit. - New `"with-replacement sample blocks join pushdown even at fraction 1"` test uses the DataFrame API to assert that Poisson sampling at fraction 1 keeps blocking join pushdown. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #56031 from cloud-fan/SPARK-55978-followup. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit e1babc6) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…tion failures ### What changes were proposed in this pull request? This PR removes the remaining use of `_LEGACY_ERROR_TEMP_0064` from parser validation paths that depended on `ParserUtils.validate(...)` ([SPARK-56638](https://issues.apache.org/jira/browse/SPARK-56638)). Specifically, this PR: - Adds named parser error conditions for the affected validation failures - Adds corresponding `QueryParsingErrors` helpers - Replaces `ParserUtils.validate(...)` call sites with specific parser errors - Removes `ParserUtils.validate(...)` - Updates parser tests and SQL golden files to assert the new error classes This includes the `TABLESAMPLE` fraction validation path, covering both existing `TABLESAMPLE` syntax and `TABLESAMPLE SYSTEM` from [#54972](#54972). ### Why are the changes needed? `ParserUtils.validate(...)` always threw `_LEGACY_ERROR_TEMP_0064`, which made different parser validation failures share the same generic legacy error class. Using named error classes makes each parser failure explicit and removes this legacy temporary error from the affected parser paths. `INVALID_TABLESAMPLE_FRACTION` uses SQLSTATE `22023` because the statement is syntactically valid, but the supplied sampling fraction is invalid/out of range. The other parser syntax validation errors are added under `INVALID_SQL_SYNTAX`, which uses SQLSTATE `42000`. ### Does this PR introduce _any_ user-facing change? Yes. The error class changes from `_LEGACY_ERROR_TEMP_0064` to named error classes for the affected parser validation failures. For invalid `TABLESAMPLE` fractions, the new error class is `INVALID_TABLESAMPLE_FRACTION`. ### How was this patch tested? ```bash build/sbt 'catalyst / Test / testOnly org.apache.spark.sql.catalyst.parser.PlanParserSuite -- -z "sampled relations"' build/sbt 'catalyst / Test / testOnly org.apache.spark.sql.catalyst.parser.PlanParserSuite -- -z "TABLESAMPLE SYSTEM - fraction out of range"' SPARK_GENERATE_GOLDEN_FILES=1 build/sbt 'sql / Test / testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z "tablesample-negative.sql"' SPARK_GENERATE_GOLDEN_FILES=1 build/sbt 'sql / Test / testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z "pipe-operators.sql"' build/sbt 'sql / Test / testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z "tablesample-negative.sql"' 'sql / Test / testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z "pipe-operators.sql"' ``` ### Was this patch authored or co-authored using generative AI tooling? Yes. Generated-by: OpenAI GPT-5.5 Codex Closes #55782 from XdithyX/SPARK-56638. Authored-by: Adithya Ajith <4adhi007@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…tion failures ### What changes were proposed in this pull request? This PR removes the remaining use of `_LEGACY_ERROR_TEMP_0064` from parser validation paths that depended on `ParserUtils.validate(...)` ([SPARK-56638](https://issues.apache.org/jira/browse/SPARK-56638)). Specifically, this PR: - Adds named parser error conditions for the affected validation failures - Adds corresponding `QueryParsingErrors` helpers - Replaces `ParserUtils.validate(...)` call sites with specific parser errors - Removes `ParserUtils.validate(...)` - Updates parser tests and SQL golden files to assert the new error classes This includes the `TABLESAMPLE` fraction validation path, covering both existing `TABLESAMPLE` syntax and `TABLESAMPLE SYSTEM` from [#54972](#54972). ### Why are the changes needed? `ParserUtils.validate(...)` always threw `_LEGACY_ERROR_TEMP_0064`, which made different parser validation failures share the same generic legacy error class. Using named error classes makes each parser failure explicit and removes this legacy temporary error from the affected parser paths. `INVALID_TABLESAMPLE_FRACTION` uses SQLSTATE `22023` because the statement is syntactically valid, but the supplied sampling fraction is invalid/out of range. The other parser syntax validation errors are added under `INVALID_SQL_SYNTAX`, which uses SQLSTATE `42000`. ### Does this PR introduce _any_ user-facing change? Yes. The error class changes from `_LEGACY_ERROR_TEMP_0064` to named error classes for the affected parser validation failures. For invalid `TABLESAMPLE` fractions, the new error class is `INVALID_TABLESAMPLE_FRACTION`. ### How was this patch tested? ```bash build/sbt 'catalyst / Test / testOnly org.apache.spark.sql.catalyst.parser.PlanParserSuite -- -z "sampled relations"' build/sbt 'catalyst / Test / testOnly org.apache.spark.sql.catalyst.parser.PlanParserSuite -- -z "TABLESAMPLE SYSTEM - fraction out of range"' SPARK_GENERATE_GOLDEN_FILES=1 build/sbt 'sql / Test / testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z "tablesample-negative.sql"' SPARK_GENERATE_GOLDEN_FILES=1 build/sbt 'sql / Test / testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z "pipe-operators.sql"' build/sbt 'sql / Test / testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z "tablesample-negative.sql"' 'sql / Test / testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z "pipe-operators.sql"' ``` ### Was this patch authored or co-authored using generative AI tooling? Yes. Generated-by: OpenAI GPT-5.5 Codex Closes #55782 from XdithyX/SPARK-56638. Authored-by: Adithya Ajith <4adhi007@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 447d3b4) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

stanyao changed the title ~~[SPARK-55978][SQL] Add TABLESAMPLE SYSTEM block sampling with DSv2 pu…~~ [SPARK-55978][SQL] Add TABLESAMPLE SYSTEM block sampling with DSv2 pushdown Mar 24, 2026

stanyao force-pushed the spark-55978-tablesample-system branch 3 times, most recently from bd6f9cf to afbd7d5 Compare March 24, 2026 16:05

stanyao force-pushed the spark-55978-tablesample-system branch from afbd7d5 to 340c545 Compare March 28, 2026 17:17

szehon-ho reviewed Mar 30, 2026

View reviewed changes

stanyao force-pushed the spark-55978-tablesample-system branch from 32fd86b to 84cdd7f Compare April 2, 2026 00:01

stanyao force-pushed the spark-55978-tablesample-system branch from 84cdd7f to b390671 Compare April 2, 2026 03:37

szehon-ho reviewed Apr 7, 2026

View reviewed changes

stanyao force-pushed the spark-55978-tablesample-system branch from 3437a06 to ee53951 Compare April 8, 2026 00:29

stanyao force-pushed the spark-55978-tablesample-system branch from ca7eab3 to 549db12 Compare April 8, 2026 23:18

stanyao force-pushed the spark-55978-tablesample-system branch from 06ab21c to 447d891 Compare April 9, 2026 05:10

stanyao force-pushed the spark-55978-tablesample-system branch from 447d891 to a2170d4 Compare April 10, 2026 23:49

stanyao requested a review from szehon-ho April 14, 2026 05:28

szehon-ho reviewed Apr 15, 2026

View reviewed changes

stanyao force-pushed the spark-55978-tablesample-system branch 3 times, most recently from 9043a4d to 311b3fe Compare April 17, 2026 02:27

stanyao force-pushed the spark-55978-tablesample-system branch 5 times, most recently from 9f24f9d to e62cacc Compare April 24, 2026 01:16

stanyao requested a review from szehon-ho April 24, 2026 16:13

szehon-ho reviewed Apr 25, 2026

View reviewed changes

stanyao force-pushed the spark-55978-tablesample-system branch from e62cacc to 0d4c4d9 Compare April 27, 2026 21:57

stanyao requested a review from szehon-ho April 28, 2026 00:15

stanyao force-pushed the spark-55978-tablesample-system branch from 0d4c4d9 to a25f63f Compare May 5, 2026 15:51

szehon-ho approved these changes May 11, 2026

View reviewed changes

cloud-fan approved these changes May 11, 2026

View reviewed changes

[SPARK-55978][SQL] Add TABLESAMPLE SYSTEM block sampling with DSv2 pu…

b1df9f0

…shdown

stanyao force-pushed the spark-55978-tablesample-system branch from a25f63f to b1df9f0 Compare May 11, 2026 20:29

stanyao mentioned this pull request May 11, 2026

[SPARK-56638][SQL] Replace legacy parser error for ParserUtils validation failures #55782

Closed

cloud-fan closed this in 5fe5451 May 12, 2026

cloud-fan mentioned this pull request May 21, 2026

[SPARK-55978][SQL][FOLLOWUP] Don't block V2 join pushdown when pushed Sample has fraction=1 without replacement #56031

Closed

stanyao deleted the spark-55978-tablesample-system branch July 31, 2026 05:24

akshaytayal mentioned this pull request Sep 17, 2026

[VL] Spark 4.2: Add new UTs — SQL DataSource V2 connector suites apache/gluten#13049

Open

3 tasks

	* - TABLESAMPLE SYSTEM(x PERCENT): Sample by data source dependent blocks or file splits.
	* - TABLESAMPLE SYSTEM(x PERCENT): Sample by data-source-dependent blocks or file splits.

	// If the data source hasn't overridden this method, it must have not added support
	// If the data source hasn't overridden this method, it must not have added support

Conversation

stanyao commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

dbtsai commented Mar 26, 2026

Uh oh!

stanyao commented Mar 28, 2026

Uh oh!

szehon-ho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

szehon-ho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stanyao commented Apr 9, 2026

Uh oh!

szehon-ho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stanyao commented Apr 16, 2026

Uh oh!

szehon-ho left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stanyao Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szehon-ho left a comment

Choose a reason for hiding this comment

Uh oh!

cloud-fan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stanyao May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stanyao commented Mar 24, 2026 •

edited

Loading

szehon-ho left a comment •

edited

Loading

stanyao Apr 27, 2026 •

edited

Loading

stanyao May 11, 2026 •

edited

Loading