Repository navigation
[SPARK-55978][SQL] Add TABLESAMPLE SYSTEM block sampling with DSv2 pushdown #54972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.sql.connector.read; | ||
|
|
||
| import org.apache.spark.annotation.Evolving; | ||
|
|
||
| /** | ||
| * The sampling method for TABLESAMPLE. | ||
| * | ||
| * @since 4.2.0 | ||
| */ | ||
| @Evolving | ||
| public enum SampleMethod { | ||
| /** Row-level sampling (BERNOULLI). Each row is independently selected. */ | ||
| BERNOULLI, | ||
| /** Block-level sampling (SYSTEM). Entire partitions/splits are included or skipped. */ | ||
| SYSTEM | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2448,10 +2448,14 @@ class AstBuilder extends DataTypeAstBuilder | |
| * - 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. | ||
| * - TABLESAMPLE(BUCKET x OUT OF y) [REPEATABLE (z)]: Sample the table down to a 'x' divided by | ||
| * 'y' fraction with seed 'z'. | ||
| */ | ||
| private def withSample(ctx: SampleContext, query: LogicalPlan): LogicalPlan = withOrigin(ctx) { | ||
| val isSystem = ctx.sampleType != null && | ||
| ctx.sampleType.getType == SqlBaseParser.SYSTEM | ||
|
|
||
| // Create a sampled plan if we need one. | ||
| def sample(fraction: Double, seed: Option[Long]): Sample = { | ||
| // The range of fraction accepted by Sample is [0, 1]. Because Hive's block sampling | ||
|
|
@@ -2461,17 +2465,25 @@ class AstBuilder extends DataTypeAstBuilder | |
| validate(fraction >= 0.0 - eps && fraction <= 1.0 + eps, | ||
| s"Sampling fraction ($fraction) must be on interval [0, 1]", | ||
| ctx) | ||
| Sample(0.0, fraction, withReplacement = false, seed, query) | ||
| val method = if (isSystem) SampleMethod.System else SampleMethod.Bernoulli | ||
| Sample(0.0, fraction, withReplacement = false, seed, query, method) | ||
| } | ||
|
|
||
| if (ctx.sampleMethod() == null) { | ||
| throw QueryParsingErrors.emptyInputForTableSampleError(ctx) | ||
| } | ||
|
|
||
| if (isSystem && ctx.seed != null) { | ||
| throw QueryParsingErrors.tableSampleSystemRepeatableError(ctx) | ||
| } | ||
|
|
||
| val seed: Option[Long] = Option(ctx.seed).map(_.getText.toLong) | ||
|
|
||
| ctx.sampleMethod() match { | ||
| case ctx: SampleByRowsContext => | ||
| if (isSystem) { | ||
| throw QueryParsingErrors.tableSampleSystemSampleMethodError("ROWS", ctx) | ||
| } | ||
| Limit(expression(ctx.expression), query) | ||
|
|
||
| case ctx: SampleByPercentileContext => | ||
|
|
@@ -2483,6 +2495,9 @@ class AstBuilder extends DataTypeAstBuilder | |
| sample(sign * fraction / 100.0d, seed) | ||
|
|
||
| case ctx: SampleByBytesContext => | ||
| if (isSystem) { | ||
| throw QueryParsingErrors.tableSampleSystemSampleMethodError("BYTES", ctx) | ||
| } | ||
| val bytesStr = ctx.bytes.getText | ||
| if (bytesStr.matches("[0-9]+[bBkKmMgG]")) { | ||
| throw QueryParsingErrors.tableSampleByBytesUnsupportedError("byteLengthLiteral", ctx) | ||
|
|
@@ -2491,6 +2506,9 @@ class AstBuilder extends DataTypeAstBuilder | |
| } | ||
|
|
||
| case ctx: SampleByBucketContext if ctx.ON() != null => | ||
| if (isSystem) { | ||
| throw QueryParsingErrors.tableSampleSystemSampleMethodError("BUCKET", ctx) | ||
| } | ||
| if (ctx.identifier != null) { | ||
| throw QueryParsingErrors.tableSampleByBytesUnsupportedError( | ||
| "BUCKET x OUT OF y ON colname", ctx) | ||
|
|
@@ -2500,6 +2518,9 @@ class AstBuilder extends DataTypeAstBuilder | |
| } | ||
|
|
||
| case ctx: SampleByBucketContext => | ||
| if (isSystem) { | ||
| throw QueryParsingErrors.tableSampleSystemSampleMethodError("BUCKET", ctx) | ||
| } | ||
| sample(ctx.numerator.getText.toDouble / ctx.denominator.getText.toDouble, seed) | ||
| } | ||
| } | ||
|
|
@@ -2912,7 +2933,7 @@ class AstBuilder extends DataTypeAstBuilder | |
| // inline table comes in two styles: | ||
| // 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated change: The em dash → hyphen change in this comment is unrelated to TABLESAMPLE; harmless but slightly widens the diff.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| // expressions like `(1 AS id, 'a' AS name)`, but they are redundant here since | ||
| // column names are determined by the table alias or generated defaults. | ||
| case struct: CreateNamedStruct => struct.valExprs.map { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keyword table: The new
SYSTEMrow showsreservedin the last column whileBERNOULLIisnon-reservedacross the board. Worth double-checking this row matches the same convention as neighboring keywords so the table stays internally consistent.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.