-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-29277][SQL] Add early DSv2 filter and projection pushdown #25955
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 4 commits
7d7c914
e7beb5b
fb7f54d
621224a
4220723
c223e05
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 |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ import org.apache.spark.sql.connector.read.{Scan, ScanBuilder, Statistics => V2S | |
| import org.apache.spark.sql.connector.read.streaming.{Offset, SparkDataStream} | ||
| import org.apache.spark.sql.connector.write.WriteBuilder | ||
| import org.apache.spark.sql.util.CaseInsensitiveStringMap | ||
| import org.apache.spark.util.Utils | ||
|
|
||
| /** | ||
| * A logical plan representing a data source v2 table. | ||
|
|
@@ -55,7 +56,52 @@ case class DataSourceV2Relation( | |
| } | ||
|
|
||
| override def computeStats(): Statistics = { | ||
| val scan = newScanBuilder().build() | ||
| if (Utils.isTesting) { | ||
| // when testing, throw an exception if this computeStats method is called because stats should | ||
| // not be accessed before pushing the projection and filters to create a scan. otherwise, the | ||
| // stats are not accurate because they are based on a full table scan of all columns. | ||
| throw new UnsupportedOperationException( | ||
|
rdblue marked this conversation as resolved.
Outdated
|
||
| s"BUG: computeStats called before pushdown on DSv2 relation: $name") | ||
| } else { | ||
| // when not testing, return stats because bad stats are better than failing a query | ||
| newScanBuilder() match { | ||
|
rdblue marked this conversation as resolved.
Outdated
Contributor
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 was inlined in a previous commit, why it's reverted? |
||
| case r: SupportsReportStatistics => | ||
| val statistics = r.estimateStatistics() | ||
| DataSourceV2Relation.transformV2Stats(statistics, None, conf.defaultSizeInBytes) | ||
| case _ => | ||
| Statistics(sizeInBytes = conf.defaultSizeInBytes) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override def newInstance(): DataSourceV2Relation = { | ||
| copy(output = output.map(_.newInstance())) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A logical plan for a DSv2 table with a scan already created. | ||
| * | ||
| * This is used in the optimizer to push filters and projection down before conversion to physical | ||
| * plan. This ensures that the stats that are used by the optimizer account for the filters and | ||
| * projection that will be pushed down. | ||
| * | ||
| * @param table a DSv2 [[Table]] | ||
| * @param scan a DSv2 [[Scan]] | ||
| * @param output the output attributes of this relation | ||
| */ | ||
| case class DataSourceV2ScanRelation( | ||
|
cloud-fan marked this conversation as resolved.
|
||
| table: Table, | ||
| scan: Scan, | ||
| output: Seq[AttributeReference]) extends LeafNode with NamedRelation { | ||
|
|
||
| override def name: String = table.name() | ||
|
|
||
| override def simpleString(maxFields: Int): String = { | ||
| s"RelationV2${truncatedString(output, "[", ", ", "]", maxFields)} $name" | ||
| } | ||
|
|
||
| override def computeStats(): Statistics = { | ||
| scan match { | ||
| case r: SupportsReportStatistics => | ||
| val statistics = r.estimateStatistics() | ||
|
|
@@ -64,10 +110,6 @@ case class DataSourceV2Relation( | |
| Statistics(sizeInBytes = conf.defaultSizeInBytes) | ||
| } | ||
| } | ||
|
|
||
| override def newInstance(): DataSourceV2Relation = { | ||
| copy(output = output.map(_.newInstance())) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,7 +51,7 @@ import org.apache.spark.sql.execution._ | |
| import org.apache.spark.sql.execution.arrow.{ArrowBatchStreamWriter, ArrowConverters} | ||
| import org.apache.spark.sql.execution.command._ | ||
| import org.apache.spark.sql.execution.datasources.LogicalRelation | ||
| import org.apache.spark.sql.execution.datasources.v2.{DataSourceV2Relation, FileTable} | ||
| import org.apache.spark.sql.execution.datasources.v2.{DataSourceV2Relation, DataSourceV2ScanRelation, FileTable} | ||
| import org.apache.spark.sql.execution.python.EvaluatePython | ||
| import org.apache.spark.sql.execution.stat.StatFunctions | ||
| import org.apache.spark.sql.internal.SQLConf | ||
|
|
@@ -3218,6 +3218,8 @@ class Dataset[T] private[sql]( | |
| fr.inputFiles | ||
| case r: HiveTableRelation => | ||
| r.tableMeta.storage.locationUri.map(_.toString).toArray | ||
| case DataSourceV2ScanRelation(table: FileTable, _, _) => | ||
| table.fileIndex.inputFiles | ||
| case DataSourceV2Relation(table: FileTable, _, _) => | ||
|
Contributor
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. In a previous discussion, we decided to make |
||
| table.fileIndex.inputFiles | ||
| }.flatten | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.