-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-37020][SQL] DS V2 LIMIT push down #34291
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 13 commits
37714db
16a617b
788f893
e6b419a
c3e5097
979e6d6
0c169c4
e82b846
c5967c4
5b7d16b
4a48624
499dff1
836746e
496878c
08f57cd
86c020d
008aadb
a36007e
e9978a8
7a3415e
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,36 @@ | ||
| /* | ||
| * 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; | ||
|
|
||
| /** | ||
| * A mix-in interface for {@link Scan}. Data sources can implement this interface to | ||
| * push down LIMIT. Please note that the combination of LIMIT with other operations | ||
| * such as AGGREGATE, GROUP BY, SORT BY, CLUSTER BY, DISTRIBUTE BY, etc. is NOT pushed down. | ||
|
cloud-fan marked this conversation as resolved.
|
||
| * | ||
| * @since 3.3.0 | ||
| */ | ||
| @Evolving | ||
| public interface SupportsPushDownLimit extends Scan { | ||
|
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. I think all pushdown API should extends |
||
|
|
||
| /** | ||
| * Pushes down LIMIT to the data source. | ||
| */ | ||
| int pushLimit(int limit); | ||
|
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. I feel this is a bit over-designed. The source should just tell Spark if the limit can be pushed or not, instead of returning the actual limit, as it's very rare that the pushed limit is different from the limit from Spark.
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -298,17 +298,22 @@ private[sql] case class JDBCRelation( | |
| requiredColumns: Array[String], | ||
| finalSchema: StructType, | ||
| filters: Array[Filter], | ||
| groupByColumns: Option[Array[String]]): RDD[Row] = { | ||
| groupByColumns: Option[Array[String]], | ||
| limit: Int): RDD[Row] = { | ||
| // If limit is pushed down, only a limited number of rows will be returned. PartitionInfo will | ||
| // be ignored and the query will be done in one task. | ||
|
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. just wondering would only one task be a problem, when reading big JDBC data source? Is it possible to lift the restriction to allow more than one task in the future?
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. The reason we only want one task is because of the JDBC partition implementation. JDBC doesn't have physical partition. For JDBC partition such as JDBC will have three parallel queries: I was initially thinking of evenly divided the number N among the queries, e.g. for LIMIT(6), I will do but it doesn't work. If there are 8 rows in the table, but the first partition only has 1 row, the 2nd partition has 1 row, the 3 partition has 6 row, the above queries return 4 rows, but LIMIT(6) should return 6 rows.
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. ah I see, thanks for explanation @huaxingao. In this case, maybe we can push the original limit to each queries? Spark will anyway to do LIMIT again after reading JDBC data source. So we don't have the correctness problem, and the performance will still be better than not pushing down limit. This is not urgent for this PR anyway.
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. Good idea! I will fix this. Thanks! |
||
| val partition = if (limit > 0 ) { Array[Partition](JDBCPartition(null, 0)) } else parts | ||
| // Rely on a type erasure hack to pass RDD[InternalRow] back as RDD[Row] | ||
| JDBCRDD.scanTable( | ||
| sparkSession.sparkContext, | ||
| schema, | ||
| requiredColumns, | ||
| filters, | ||
| parts, | ||
| partition, | ||
| jdbcOptions, | ||
| Some(finalSchema), | ||
| groupByColumns).asInstanceOf[RDD[Row]] | ||
| groupByColumns, | ||
| limit).asInstanceOf[RDD[Row]] | ||
| } | ||
|
|
||
| override def insert(data: DataFrame, overwrite: Boolean): Unit = { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.