You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/11/29 10:11:54 UTC
[GitHub] [spark] beliefer opened a new pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
beliefer opened a new pull request #34738:
URL: https://github.com/apache/spark/pull/34738
### What changes were proposed in this pull request?
Currently, Spark supports push down limit to data source.
However, in the user's scenario, limit must have the premise of order by. Because limit and order by are more valuable together.
On the other hand, push down top N(same as order by ... limit N) outputs the data with basic order to Spark sort, the the sort of Spark may have some performance improvement.
### Why are the changes needed?
1. push down top N is very useful for users scenario.
2. push down top N could improves the performance of sort.
### Does this PR introduce _any_ user-facing change?
'No'. Just change the physical execute.
### How was this patch tested?
New tests.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989881120
**[Test build #146041 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146041/testReport)** for PR 34738 at commit [`9c0cbc0`](https://github.com/apache/spark/commit/9c0cbc07c9b79ee3cc585bcb613f11cd4e214139).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-990133358
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146041/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r767462841
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala
##########
@@ -196,6 +196,10 @@ class JDBCOptions(
// This only applies to Data Source V2 JDBC
val pushDownLimit = parameters.getOrElse(JDBC_PUSHDOWN_LIMIT, "false").toBoolean
+ // An option to allow/disallow pushing down query of top N into V2 JDBC data source
+ // This only applies to Data Source V2 JDBC
+ val pushDownTopN = parameters.getOrElse(JDBC_PUSHDOWN_TOP_N, "false").toBoolean
Review comment:
I'm wondering if we should just reuse the limit pushdown option, as it's kind of a special case of LIMIT.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-988567892
ping @cloud-fan
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765793651
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -138,33 +140,117 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkPushedLimit(df4, false, 0)
checkAnswer(df4, Seq(Row(1, 19000.00)))
+ val name = udf { (x: String) => x.matches("cat|dav|amy") }
+ val sub = udf { (x: String) => x.substring(0, 3) }
val df5 = spark.read
.table("h2.test.employee")
- .sort("SALARY")
+ .select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
+ .filter(name($"shortName"))
.limit(1)
+ // LIMIT is pushed down only if all the filters are pushed down
checkPushedLimit(df5, false, 0)
- checkAnswer(df5, Seq(Row(1, "cathy", 9000.00, 1200.0)))
+ checkAnswer(df5, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkPushedLimit(df: DataFrame, pushed: Boolean, limit: Int): Unit = {
+ df.queryExecution.optimizedPlan.collect {
+ case relation: DataSourceV2ScanRelation => relation.scan match {
+ case v1: V1ScanWrapper =>
+ if (pushed) {
+ assert(v1.pushedDownOperators.limit === Some(limit))
+ } else {
+ assert(v1.pushedDownOperators.limit.isEmpty)
+ }
+ }
+ }
+ }
+
+ test("simple scan with top N") {
+ val df1 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary").limit(1)
+ val expectedSorts1 =
+ Seq(SortValue(FieldReference("salary"), SortDirection.ASCENDING, NullOrdering.NULLS_FIRST))
+ checkPushedTopN(df1, true, 1, expectedSorts1)
+ checkAnswer(df1, Seq(Row(1, "cathy", 9000.00, 1200.0)))
+
+ val df2 = spark.read
+ .option("partitionColumn", "dept")
+ .option("lowerBound", "0")
+ .option("upperBound", "2")
+ .option("numPartitions", "2")
+ .table("h2.test.employee")
+ .filter($"dept" > 1)
+ .orderBy($"salary".desc)
+ .limit(1)
+ val expectedSorts2 =
+ Seq(SortValue(FieldReference("salary"), SortDirection.DESCENDING, NullOrdering.NULLS_LAST))
+ checkPushedTopN(df2, true, 1, expectedSorts2)
+ checkAnswer(df2, Seq(Row(2, "alex", 12000.00, 1200.0)))
+
+ val df3 =
+ sql("SELECT name FROM h2.test.employee WHERE dept > 1 ORDER BY salary NULLS LAST LIMIT 1")
+ val scan = df3.queryExecution.optimizedPlan.collectFirst {
+ case s: DataSourceV2ScanRelation => s
+ }.get
+ assert(scan.schema.names.sameElements(Seq("NAME")))
+ val expectedSorts3 =
+ Seq(SortValue(FieldReference("salary"), SortDirection.ASCENDING, NullOrdering.NULLS_LAST))
+ checkPushedTopN(df3, true, 1, expectedSorts3)
+ checkAnswer(df3, Seq(Row("david")))
+
+ val df4 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary")
+ checkPushedTopN(df4, false, 0)
+ checkAnswer(df4, Seq(Row(1, "cathy", 9000.00, 1200.0), Row(1, "amy", 10000.00, 1000.0)))
+
+ val df5 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).limit(1)
+ checkPushedTopN(df5, false, 1)
+ checkAnswer(df5, Seq(Row(1, "amy", 10000.00, 1000.0)))
+
+ val df6 = spark.read
+ .table("h2.test.employee")
+ .groupBy("DEPT").sum("SALARY")
+ .orderBy("DEPT")
+ .limit(1)
+ checkPushedTopN(df6, false, 0)
+ checkAnswer(df6, Seq(Row(1, 19000.00)))
+
+ val df7 = spark.read
+ .table("h2.test.employee")
+ .sort("SALARY")
+ .limit(1)
+ val expectedSorts5 =
+ Seq(SortValue(FieldReference("SALARY"), SortDirection.ASCENDING, NullOrdering.NULLS_FIRST))
+ checkPushedTopN(df7, true, 1, expectedSorts5)
+ checkAnswer(df7, Seq(Row(1, "cathy", 9000.00, 1200.0)))
val name = udf { (x: String) => x.matches("cat|dav|amy") }
val sub = udf { (x: String) => x.substring(0, 3) }
- val df6 = spark.read
+ val df8 = spark.read
.table("h2.test.employee")
.select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
.filter(name($"shortName"))
+ .sort($"SALARY".desc)
.limit(1)
// LIMIT is pushed down only if all the filters are pushed down
- checkPushedLimit(df6, false, 0)
- checkAnswer(df6, Seq(Row(10000.00, 1000.0, "amy")))
+ checkPushedTopN(df8, false, 0)
+ checkAnswer(df8, Seq(Row(10000.00, 1000.0, "amy")))
}
- private def checkPushedLimit(df: DataFrame, pushed: Boolean, limit: Int): Unit = {
+ private def checkPushedTopN(df: DataFrame, pushed: Boolean, limit: Int = 0,
Review comment:
ditto, `limit: Option[Int]` is better
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989983349
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50516/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994481810
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50693/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994951556
thanks, merging to master!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994351553
**[Test build #146219 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146219/testReport)** for PR 34738 at commit [`eb32095`](https://github.com/apache/spark/commit/eb32095d4dc6967a9cb48e0335f04bfd023ff5a9).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983750385
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50292/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r760144434
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -249,13 +248,25 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
def applyLimit(plan: LogicalPlan): LogicalPlan = plan.transform {
case globalLimit @ Limit(IntegerLiteral(limitValue), child) =>
child match {
- case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.length == 0 =>
+ case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty =>
val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limitValue)
if (limitPushed) {
sHolder.pushedLimit = Some(limitValue)
}
globalLimit
- case _ => globalLimit
+ case _ =>
+ child transform {
Review comment:
this looks scary, as we don't know what's between limit and sort, and it's dangerous to push down top N.
I think we need to do an exact match: `case Sort(..., ScanOperation...)`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r761937564
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceStrategy.scala
##########
@@ -726,6 +726,23 @@ object DataSourceStrategy
}
}
+ protected[sql] def translateSortOrders(sortOrders: Seq[SortOrder]): Seq[SortOrderV2] = {
+ sortOrders.map {
+ case SortOrder(PushableColumnWithoutNestedColumn(name), directionV1, nullOrderingV1, _) =>
+ val (directionV2, nullOrderingV2) = (directionV1, nullOrderingV1) match {
+ case (Ascending, NullsFirst) =>
+ (SortDirection.ASCENDING, NullOrdering.NULLS_FIRST)
+ case (Ascending, NullsLast) =>
+ (SortDirection.ASCENDING, NullOrdering.NULLS_LAST)
+ case (Descending, NullsFirst) =>
+ (SortDirection.DESCENDING, NullOrdering.NULLS_FIRST)
+ case (Descending, NullsLast) =>
+ (SortDirection.DESCENDING, NullOrdering.NULLS_LAST)
Review comment:
```
val directionV2 = directionV1 match ...
val nullOrderingV2 = ...
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r761983079
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala
##########
@@ -180,12 +181,14 @@ object JDBCRDD extends Logging {
* @param options - JDBC options that contains url, table and other information.
* @param outputSchema - The schema of the columns or aggregate columns to SELECT.
* @param groupByColumns - The pushed down group by columns.
+ * @param sample - The pushed down tableSample.
* @param limit - The pushed down limit. If the value is 0, it means no limit or limit
* is not pushed down.
- * @param sample - The pushed down tableSample.
Review comment:
Because the order of param comments is not consistent with the order of parameters.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981791163
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145711/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989969947
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50517/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-990125830
**[Test build #146041 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146041/testReport)** for PR 34738 at commit [`9c0cbc0`](https://github.com/apache/spark/commit/9c0cbc07c9b79ee3cc585bcb613f11cd4e214139).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989983349
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50516/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994410665
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146208/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r770010058
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceStrategy.scala
##########
@@ -726,6 +726,21 @@ object DataSourceStrategy
}
}
+ protected[sql] def translateSortOrders(sortOrders: Seq[SortOrder]): Seq[SortOrderV2] = {
+ sortOrders.map {
+ case SortOrder(PushableColumnWithoutNestedColumn(name), directionV1, nullOrderingV1, _) =>
Review comment:
In addition, this will cause `scala.MatchError` in Scala 2.12. We need a new test case which is not matched this case, @beliefer .
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r769175170
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -246,17 +247,35 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
}
}
+ private def pushDownLimit(plan: LogicalPlan, limit: Int): LogicalPlan = plan match {
+ case operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty =>
+ val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limit)
+ if (limitPushed) {
+ sHolder.pushedLimit = Some(limit)
+ }
+ operation
+ case s @ Sort(order, _, operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.isEmpty =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limit)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limit)
+ sHolder.sortValues = orders
+ operation
+ } else {
+ s
+ }
+ case p: Project =>
+ val newChild = pushDownLimit(p.child, limit)
+ p.withNewChildren(Seq(newChild))
+ case other => other
+ }
+
def applyLimit(plan: LogicalPlan): LogicalPlan = plan.transform {
case globalLimit @ Limit(IntegerLiteral(limitValue), child) =>
- child match {
- case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.length == 0 =>
- val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limitValue)
- if (limitPushed) {
- sHolder.pushedLimit = Some(limitValue)
- }
- globalLimit
- case _ => globalLimit
- }
+ val newChild = pushDownLimit(child, limitValue)
+ val newLocalLimit = globalLimit.child.asInstanceOf[LocalLimit].withNewChildren(Seq(newChild))
+ globalLimit
Review comment:
OK
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981508704
**[Test build #145711 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145711/testReport)** for PR 34738 at commit [`90e0250`](https://github.com/apache/spark/commit/90e02500ef7c8109e27423c50cf0e09ebc6c4e92).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983984304
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145817/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982644936
**[Test build #145758 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145758/testReport)** for PR 34738 at commit [`52213ed`](https://github.com/apache/spark/commit/52213edd20ab5535ebd9c01549b10a8cb4535bf3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r760697257
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsPushDownTopN.java
##########
@@ -0,0 +1,37 @@
+/*
+ * 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;
+import org.apache.spark.sql.catalyst.expressions.SortOrder;
+
+/**
+ * A mix-in interface for {@link Scan}. Data sources can implement this interface to
+ * push down top N. Please note that the combination of LIMIT with other operations
+ * such as AGGREGATE, GROUP BY, CLUSTER BY, DISTRIBUTE BY, etc. is NOT pushed down.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public interface SupportsPushDownTopN extends ScanBuilder {
+
+ /**
+ * Pushes down top N to the data source.
+ */
+ boolean pushTopN(SortOrder[] orders, int limit);
Review comment:
updated.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981627364
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50184/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985740305
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50386/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985663681
**[Test build #145911 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145911/testReport)** for PR 34738 at commit [`3b9d980`](https://github.com/apache/spark/commit/3b9d9804c4120d825b2bff8daf6212c4127a2e9e).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r768920956
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -246,17 +247,35 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
}
}
+ private def pushDownLimit(plan: LogicalPlan, limit: Int): LogicalPlan = plan match {
+ case operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty =>
+ val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limit)
+ if (limitPushed) {
+ sHolder.pushedLimit = Some(limit)
+ }
+ operation
+ case s @ Sort(order, _, operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.isEmpty =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limit)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limit)
+ sHolder.sortValues = orders
+ operation
+ } else {
+ s
+ }
+ case p: Project =>
+ val newChild = pushDownLimit(p.child, limit)
+ p.withNewChildren(Seq(newChild))
+ case other => other
+ }
+
def applyLimit(plan: LogicalPlan): LogicalPlan = plan.transform {
case globalLimit @ Limit(IntegerLiteral(limitValue), child) =>
- child match {
- case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.length == 0 =>
- val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limitValue)
- if (limitPushed) {
- sHolder.pushedLimit = Some(limitValue)
- }
- globalLimit
- case _ => globalLimit
- }
+ val newChild = pushDownLimit(child, limitValue)
+ val newLocalLimit = globalLimit.child.asInstanceOf[LocalLimit].withNewChildren(Seq(newChild))
+ globalLimit
Review comment:
Shall we also update the test to make sure the Sort operator is removed from the query plan?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994277684
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50682/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994693356
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146219/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995317082
Im gonna revert this because it did not pass the tests.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-990037101
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50517/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989929873
**[Test build #146042 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146042/testReport)** for PR 34738 at commit [`ac24274`](https://github.com/apache/spark/commit/ac24274f74d51a987bb1affa23e9894e6fa192c7).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r767462496
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala
##########
@@ -142,13 +142,25 @@ case class RowDataSourceScanExec(
handledFilters
}
+ val topNOrLimitInfo =
+ if (pushedDownOperators.limit.isDefined && pushedDownOperators.sortValues.nonEmpty) {
+ val pushedTopN =
+ s"""
+ |ORDER BY ${seqToString(pushedDownOperators.sortValues.map(_.describe()))}
+ |LIMIT ${pushedDownOperators.limit.get}
+ |""".stripMargin.replaceAll("\n", " ")
+ Some("pushedTopN" -> pushedTopN)
+ } else {
+ pushedDownOperators.limit.map(value => "PushedLimit" -> s"LIMIT $value")
Review comment:
indentation is wrong
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765790376
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -138,33 +140,117 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkPushedLimit(df4, false, 0)
checkAnswer(df4, Seq(Row(1, 19000.00)))
+ val name = udf { (x: String) => x.matches("cat|dav|amy") }
+ val sub = udf { (x: String) => x.substring(0, 3) }
val df5 = spark.read
.table("h2.test.employee")
- .sort("SALARY")
+ .select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
+ .filter(name($"shortName"))
.limit(1)
+ // LIMIT is pushed down only if all the filters are pushed down
checkPushedLimit(df5, false, 0)
- checkAnswer(df5, Seq(Row(1, "cathy", 9000.00, 1200.0)))
+ checkAnswer(df5, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkPushedLimit(df: DataFrame, pushed: Boolean, limit: Int): Unit = {
+ df.queryExecution.optimizedPlan.collect {
+ case relation: DataSourceV2ScanRelation => relation.scan match {
+ case v1: V1ScanWrapper =>
+ if (pushed) {
+ assert(v1.pushedDownOperators.limit === Some(limit))
+ } else {
+ assert(v1.pushedDownOperators.limit.isEmpty)
+ }
+ }
+ }
+ }
+
+ test("simple scan with top N") {
+ val df1 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary").limit(1)
+ val expectedSorts1 =
+ Seq(SortValue(FieldReference("salary"), SortDirection.ASCENDING, NullOrdering.NULLS_FIRST))
Review comment:
let's create a method to construct sort orders, to avoid duplicated code.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765775274
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala
##########
@@ -142,13 +142,25 @@ case class RowDataSourceScanExec(
handledFilters
}
+ val optionOutputMap =
+ if (pushedDownOperators.limit.isDefined && pushedDownOperators.sortValues.nonEmpty) {
+ val pushedTopN =
+ s"""
+ |ORDER BY ${seqToString(pushedDownOperators.sortValues.map(_.describe()))}
+ |LIMIT ${pushedDownOperators.limit.get}
Review comment:
nit: shall we output only one line? otherwise the plan EXPLAIN result may look very weird.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r760144783
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -255,7 +256,20 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
sHolder.pushedLimit = Some(limitValue)
}
globalLimit
- case _ => globalLimit
+ case _ =>
+ child transform {
+ case sort @ Sort(order, _, ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.length == 0 =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limitValue)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limitValue)
+ sHolder.sortValues = orders
+ }
+ sort
Review comment:
Yea I think we can remove the sort node from the query plan.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983706656
**[Test build #145817 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145817/testReport)** for PR 34738 at commit [`730e5d0`](https://github.com/apache/spark/commit/730e5d02f1cce5c829015a128e1a797121eeb6ce).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981579447
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50182/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981582570
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50176/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r761938227
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala
##########
@@ -180,12 +181,14 @@ object JDBCRDD extends Logging {
* @param options - JDBC options that contains url, table and other information.
* @param outputSchema - The schema of the columns or aggregate columns to SELECT.
* @param groupByColumns - The pushed down group by columns.
+ * @param sample - The pushed down tableSample.
* @param limit - The pushed down limit. If the value is 0, it means no limit or limit
* is not pushed down.
- * @param sample - The pushed down tableSample.
Review comment:
why do you move it before `limit`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r761935682
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsPushDownTopN.java
##########
@@ -0,0 +1,37 @@
+/*
+ * 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;
+import org.apache.spark.sql.connector.expressions.SortOrder;
+
+/**
+ * A mix-in interface for {@link Scan}. Data sources can implement this interface to
+ * push down top N. Please note that the combination of LIMIT with other operations
Review comment:
let's explain `top N` in the doc. It's LIMIT right after SORT.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992406457
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50607/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992441587
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50607/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992634291
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146132/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994693356
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146219/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994784544
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50705/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r770002006
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceStrategy.scala
##########
@@ -726,6 +726,21 @@ object DataSourceStrategy
}
}
+ protected[sql] def translateSortOrders(sortOrders: Seq[SortOrder]): Seq[SortOrderV2] = {
+ sortOrders.map {
Review comment:
Hi, All. This broke Scala 2.13 compilation.
```
[error] /home/runner/work/spark/spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceStrategy.scala:730:20: match may not be exhaustive.
[error] It would fail on the following input: SortOrder(_, _, _, _)
[error] sortOrders.map {
[error] ^
[warn] 24 warnings found
[error] one error found
[error] (sql / Compile / compileIncremental) Compilation failed
[error] Total time: 267 s (04:27), completed Dec 15, 2021 5:57:25 AM
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995178738
Could you make a follow-up to recover Scala 2.13, @beliefer ?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994412771
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50693/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981790061
**[Test build #145714 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145714/testReport)** for PR 34738 at commit [`63d9168`](https://github.com/apache/spark/commit/63d916864031aa2b30859138003a3b22e1bcfa9c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r760139088
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala
##########
@@ -149,6 +149,7 @@ case class RowDataSourceScanExec(
Map("PushedAggregates" -> seqToString(v.aggregateExpressions),
"PushedGroupByColumns" -> seqToString(v.groupByColumns))} ++
pushedDownOperators.limit.map(value => "PushedLimit" -> s"LIMIT $value") ++
+ Map("PushedSortOrders" -> seqToString(pushedDownOperators.sortValues)) ++
Review comment:
can we use `pushedTopN` and display both sort orders and limit n here?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983706656
**[Test build #145817 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145817/testReport)** for PR 34738 at commit [`730e5d0`](https://github.com/apache/spark/commit/730e5d02f1cce5c829015a128e1a797121eeb6ce).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982394314
**[Test build #145758 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145758/testReport)** for PR 34738 at commit [`52213ed`](https://github.com/apache/spark/commit/52213edd20ab5535ebd9c01549b10a8cb4535bf3).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r761940490
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -249,12 +250,30 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
def applyLimit(plan: LogicalPlan): LogicalPlan = plan.transform {
case globalLimit @ Limit(IntegerLiteral(limitValue), child) =>
child match {
- case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.length == 0 =>
+ case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty =>
val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limitValue)
if (limitPushed) {
sHolder.pushedLimit = Some(limitValue)
}
globalLimit
+ case Sort(order, _, ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.isEmpty =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limitValue)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limitValue)
+ sHolder.sortValues = orders
+ }
+ globalLimit
Review comment:
shall we remove the Sort node from the plan?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985660354
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50380/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994684474
**[Test build #146219 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146219/testReport)** for PR 34738 at commit [`eb32095`](https://github.com/apache/spark/commit/eb32095d4dc6967a9cb48e0335f04bfd023ff5a9).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994947920
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146231/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994481810
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50693/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #34738:
URL: https://github.com/apache/spark/pull/34738
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995330022
Thank you for the decision, @HyukjinKwon !
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r768923388
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -120,56 +122,119 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
.table("h2.test.employee")
.filter($"dept" > 1)
.limit(1)
- checkPushedLimit(df2, true, 1)
+ checkPushedLimit(df2, Some(1))
checkAnswer(df2, Seq(Row(2, "alex", 12000.00, 1200.0)))
val df3 = sql("SELECT name FROM h2.test.employee WHERE dept > 1 LIMIT 1")
val scan = df3.queryExecution.optimizedPlan.collectFirst {
case s: DataSourceV2ScanRelation => s
}.get
assert(scan.schema.names.sameElements(Seq("NAME")))
- checkPushedLimit(df3, true, 1)
+ checkPushedLimit(df3, Some(1))
checkAnswer(df3, Seq(Row("alex")))
val df4 = spark.read
.table("h2.test.employee")
.groupBy("DEPT").sum("SALARY")
.limit(1)
- checkPushedLimit(df4, false, 0)
+ checkPushedLimit(df4, None)
checkAnswer(df4, Seq(Row(1, 19000.00)))
- val df5 = spark.read
- .table("h2.test.employee")
- .sort("SALARY")
- .limit(1)
- checkPushedLimit(df5, false, 0)
- checkAnswer(df5, Seq(Row(1, "cathy", 9000.00, 1200.0)))
-
val name = udf { (x: String) => x.matches("cat|dav|amy") }
val sub = udf { (x: String) => x.substring(0, 3) }
- val df6 = spark.read
+ val df5 = spark.read
.table("h2.test.employee")
.select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
.filter(name($"shortName"))
.limit(1)
// LIMIT is pushed down only if all the filters are pushed down
- checkPushedLimit(df6, false, 0)
- checkAnswer(df6, Seq(Row(10000.00, 1000.0, "amy")))
+ checkPushedLimit(df5, None)
+ checkAnswer(df5, Seq(Row(10000.00, 1000.0, "amy")))
}
- private def checkPushedLimit(df: DataFrame, pushed: Boolean, limit: Int): Unit = {
+ private def checkPushedLimit(df: DataFrame, limit: Option[Int] = None,
+ sortValues: Seq[SortValue] = Nil): Unit = {
df.queryExecution.optimizedPlan.collect {
case relation: DataSourceV2ScanRelation => relation.scan match {
case v1: V1ScanWrapper =>
- if (pushed) {
- assert(v1.pushedDownOperators.limit === Some(limit))
- } else {
- assert(v1.pushedDownOperators.limit.isEmpty)
- }
+ assert(v1.pushedDownOperators.limit === limit)
+ assert(v1.pushedDownOperators.sortValues === sortValues)
}
}
}
+ test("simple scan with top N") {
+ val df1 = spark.read
+ .table("h2.test.employee")
+ .sort("salary")
+ .limit(1)
+ checkPushedLimit(df1, Some(1), createSortValues())
+ checkAnswer(df1, Seq(Row(1, "cathy", 9000.00, 1200.0)))
+
+ val df2 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary").limit(1)
+ checkPushedLimit(df2, Some(1), createSortValues())
+ checkAnswer(df2, Seq(Row(1, "cathy", 9000.00, 1200.0)))
+
+ val df3 = spark.read
+ .option("partitionColumn", "dept")
+ .option("lowerBound", "0")
+ .option("upperBound", "2")
+ .option("numPartitions", "2")
+ .table("h2.test.employee")
+ .filter($"dept" > 1)
+ .orderBy($"salary".desc)
+ .limit(1)
+ checkPushedLimit(
+ df3, Some(1), createSortValues(SortDirection.DESCENDING, NullOrdering.NULLS_LAST))
+ checkAnswer(df3, Seq(Row(2, "alex", 12000.00, 1200.0)))
+
+ val df4 =
+ sql("SELECT name FROM h2.test.employee WHERE dept > 1 ORDER BY salary NULLS LAST LIMIT 1")
+ val scan = df4.queryExecution.optimizedPlan.collectFirst {
+ case s: DataSourceV2ScanRelation => s
+ }.get
+ assert(scan.schema.names.sameElements(Seq("NAME", "SALARY")))
+ checkPushedLimit(df4, Some(1), createSortValues(nullOrdering = NullOrdering.NULLS_LAST))
+ checkAnswer(df4, Seq(Row("david")))
+
+ val df5 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary")
+ checkPushedLimit(df5, None, Seq.empty)
+ checkAnswer(df5, Seq(Row(1, "cathy", 9000.00, 1200.0), Row(1, "amy", 10000.00, 1000.0)))
+
+ val df6 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).limit(1)
Review comment:
This has been covered by the limit pushdown tests already.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r768921901
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -43,6 +44,7 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
.set("spark.sql.catalog.h2.driver", "org.h2.Driver")
.set("spark.sql.catalog.h2.pushDownAggregate", "true")
.set("spark.sql.catalog.h2.pushDownLimit", "true")
+ .set("spark.sql.catalog.h2.pushDownTopN", "true")
Review comment:
this is not needed now.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995438095
@beliefer your github action environment seems broken and I can hardly see a successful run in your github action in various PRs, and I have to rely on the Jenkins results. Maybe let's take this chance to fix your github action, please ping us in your re-submit PR and we can help you investigate if the github action consistently fails.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-990037101
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50517/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992634291
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146132/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992463145
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50607/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r767463374
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -246,16 +247,42 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
}
}
+ private def pushDownLimit(plan: LogicalPlan, limit: Int): LogicalPlan = plan match {
+ case operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty =>
+ val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limit)
+ if (limitPushed) {
+ sHolder.pushedLimit = Some(limit)
+ }
+ operation
+ case s @ Sort(order, _, operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.isEmpty =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limit)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limit)
+ sHolder.sortValues = orders
+ operation
+ } else {
+ s
+ }
+ case p: Project =>
+ val newChild = pushDownLimit(p.child, limit)
+ if (newChild == p.child) {
Review comment:
we can call `p.withNewChildren(Seq(newChild))`, which does this check for you.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981541382
**[Test build #145714 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145714/testReport)** for PR 34738 at commit [`63d9168`](https://github.com/apache/spark/commit/63d916864031aa2b30859138003a3b22e1bcfa9c).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981497506
**[Test build #145706 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145706/testReport)** for PR 34738 at commit [`4872dcc`](https://github.com/apache/spark/commit/4872dcc6cc3f0debb3f8c653110ee442a4e3d58c).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981496208
**[Test build #145706 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145706/testReport)** for PR 34738 at commit [`4872dcc`](https://github.com/apache/spark/commit/4872dcc6cc3f0debb3f8c653110ee442a4e3d58c).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982394314
**[Test build #145758 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145758/testReport)** for PR 34738 at commit [`52213ed`](https://github.com/apache/spark/commit/52213edd20ab5535ebd9c01549b10a8cb4535bf3).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r760142801
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushedDownOperators.scala
##########
@@ -25,4 +26,5 @@ import org.apache.spark.sql.connector.expressions.aggregate.Aggregation
case class PushedDownOperators(
aggregation: Option[Aggregation],
sample: Option[TableSampleInfo],
- limit: Option[Int])
+ limit: Option[Int],
+ sortValues: Seq[SortOrder])
Review comment:
let's add an assert that, `sortValues` can only present if limit is present
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983810243
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50292/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994252289
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50682/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994228821
**[Test build #146208 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146208/testReport)** for PR 34738 at commit [`4f69ee8`](https://github.com/apache/spark/commit/4f69ee87abaf9e4305911b2f104b8e4164e77a79).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994699634
retest this please
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995351128
> Im gonna revert this because it did not pass the tests.
Could I open another PR to finish this issue ? @dongjoon-hyun @HyukjinKwon cc @cloud-fan
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995448712
> @beliefer your github action environment seems broken and I can hardly see a successful run in your github action in various PRs, and I have to rely on the Jenkins results. Maybe let's take this chance to fix your github action, please ping us in your re-submit PR and we can help you investigate if the github action consistently fails with unknown reasons.
@cloud-fan Thank you help me fix the issue of github action environment.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983793874
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50292/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yikf commented on a change in pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
yikf commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r758343688
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -255,7 +256,20 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
sHolder.pushedLimit = Some(limitValue)
}
globalLimit
- case _ => globalLimit
+ case _ =>
+ child transform {
+ case sort @ Sort(order, _, ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.length == 0 =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
Review comment:
1. The improve is pushDown in V2 Relation, Is it necessary to in the DataSourceV2Strategy
2. Optimizer rely the DataSourceStrategy, As if the upper layer depended on lower layer
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981627083
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50182/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982495101
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50230/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981496208
**[Test build #145706 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145706/testReport)** for PR 34738 at commit [`4872dcc`](https://github.com/apache/spark/commit/4872dcc6cc3f0debb3f8c653110ee442a4e3d58c).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981497535
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145706/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981497535
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145706/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982127087
cc @cloud-fan
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985663681
**[Test build #145911 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145911/testReport)** for PR 34738 at commit [`3b9d980`](https://github.com/apache/spark/commit/3b9d9804c4120d825b2bff8daf6212c4127a2e9e).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985776309
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145905/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985842861
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145911/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989929873
**[Test build #146042 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146042/testReport)** for PR 34738 at commit [`ac24274`](https://github.com/apache/spark/commit/ac24274f74d51a987bb1affa23e9894e6fa192c7).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989942952
**[Test build #146042 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146042/testReport)** for PR 34738 at commit [`ac24274`](https://github.com/apache/spark/commit/ac24274f74d51a987bb1affa23e9894e6fa192c7).
* This patch **fails Java style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan edited a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan edited a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995438095
@beliefer your github action environment seems broken and I can hardly see a successful run in your github action in various PRs, and I have to rely on the Jenkins results. Maybe let's take this chance to fix your github action, please ping us in your re-submit PR and we can help you investigate if the github action consistently fails with unknown reasons.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992357130
**[Test build #146132 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146132/testReport)** for PR 34738 at commit [`082ded9`](https://github.com/apache/spark/commit/082ded98d4a8a2f5e39f2c672b035088494e4886).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982429646
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50230/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981680002
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50184/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981680002
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50184/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r769248348
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -194,39 +196,49 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
val scan = df4.queryExecution.optimizedPlan.collectFirst {
case s: DataSourceV2ScanRelation => s
}.get
- assert(scan.schema.names.sameElements(Seq("NAME", "SALARY")))
+ assert(scan.schema.names.sameElements(Seq("NAME")))
+ checkSortRemoved(df4)
checkPushedLimit(df4, Some(1), createSortValues(nullOrdering = NullOrdering.NULLS_LAST))
checkAnswer(df4, Seq(Row("david")))
val df5 = spark.read.table("h2.test.employee")
.where($"dept" === 1).orderBy($"salary")
- checkPushedLimit(df5, None, Seq.empty)
+ checkSortRemoved(df5, false)
+ checkPushedLimit(df5, None)
checkAnswer(df5, Seq(Row(1, "cathy", 9000.00, 1200.0), Row(1, "amy", 10000.00, 1000.0)))
- val df6 = spark.read.table("h2.test.employee")
- .where($"dept" === 1).limit(1)
- checkPushedLimit(df6, Some(1), Seq.empty)
- checkAnswer(df6, Seq(Row(1, "amy", 10000.00, 1000.0)))
-
- val df7 = spark.read
+ val df6 = spark.read
.table("h2.test.employee")
.groupBy("DEPT").sum("SALARY")
.orderBy("DEPT")
.limit(1)
- checkPushedLimit(df7)
- checkAnswer(df7, Seq(Row(1, 19000.00)))
+ checkSortRemoved(df6, false)
+ checkPushedLimit(df6)
+ checkAnswer(df6, Seq(Row(1, 19000.00)))
val name = udf { (x: String) => x.matches("cat|dav|amy") }
val sub = udf { (x: String) => x.substring(0, 3) }
- val df8 = spark.read
+ val df7 = spark.read
.table("h2.test.employee")
.select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
.filter(name($"shortName"))
.sort($"SALARY".desc)
.limit(1)
+ checkSortRemoved(df7, false)
// LIMIT is pushed down only if all the filters are pushed down
- checkPushedLimit(df8)
- checkAnswer(df8, Seq(Row(10000.00, 1000.0, "amy")))
+ checkPushedLimit(df7)
+ checkAnswer(df7, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkSortRemoved(df: DataFrame, removed: Boolean = true): Unit = {
Review comment:
Can we do this in `checkPushedLimit`? If `sortValues` is nonEmpty, we check if the sort has been removed from the query plan.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994276250
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50682/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994410665
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146208/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994772908
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50705/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994701781
**[Test build #146231 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146231/testReport)** for PR 34738 at commit [`eb32095`](https://github.com/apache/spark/commit/eb32095d4dc6967a9cb48e0335f04bfd023ff5a9).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981627083
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50182/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r760174691
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
##########
@@ -370,6 +370,8 @@ abstract class JdbcDialect extends Serializable with Logging{
*/
def supportsLimit(): Boolean = true
+ def supportsTopN(): Boolean = true
Review comment:
I guess not exists the one.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985569736
**[Test build #145905 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145905/testReport)** for PR 34738 at commit [`bf105ca`](https://github.com/apache/spark/commit/bf105ca9cb8e51872143fd6c47e49ab0d4510560).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r760145878
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
##########
@@ -370,6 +370,8 @@ abstract class JdbcDialect extends Serializable with Logging{
*/
def supportsLimit(): Boolean = true
+ def supportsTopN(): Boolean = true
Review comment:
is there any database that do not support limit and sort?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985842861
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145911/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983984304
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145817/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765782016
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala
##########
@@ -142,13 +142,25 @@ case class RowDataSourceScanExec(
handledFilters
}
+ val optionOutputMap =
+ if (pushedDownOperators.limit.isDefined && pushedDownOperators.sortValues.nonEmpty) {
+ val pushedTopN =
+ s"""
+ |ORDER BY ${seqToString(pushedDownOperators.sortValues.map(_.describe()))}
+ |LIMIT ${pushedDownOperators.limit.get}
Review comment:
OK
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-990133358
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146041/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r767597686
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -246,16 +247,42 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
}
}
+ private def pushDownLimit(plan: LogicalPlan, limit: Int): LogicalPlan = plan match {
+ case operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty =>
+ val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limit)
+ if (limitPushed) {
+ sHolder.pushedLimit = Some(limit)
+ }
+ operation
+ case s @ Sort(order, _, operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.isEmpty =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limit)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limit)
+ sHolder.sortValues = orders
+ operation
+ } else {
+ s
+ }
+ case p: Project =>
+ val newChild = pushDownLimit(p.child, limit)
+ if (newChild == p.child) {
Review comment:
Thank you for the point.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r768920647
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -246,17 +247,35 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
}
}
+ private def pushDownLimit(plan: LogicalPlan, limit: Int): LogicalPlan = plan match {
+ case operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty =>
+ val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limit)
+ if (limitPushed) {
+ sHolder.pushedLimit = Some(limit)
+ }
+ operation
+ case s @ Sort(order, _, operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.isEmpty =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limit)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limit)
+ sHolder.sortValues = orders
+ operation
+ } else {
+ s
+ }
+ case p: Project =>
+ val newChild = pushDownLimit(p.child, limit)
+ p.withNewChildren(Seq(newChild))
+ case other => other
+ }
+
def applyLimit(plan: LogicalPlan): LogicalPlan = plan.transform {
case globalLimit @ Limit(IntegerLiteral(limitValue), child) =>
- child match {
- case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.length == 0 =>
- val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limitValue)
- if (limitPushed) {
- sHolder.pushedLimit = Some(limitValue)
- }
- globalLimit
- case _ => globalLimit
- }
+ val newChild = pushDownLimit(child, limitValue)
+ val newLocalLimit = globalLimit.child.asInstanceOf[LocalLimit].withNewChildren(Seq(newChild))
+ globalLimit
Review comment:
```suggestion
globalLimit.copy(child = newLocalLimit)
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r761936457
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala
##########
@@ -142,13 +142,24 @@ case class RowDataSourceScanExec(
handledFilters
}
+ val pushedTopN =
+ if (pushedDownOperators.limit.isDefined && pushedDownOperators.sortValues.nonEmpty) {
+ s"""
+ |ORDER BY ${seqToString(pushedDownOperators.sortValues.map(_.describe()))}
+ |LIMIT ${pushedDownOperators.limit.get}
+ |""".stripMargin
+ } else {
+ ""
+ }
+
Map(
"ReadSchema" -> requiredSchema.catalogString,
"PushedFilters" -> seqToString(markedFilters.toSeq)) ++
pushedDownOperators.aggregation.fold(Map[String, String]()) { v =>
Map("PushedAggregates" -> seqToString(v.aggregateExpressions),
"PushedGroupByColumns" -> seqToString(v.groupByColumns))} ++
pushedDownOperators.limit.map(value => "PushedLimit" -> s"LIMIT $value") ++
+ Map("pushedTopN" -> pushedTopN) ++
Review comment:
We should only print the `pushedTopN` entry if there is a top n being pushed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r761942677
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -138,11 +140,79 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkPushedLimit(df4, false, 0)
checkAnswer(df4, Seq(Row(1, 19000.00)))
+ val name = udf { (x: String) => x.matches("cat|dav|amy") }
+ val sub = udf { (x: String) => x.substring(0, 3) }
val df5 = spark.read
.table("h2.test.employee")
- .sort("SALARY")
+ .select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
+ .filter(name($"shortName"))
.limit(1)
+ // LIMIT is pushed down only if all the filters are pushed down
checkPushedLimit(df5, false, 0)
+ checkAnswer(df5, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkPushedLimit(df: DataFrame, pushed: Boolean, limit: Int): Unit = {
+ df.queryExecution.optimizedPlan.collect {
+ case relation: DataSourceV2ScanRelation => relation.scan match {
+ case v1: V1ScanWrapper =>
+ if (pushed) {
+ assert(v1.pushedDownOperators.limit === Some(limit))
+ } else {
+ assert(v1.pushedDownOperators.limit.isEmpty)
+ }
+ }
+ }
+ }
+
+ test("simple scan with top N") {
+ val df1 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary").limit(1)
Review comment:
shall we also test that, sort without limit is not pushed?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r762066369
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -138,11 +140,79 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkPushedLimit(df4, false, 0)
checkAnswer(df4, Seq(Row(1, 19000.00)))
+ val name = udf { (x: String) => x.matches("cat|dav|amy") }
+ val sub = udf { (x: String) => x.substring(0, 3) }
val df5 = spark.read
.table("h2.test.employee")
- .sort("SALARY")
+ .select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
+ .filter(name($"shortName"))
.limit(1)
+ // LIMIT is pushed down only if all the filters are pushed down
checkPushedLimit(df5, false, 0)
+ checkAnswer(df5, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkPushedLimit(df: DataFrame, pushed: Boolean, limit: Int): Unit = {
+ df.queryExecution.optimizedPlan.collect {
+ case relation: DataSourceV2ScanRelation => relation.scan match {
+ case v1: V1ScanWrapper =>
+ if (pushed) {
+ assert(v1.pushedDownOperators.limit === Some(limit))
+ } else {
+ assert(v1.pushedDownOperators.limit.isEmpty)
+ }
+ }
+ }
+ }
+
+ test("simple scan with top N") {
+ val df1 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary").limit(1)
Review comment:
OK
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985697654
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50386/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985771054
**[Test build #145905 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145905/testReport)** for PR 34738 at commit [`bf105ca`](https://github.com/apache/spark/commit/bf105ca9cb8e51872143fd6c47e49ab0d4510560).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class TimedeltaOps(DataTypeOps):`
* `class TimedeltaIndex(Index):`
* `class MissingPandasLikeTimedeltaIndex(MissingPandasLikeIndex):`
* `class UDFBasicProfiler(BasicProfiler):`
* `class ExecutorPodsPollingSnapshotSource(`
* `class ExecutorPodsWatchSnapshotSource(`
* ` class AnsiCombinedTypeCoercionRule(rules: Seq[TypeCoercionRule]) extends`
* ` class CombinedTypeCoercionRule(rules: Seq[TypeCoercionRule]) extends TypeCoercionRule `
* `case class PrettyPythonUDF(`
* `case class CreateTable(`
* `case class TableSpec(`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985569736
**[Test build #145905 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145905/testReport)** for PR 34738 at commit [`bf105ca`](https://github.com/apache/spark/commit/bf105ca9cb8e51872143fd6c47e49ab0d4510560).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981537582
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50176/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r758929999
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsPushDownLimit.java
##########
@@ -33,4 +34,9 @@
* Pushes down LIMIT to the data source.
*/
boolean pushLimit(int limit);
+
+ /**
+ * Pushes down top N to the data source.
+ */
+ boolean pushTopN(SortValue[] orders, int limit);
Review comment:
or probably we should have a new interface `SupportsPushDownTopN`, as Spark can either push down limit, or top n, but not both.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r758929705
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsPushDownLimit.java
##########
@@ -18,6 +18,7 @@
package org.apache.spark.sql.connector.read;
import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.SortValue;
Review comment:
`SortValue` is `private[sql]`, we shouldn't expose it in the public APIs. We should use `SortOrder` instead
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981582570
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50176/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r758422144
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -255,7 +256,20 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
sHolder.pushedLimit = Some(limitValue)
}
globalLimit
- case _ => globalLimit
+ case _ =>
+ child transform {
+ case sort @ Sort(order, _, ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.length == 0 =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limitValue)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limitValue)
+ sHolder.sortValues = orders
+ }
+ sort
Review comment:
Some database supports partition sort.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982495101
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50230/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994784544
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50705/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994393344
**[Test build #146208 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146208/testReport)** for PR 34738 at commit [`4f69ee8`](https://github.com/apache/spark/commit/4f69ee87abaf9e4305911b2f104b8e4164e77a79).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994947920
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146231/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994351553
**[Test build #146219 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146219/testReport)** for PR 34738 at commit [`eb32095`](https://github.com/apache/spark/commit/eb32095d4dc6967a9cb48e0335f04bfd023ff5a9).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995366858
Yeah, sure. Please go ahead. Thanks @beliefer .
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992463145
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50607/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r767464502
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -120,52 +122,124 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
.table("h2.test.employee")
.filter($"dept" > 1)
.limit(1)
- checkPushedLimit(df2, true, 1)
+ checkPushedLimit(df2, Some(1))
checkAnswer(df2, Seq(Row(2, "alex", 12000.00, 1200.0)))
val df3 = sql("SELECT name FROM h2.test.employee WHERE dept > 1 LIMIT 1")
val scan = df3.queryExecution.optimizedPlan.collectFirst {
case s: DataSourceV2ScanRelation => s
}.get
assert(scan.schema.names.sameElements(Seq("NAME")))
- checkPushedLimit(df3, true, 1)
+ checkPushedLimit(df3, Some(1))
checkAnswer(df3, Seq(Row("alex")))
val df4 = spark.read
.table("h2.test.employee")
.groupBy("DEPT").sum("SALARY")
.limit(1)
- checkPushedLimit(df4, false, 0)
+ checkPushedLimit(df4, None)
checkAnswer(df4, Seq(Row(1, 19000.00)))
+ val name = udf { (x: String) => x.matches("cat|dav|amy") }
+ val sub = udf { (x: String) => x.substring(0, 3) }
val df5 = spark.read
.table("h2.test.employee")
- .sort("SALARY")
+ .select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
+ .filter(name($"shortName"))
+ .limit(1)
+ // LIMIT is pushed down only if all the filters are pushed down
+ checkPushedLimit(df5, None)
+ checkAnswer(df5, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkPushedLimit(df: DataFrame, limit: Option[Int]): Unit = {
Review comment:
shall we merge `checkPushedTopN` into this? `def checkPushedLimit(df: DataFrame, limit: Option[Int], sortValues: Seq[SortValue] = Nil)`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765776937
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala
##########
@@ -142,13 +142,25 @@ case class RowDataSourceScanExec(
handledFilters
}
+ val optionOutputMap =
+ if (pushedDownOperators.limit.isDefined && pushedDownOperators.sortValues.nonEmpty) {
+ val pushedTopN =
+ s"""
+ |ORDER BY ${seqToString(pushedDownOperators.sortValues.map(_.describe()))}
+ |LIMIT ${pushedDownOperators.limit.get}
+ |""".stripMargin
+ Map("pushedTopN" -> pushedTopN)
+ } else {
+ pushedDownOperators.aggregation.fold(Map[String, String]()) { v =>
+ Map("PushedAggregates" -> seqToString(v.aggregateExpressions),
+ "PushedGroupByColumns" -> seqToString(v.groupByColumns))} ++
+ pushedDownOperators.limit.map(value => "PushedLimit" -> s"LIMIT $value")
+ }
+
Map(
"ReadSchema" -> requiredSchema.catalogString,
"PushedFilters" -> seqToString(markedFilters.toSeq)) ++
- pushedDownOperators.aggregation.fold(Map[String, String]()) { v =>
- Map("PushedAggregates" -> seqToString(v.aggregateExpressions),
- "PushedGroupByColumns" -> seqToString(v.groupByColumns))} ++
- pushedDownOperators.limit.map(value => "PushedLimit" -> s"LIMIT $value") ++
Review comment:
Why do we remove them? I think we should just append the top n information
```
val topNInfo = if (pushedDownOperators.limit.isDefined && pushedDownOperators.sortValues.nonEmpty) {
Some(.. -> ...)
} else {
None
}
...
pushedDownOperators.limit.map(value => "PushedLimit" -> s"LIMIT $value") ++
topNInfo ++
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992357130
**[Test build #146132 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146132/testReport)** for PR 34738 at commit [`082ded9`](https://github.com/apache/spark/commit/082ded98d4a8a2f5e39f2c672b035088494e4886).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989881120
**[Test build #146041 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146041/testReport)** for PR 34738 at commit [`9c0cbc0`](https://github.com/apache/spark/commit/9c0cbc07c9b79ee3cc585bcb613f11cd4e214139).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989943056
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146042/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r767463616
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -246,16 +247,42 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
}
}
+ private def pushDownLimit(plan: LogicalPlan, limit: Int): LogicalPlan = plan match {
+ case operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty =>
+ val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limit)
+ if (limitPushed) {
+ sHolder.pushedLimit = Some(limit)
+ }
+ operation
+ case s @ Sort(order, _, operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.isEmpty =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limit)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limit)
+ sHolder.sortValues = orders
+ operation
+ } else {
+ s
+ }
+ case p: Project =>
+ val newChild = pushDownLimit(p.child, limit)
+ if (newChild == p.child) {
+ p
+ } else {
+ p.copy(child = newChild)
+ }
+ case other => other
+ }
+
def applyLimit(plan: LogicalPlan): LogicalPlan = plan.transform {
case globalLimit @ Limit(IntegerLiteral(limitValue), child) =>
- child match {
- case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.length == 0 =>
- val limitPushed = PushDownUtils.pushLimit(sHolder.builder, limitValue)
- if (limitPushed) {
- sHolder.pushedLimit = Some(limitValue)
- }
- globalLimit
- case _ => globalLimit
+ val newChild = pushDownLimit(child, limitValue)
+ if (newChild == child) {
Review comment:
ditto
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765789518
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -138,33 +140,117 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkPushedLimit(df4, false, 0)
checkAnswer(df4, Seq(Row(1, 19000.00)))
+ val name = udf { (x: String) => x.matches("cat|dav|amy") }
+ val sub = udf { (x: String) => x.substring(0, 3) }
val df5 = spark.read
.table("h2.test.employee")
- .sort("SALARY")
+ .select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
+ .filter(name($"shortName"))
.limit(1)
+ // LIMIT is pushed down only if all the filters are pushed down
checkPushedLimit(df5, false, 0)
- checkAnswer(df5, Seq(Row(1, "cathy", 9000.00, 1200.0)))
+ checkAnswer(df5, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkPushedLimit(df: DataFrame, pushed: Boolean, limit: Int): Unit = {
Review comment:
not related to this PR, but I think `def checkPushedLimit(df: DataFrame, limit: Option[Int])` is better.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r769257701
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -194,39 +196,49 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
val scan = df4.queryExecution.optimizedPlan.collectFirst {
case s: DataSourceV2ScanRelation => s
}.get
- assert(scan.schema.names.sameElements(Seq("NAME", "SALARY")))
+ assert(scan.schema.names.sameElements(Seq("NAME")))
+ checkSortRemoved(df4)
checkPushedLimit(df4, Some(1), createSortValues(nullOrdering = NullOrdering.NULLS_LAST))
checkAnswer(df4, Seq(Row("david")))
val df5 = spark.read.table("h2.test.employee")
.where($"dept" === 1).orderBy($"salary")
- checkPushedLimit(df5, None, Seq.empty)
+ checkSortRemoved(df5, false)
+ checkPushedLimit(df5, None)
checkAnswer(df5, Seq(Row(1, "cathy", 9000.00, 1200.0), Row(1, "amy", 10000.00, 1000.0)))
- val df6 = spark.read.table("h2.test.employee")
- .where($"dept" === 1).limit(1)
- checkPushedLimit(df6, Some(1), Seq.empty)
- checkAnswer(df6, Seq(Row(1, "amy", 10000.00, 1000.0)))
-
- val df7 = spark.read
+ val df6 = spark.read
.table("h2.test.employee")
.groupBy("DEPT").sum("SALARY")
.orderBy("DEPT")
.limit(1)
- checkPushedLimit(df7)
- checkAnswer(df7, Seq(Row(1, 19000.00)))
+ checkSortRemoved(df6, false)
+ checkPushedLimit(df6)
+ checkAnswer(df6, Seq(Row(1, 19000.00)))
val name = udf { (x: String) => x.matches("cat|dav|amy") }
val sub = udf { (x: String) => x.substring(0, 3) }
- val df8 = spark.read
+ val df7 = spark.read
.table("h2.test.employee")
.select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
.filter(name($"shortName"))
.sort($"SALARY".desc)
.limit(1)
+ checkSortRemoved(df7, false)
// LIMIT is pushed down only if all the filters are pushed down
- checkPushedLimit(df8)
- checkAnswer(df8, Seq(Row(10000.00, 1000.0, "amy")))
+ checkPushedLimit(df7)
+ checkAnswer(df7, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkSortRemoved(df: DataFrame, removed: Boolean = true): Unit = {
Review comment:
OK
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-995369566
> Yeah, sure. Please go ahead. Thanks @beliefer .
Thank you.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994944976
**[Test build #146231 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146231/testReport)** for PR 34738 at commit [`eb32095`](https://github.com/apache/spark/commit/eb32095d4dc6967a9cb48e0335f04bfd023ff5a9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994228821
**[Test build #146208 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146208/testReport)** for PR 34738 at commit [`4f69ee8`](https://github.com/apache/spark/commit/4f69ee87abaf9e4305911b2f104b8e4164e77a79).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994475125
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50693/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981760562
**[Test build #145711 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145711/testReport)** for PR 34738 at commit [`90e0250`](https://github.com/apache/spark/commit/90e02500ef7c8109e27423c50cf0e09ebc6c4e92).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yikf commented on a change in pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
yikf commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r758343688
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -255,7 +256,20 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
sHolder.pushedLimit = Some(limitValue)
}
globalLimit
- case _ => globalLimit
+ case _ =>
+ child transform {
+ case sort @ Sort(order, _, ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.length == 0 =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
Review comment:
1. The improve is pushDown in V2 Relation, Is it necessary to in the DataSourceV2Strategy
2. Optimizer rely the DataSourceStrategy, As if the upper layer depended on lower layer
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981679955
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50184/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981792317
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145714/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] c21 commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
c21 commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981953879
cc @huaxingao FYI.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981508704
**[Test build #145711 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145711/testReport)** for PR 34738 at commit [`90e0250`](https://github.com/apache/spark/commit/90e02500ef7c8109e27423c50cf0e09ebc6c4e92).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983982638
**[Test build #145817 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145817/testReport)** for PR 34738 at commit [`730e5d0`](https://github.com/apache/spark/commit/730e5d02f1cce5c829015a128e1a797121eeb6ce).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983200306
ping @cloud-fan
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982646648
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145758/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985660354
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50380/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985607139
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50380/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985776309
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145905/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985836480
**[Test build #145911 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145911/testReport)** for PR 34738 at commit [`3b9d980`](https://github.com/apache/spark/commit/3b9d9804c4120d825b2bff8daf6212c4127a2e9e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982483660
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50230/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981541382
**[Test build #145714 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145714/testReport)** for PR 34738 at commit [`63d9168`](https://github.com/apache/spark/commit/63d916864031aa2b30859138003a3b22e1bcfa9c).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981581289
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50176/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981792317
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145714/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981791163
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145711/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-983810243
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50292/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-982646648
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145758/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-981612751
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50182/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yikf commented on a change in pull request #34738: [SPARK-37483][SQL] Support pushdown down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
yikf commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r758361092
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsPushDownLimit.java
##########
@@ -33,4 +34,9 @@
* Pushes down LIMIT to the data source.
*/
boolean pushLimit(int limit);
+
+ /**
+ * Pushes down top N to the data source.
+ */
+ boolean pushTopN(SortValue[] orders, int limit);
Review comment:
There is a little strange that `pushTopN` with the return value of the `Boolean`, How about adding two methods like `pushTopN` and `pushedTopN`, In this way, the responsibilities of each method are cleaner. FYI
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -255,7 +256,20 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
sHolder.pushedLimit = Some(limitValue)
}
globalLimit
- case _ => globalLimit
+ case _ =>
+ child transform {
+ case sort @ Sort(order, _, ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.length == 0 =>
Review comment:
How about using `filter.isEmpty`?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -255,7 +256,20 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
sHolder.pushedLimit = Some(limitValue)
}
globalLimit
- case _ => globalLimit
+ case _ =>
+ child transform {
+ case sort @ Sort(order, _, ScanOperation(_, filter, sHolder: ScanBuilderHolder))
+ if filter.length == 0 =>
+ val orders = DataSourceStrategy.translateSortOrders(order)
+ val topNPushed = PushDownUtils.pushTopN(sHolder.builder, orders.toArray, limitValue)
+ if (topNPushed) {
+ sHolder.pushedLimit = Some(limitValue)
+ sHolder.sortValues = orders
+ }
+ sort
Review comment:
The limit with sort has been pushed done, Whether the sort node can be remove?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r760148616
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsPushDownTopN.java
##########
@@ -0,0 +1,37 @@
+/*
+ * 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;
+import org.apache.spark.sql.catalyst.expressions.SortOrder;
+
+/**
+ * A mix-in interface for {@link Scan}. Data sources can implement this interface to
+ * push down top N. Please note that the combination of LIMIT with other operations
+ * such as AGGREGATE, GROUP BY, CLUSTER BY, DISTRIBUTE BY, etc. is NOT pushed down.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public interface SupportsPushDownTopN extends ScanBuilder {
+
+ /**
+ * Pushes down top N to the data source.
+ */
+ boolean pushTopN(SortOrder[] orders, int limit);
Review comment:
We can not expose catalyst expressions in the public API, we should use `org.apache.spark.sql.connector.expressions.SortOrder`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985652629
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50380/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985740305
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50386/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-985735717
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50386/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r768235280
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala
##########
@@ -142,13 +142,25 @@ case class RowDataSourceScanExec(
handledFilters
}
+ val topNOrLimitInfo =
+ if (pushedDownOperators.limit.isDefined && pushedDownOperators.sortValues.nonEmpty) {
+ val pushedTopN =
Review comment:
OK
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r767462354
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala
##########
@@ -142,13 +142,25 @@ case class RowDataSourceScanExec(
handledFilters
}
+ val topNOrLimitInfo =
+ if (pushedDownOperators.limit.isDefined && pushedDownOperators.sortValues.nonEmpty) {
+ val pushedTopN =
Review comment:
```
val pushedTopN = s"ORDER BY ${seqToString(pushedDownOperators.sortValues.map(_.describe()))}" +
" LIMIT ${pushedDownOperators.limit.get}"
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-992617824
**[Test build #146132 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146132/testReport)** for PR 34738 at commit [`082ded9`](https://github.com/apache/spark/commit/082ded98d4a8a2f5e39f2c672b035088494e4886).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r770010058
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceStrategy.scala
##########
@@ -726,6 +726,21 @@ object DataSourceStrategy
}
}
+ protected[sql] def translateSortOrders(sortOrders: Seq[SortOrder]): Seq[SortOrderV2] = {
+ sortOrders.map {
+ case SortOrder(PushableColumnWithoutNestedColumn(name), directionV1, nullOrderingV1, _) =>
Review comment:
In addition, this will cause `scala.MatchError` in Scala 2.12. We need a new test test which is not matched this case, @beliefer .
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994738379
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50705/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-994277684
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50682/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765786630
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -249,12 +250,37 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan] with PredicateHelper {
def applyLimit(plan: LogicalPlan): LogicalPlan = plan.transform {
case globalLimit @ Limit(IntegerLiteral(limitValue), child) =>
child match {
Review comment:
a simpler way to skip Project node
```
private def pushDownLimit(plan: LogicalPlan, limit: Int) = plan match {
case p: Project => pushDownLimit(p.child)
case ScanOperation(_, filter, sHolder: ScanBuilderHolder) if filter.isEmpty => ...
case Sort(order, _, operation @ ScanOperation(_, filter, sHolder: ScanBuilderHolder)) ...
}
def applyLimit(plan: LogicalPlan): LogicalPlan = plan.transform {
case globalLimit @ Limit(IntegerLiteral(limitValue), child) => pushDownLimit(child, limitValue)
}
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989917621
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50516/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989971822
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50516/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-990022473
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50517/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765774402
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsPushDownTopN.java
##########
@@ -0,0 +1,37 @@
+/*
+ * 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;
+import org.apache.spark.sql.connector.expressions.SortOrder;
+
+/**
+ * A mix-in interface for {@link Scan}. Data sources can implement this interface to push down
Review comment:
```suggestion
* A mix-in interface for {@link ScanBuilder}. Data sources can implement this interface to push down
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34738:
URL: https://github.com/apache/spark/pull/34738#discussion_r765793372
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
##########
@@ -138,33 +140,117 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkPushedLimit(df4, false, 0)
checkAnswer(df4, Seq(Row(1, 19000.00)))
+ val name = udf { (x: String) => x.matches("cat|dav|amy") }
+ val sub = udf { (x: String) => x.substring(0, 3) }
val df5 = spark.read
.table("h2.test.employee")
- .sort("SALARY")
+ .select($"SALARY", $"BONUS", sub($"NAME").as("shortName"))
+ .filter(name($"shortName"))
.limit(1)
+ // LIMIT is pushed down only if all the filters are pushed down
checkPushedLimit(df5, false, 0)
- checkAnswer(df5, Seq(Row(1, "cathy", 9000.00, 1200.0)))
+ checkAnswer(df5, Seq(Row(10000.00, 1000.0, "amy")))
+ }
+
+ private def checkPushedLimit(df: DataFrame, pushed: Boolean, limit: Int): Unit = {
+ df.queryExecution.optimizedPlan.collect {
+ case relation: DataSourceV2ScanRelation => relation.scan match {
+ case v1: V1ScanWrapper =>
+ if (pushed) {
+ assert(v1.pushedDownOperators.limit === Some(limit))
+ } else {
+ assert(v1.pushedDownOperators.limit.isEmpty)
+ }
+ }
+ }
+ }
+
+ test("simple scan with top N") {
+ val df1 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary").limit(1)
+ val expectedSorts1 =
+ Seq(SortValue(FieldReference("salary"), SortDirection.ASCENDING, NullOrdering.NULLS_FIRST))
+ checkPushedTopN(df1, true, 1, expectedSorts1)
+ checkAnswer(df1, Seq(Row(1, "cathy", 9000.00, 1200.0)))
+
+ val df2 = spark.read
+ .option("partitionColumn", "dept")
+ .option("lowerBound", "0")
+ .option("upperBound", "2")
+ .option("numPartitions", "2")
+ .table("h2.test.employee")
+ .filter($"dept" > 1)
+ .orderBy($"salary".desc)
+ .limit(1)
+ val expectedSorts2 =
+ Seq(SortValue(FieldReference("salary"), SortDirection.DESCENDING, NullOrdering.NULLS_LAST))
+ checkPushedTopN(df2, true, 1, expectedSorts2)
+ checkAnswer(df2, Seq(Row(2, "alex", 12000.00, 1200.0)))
+
+ val df3 =
+ sql("SELECT name FROM h2.test.employee WHERE dept > 1 ORDER BY salary NULLS LAST LIMIT 1")
+ val scan = df3.queryExecution.optimizedPlan.collectFirst {
+ case s: DataSourceV2ScanRelation => s
+ }.get
+ assert(scan.schema.names.sameElements(Seq("NAME")))
+ val expectedSorts3 =
+ Seq(SortValue(FieldReference("salary"), SortDirection.ASCENDING, NullOrdering.NULLS_LAST))
+ checkPushedTopN(df3, true, 1, expectedSorts3)
+ checkAnswer(df3, Seq(Row("david")))
+
+ val df4 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).orderBy($"salary")
+ checkPushedTopN(df4, false, 0)
+ checkAnswer(df4, Seq(Row(1, "cathy", 9000.00, 1200.0), Row(1, "amy", 10000.00, 1000.0)))
+
+ val df5 = spark.read.table("h2.test.employee")
+ .where($"dept" === 1).limit(1)
+ checkPushedTopN(df5, false, 1)
+ checkAnswer(df5, Seq(Row(1, "amy", 10000.00, 1000.0)))
+
+ val df6 = spark.read
+ .table("h2.test.employee")
+ .groupBy("DEPT").sum("SALARY")
+ .orderBy("DEPT")
+ .limit(1)
+ checkPushedTopN(df6, false, 0)
+ checkAnswer(df6, Seq(Row(1, 19000.00)))
+
+ val df7 = spark.read
Review comment:
This is probably the simplest case, why it's `df7` not `df1`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34738: [SPARK-37483][SQL] Support push down top N to JDBC data source V2
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34738:
URL: https://github.com/apache/spark/pull/34738#issuecomment-989943056
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146042/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org