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 2022/04/07 00:40:22 UTC
[GitHub] [spark] beliefer opened a new pull request, #36092: [SPARK-38391][SPARK-38768][SQL][FOLLOWUP] Add comments for `pushLimit` and `pushTopN` of `PushDownUtils`
beliefer opened a new pull request, #36092:
URL: https://github.com/apache/spark/pull/36092
### What changes were proposed in this pull request?
`pushLimit` and `pushTopN` of `PushDownUtils` returns tuple of boolean. It will be good to explain what the boolean value represents.
### Why are the changes needed?
Make DS V2 API more friendly to developers.
### Does this PR introduce _any_ user-facing change?
'No'.
Just update comments.
### How was this patch tested?
N/A
--
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 closed pull request #36092: [SPARK-38391][SPARK-38768][SQL][FOLLOWUP] Add comments for `pushLimit` and `pushTopN` of `PushDownUtils`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #36092: [SPARK-38391][SPARK-38768][SQL][FOLLOWUP] Add comments for `pushLimit` and `pushTopN` of `PushDownUtils`
URL: https://github.com/apache/spark/pull/36092
--
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 #36092: [SPARK-38391][SPARK-38768][SQL][FOLLOWUP] Add comments for `pushLimit` and `pushTopN` of `PushDownUtils`
Posted by GitBox <gi...@apache.org>.
beliefer commented on PR #36092:
URL: https://github.com/apache/spark/pull/36092#issuecomment-1093590707
ping @huaxingao 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] huaxingao commented on a diff in pull request #36092: [SPARK-38391][SPARK-38768][SQL][FOLLOWUP] Add comments for `pushLimit` and `pushTopN` of `PushDownUtils`
Posted by GitBox <gi...@apache.org>.
huaxingao commented on code in PR #36092:
URL: https://github.com/apache/spark/pull/36092#discussion_r846583108
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala:
##########
@@ -116,7 +116,11 @@ object PushDownUtils extends PredicateHelper {
}
/**
- * Pushes down LIMIT to the data source Scan
+ * Pushes down LIMIT to the data source Scan.
+ *
+ * @return the tuple of Boolean. The first Boolean value represents whether to push down, and
+ * the second Boolean value represents whether to push down partially which means to
+ * keep the `Limit`.
Review Comment:
can we be a little more clear and say something like
`...the second Boolean value represents whether to push down partially, which means Spark will keep the Limit and do it again.`
--
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 a diff in pull request #36092: [SPARK-38391][SPARK-38768][SQL][FOLLOWUP] Add comments for `pushLimit` and `pushTopN` of `PushDownUtils`
Posted by GitBox <gi...@apache.org>.
huaxingao commented on code in PR #36092:
URL: https://github.com/apache/spark/pull/36092#discussion_r846583474
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala:
##########
@@ -127,7 +131,11 @@ object PushDownUtils extends PredicateHelper {
}
/**
- * Pushes down top N to the data source Scan
+ * Pushes down top N to the data source Scan.
+ *
+ * @return the tuple of Boolean. The first Boolean value represents whether to push down, and
+ * the second Boolean value represents whether to push down partially which means to
+ * keep the `Sort` and `Limit`.
Review Comment:
same comment as the above
--
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 diff in pull request #36092: [SPARK-38391][SPARK-38768][SQL][FOLLOWUP] Add comments for `pushLimit` and `pushTopN` of `PushDownUtils`
Posted by GitBox <gi...@apache.org>.
beliefer commented on code in PR #36092:
URL: https://github.com/apache/spark/pull/36092#discussion_r846710829
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala:
##########
@@ -116,7 +116,11 @@ object PushDownUtils extends PredicateHelper {
}
/**
- * Pushes down LIMIT to the data source Scan
+ * Pushes down LIMIT to the data source Scan.
+ *
+ * @return the tuple of Boolean. The first Boolean value represents whether to push down, and
+ * the second Boolean value represents whether to push down partially which means to
+ * keep the `Limit`.
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 #36092: [SPARK-38391][SPARK-38768][SQL][FOLLOWUP] Add comments for `pushLimit` and `pushTopN` of `PushDownUtils`
Posted by GitBox <gi...@apache.org>.
beliefer commented on PR #36092:
URL: https://github.com/apache/spark/pull/36092#issuecomment-1094512833
@dongjoon-hyun Thank you. @huaxingao Thank you for your review.
--
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