Skip to content

[SPARK-37904][SQL] Improve RebalancePartitions in rules of Optimizer#35208

Closed
ulysses-you wants to merge 2 commits intoapache:masterfrom
ulysses-you:rebalance
Closed

[SPARK-37904][SQL] Improve RebalancePartitions in rules of Optimizer#35208
ulysses-you wants to merge 2 commits intoapache:masterfrom
ulysses-you:rebalance

Conversation

@ulysses-you
Copy link
Contributor

What changes were proposed in this pull request?

Improve RebalancePartitions in following rules:

  • NestedColumnAliasing
  • CollapseRepartition
  • EliminateSorts
  • FoldablePropagation
  • PropagateEmptyRelationBase

Why are the changes needed?

After SPARK-37267, we support do optimize rebalance partitions in everywhere of plan rather than limit to the root node. So It should make sense to also let RebalancePartitions work in all rules of Optimizer like Repartition and RepartitionByExpression did.

Does this PR introduce any user-facing change?

no

How was this patch tested?

Add test in:

  • NestedColumnAliasingSuite
  • CollapseRepartitionSuite
  • EliminateSortsBeforeRepartitionSuite
  • FoldablePropagationSuite
  • PropagateEmptyRelationSuite

@github-actions github-actions bot added the SQL label Jan 14, 2022
@ulysses-you
Copy link
Contributor Author

cc @cloud-fan @viirya

val PROJECT: Value = Value
val RELATION_TIME_TRAVEL: Value = Value
val REPARTITION_OPERATION: Value = Value
val REBALANCE: Value = Value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: REBALANCE_PARTITION?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to REBALANCE_PARTITIONS

// test root node
val plan1 = emptyRelation.rebalance($"a").analyze
val optimized1 = Optimize.execute(plan1)
val expected1 = emptyRelation.analyze
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: expected1 and expected2 are same. Maybe just expected.

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@HyukjinKwon
Copy link
Member

cc @wangyum too

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in b50d450 Jan 17, 2022
@ulysses-you ulysses-you deleted the rebalance branch January 17, 2022 09:52
@ulysses-you
Copy link
Contributor Author

thank you all !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants