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/07/17 19:48:02 UTC
[GitHub] [spark] EnricoMi opened a new pull request, #37211: [SPARK-39644][SQL][WIP] Add RangePartitioning reporting for V2 DataSources
EnricoMi opened a new pull request, #37211:
URL: https://github.com/apache/spark/pull/37211
### What changes were proposed in this pull request?
DataSourceV2 allows data sources to report existing partitioning of read data (`org.apache.spark.sql.connector.read.partitioning`). Currently, there is only `KeyGroupedPartitioning` and `UnknownPartitioning`. Data sources should be able to report global ordered data so that downstream operations can exploit this.
The following is required for this to work:
- Define `RangePartitioning` as a new implementation of `Partitioning`
- Add Catalyst rules that handle this partitioning
- Add a test source that reports ordering to proof that subsequent operations requiring order do not invoke sorting data
### Why are the changes needed?
Transformations that require certain order or partitioning should not shuffle or sort data when that is redundant.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Tests in `DataSourceV2Suite`.
--
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] EnricoMi commented on pull request #37211: [SPARK-39644][SQL] Add RangePartitioning reporting for V2 DataSources
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #37211:
URL: https://github.com/apache/spark/pull/37211#issuecomment-1214972231
@sunchao @cloud-fan @HyukjinKwon this is ready for 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
[GitHub] [spark] AmplabJenkins commented on pull request #37211: [SPARK-39644][SQL][WIP] Add RangePartitioning reporting for V2 DataSources
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #37211:
URL: https://github.com/apache/spark/pull/37211#issuecomment-1186609369
Can one of the admins verify this patch?
--
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 diff in pull request #37211: [SPARK-39644][SQL] Add RangePartitioning reporting for V2 DataSources
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #37211:
URL: https://github.com/apache/spark/pull/37211#discussion_r948663939
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala:
##########
@@ -119,13 +119,16 @@ case class DataSourceV2Relation(
* @param output the output attributes of this relation
* @param keyGroupedPartitioning if set, the partitioning expressions that are used to split the
* rows in the scan across different partitions
- * @param ordering if set, the ordering provided by the scan
+ * @param rangePartitioning if set, the range partitioning expressions that are used to split the
+ * rows in the scan across different partitions
+ * @param ordering if set, the in-partition ordering provided by the scan
*/
case class DataSourceV2ScanRelation(
relation: DataSourceV2Relation,
scan: Scan,
output: Seq[AttributeReference],
keyGroupedPartitioning: Option[Seq[Expression]] = None,
+ rangePartitioning: Option[Seq[SortOrder]] = None,
Review Comment:
we need to clearly define the semantics here, as there are two sort orders now. More specially, what if a scan reports both `RangePartitioning` and ordering? There are a few options:
1. require the reported `RangePartitioning` and ordering to be compatible
2. let `RangePartitioning` only define cross-partitions ordering, while `SupportsReportOrdering` is for data ordering within each partition.
--
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] github-actions[bot] closed pull request #37211: [SPARK-39644][SQL] Add RangePartitioning reporting for V2 DataSources
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #37211: [SPARK-39644][SQL] Add RangePartitioning reporting for V2 DataSources
URL: https://github.com/apache/spark/pull/37211
--
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] EnricoMi commented on a diff in pull request #37211: [SPARK-39644][SQL] Add RangePartitioning reporting for V2 DataSources
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on code in PR #37211:
URL: https://github.com/apache/spark/pull/37211#discussion_r949983359
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala:
##########
@@ -119,13 +119,16 @@ case class DataSourceV2Relation(
* @param output the output attributes of this relation
* @param keyGroupedPartitioning if set, the partitioning expressions that are used to split the
* rows in the scan across different partitions
- * @param ordering if set, the ordering provided by the scan
+ * @param rangePartitioning if set, the range partitioning expressions that are used to split the
+ * rows in the scan across different partitions
+ * @param ordering if set, the in-partition ordering provided by the scan
*/
case class DataSourceV2ScanRelation(
relation: DataSourceV2Relation,
scan: Scan,
output: Seq[AttributeReference],
keyGroupedPartitioning: Option[Seq[Expression]] = None,
+ rangePartitioning: Option[Seq[SortOrder]] = None,
Review Comment:
Those semantics are not introduced by this change, Spark already has the notion of global order and in-partition order (so 2.). I agree, this has to be documented explicitly and carefully wherever this is relevant in the code.
Can you give an example of "incompatible" global and in-partition ordering? I think they are orthogonal if you think of range partitioning as a key-grouped partitioning with ordered keys while data inside partitions can be ordered arbitrarily.
--
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 diff in pull request #37211: [SPARK-39644][SQL] Add RangePartitioning reporting for V2 DataSources
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #37211:
URL: https://github.com/apache/spark/pull/37211#discussion_r1017640498
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ContinuousScanExec.scala:
##########
@@ -33,6 +33,7 @@ case class ContinuousScanExec(
@transient stream: ContinuousStream,
@transient start: Offset,
keyGroupedPartitioning: Option[Seq[Expression]] = None,
+ rangePartitioning: Option[Seq[SortOrder]] = None,
Review Comment:
We need to create an abstraction for this, instead of adding more and more fields to these scan 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] EnricoMi commented on pull request #37211: [SPARK-39644][SQL][WIP] Add RangePartitioning reporting for V2 DataSources
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #37211:
URL: https://github.com/apache/spark/pull/37211#issuecomment-1186598134
@sunchao here is the `RangePartitioning`. Tests show that data sources that report this partitioning avoid shuffle and sorting for supported right transformations.
The `RangePartitioning` is derived from `KeyGroupedPartitioning`, because it as well partitions by the sort keys, while in addition the keys are sorted.
Do you agree adding `rangePartitioning` to `DataSourceV2ScanRelation` and `DataSourceV2ScanExecBase` is the right thing, or should those `SortOrder` expressions be used as `keyGroupedPartitioning` instead and taken by `DataSourceV2Strategy` from there?
--
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] github-actions[bot] commented on pull request #37211: [SPARK-39644][SQL] Add RangePartitioning reporting for V2 DataSources
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37211:
URL: https://github.com/apache/spark/pull/37211#issuecomment-1613947443
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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