rdblue commented on a change in pull request #29089:
URL: https://github.com/apache/spark/pull/29089#discussion_r453998376
##########
File path:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/EliminateSortsSuite.scala
##########
@@ -310,4 +310,109 @@ class EliminateSortsSuite extends PlanTest {
val correctAnswer = PushDownOptimizer.execute(noOrderByPlan.analyze)
comparePlans(optimized, correctAnswer)
}
+
+ testRepartitionOptimization(
Review comment:
I prefer not to replace `test` because it makes tests difficult to run
individually (at least in my IntelliJ environment).
It also tends to increase readability. Here, you're passing a function to
`testRepartitionOptimization` that gets passed a function that modifies the
logical plan. I think it would be easier to read if these were separate suites,
with a suite-level repartition function:
```scala
class EliminateSortsInRepartitionSuite extends ... {
def repartition(plan: LogicalPlan): LogicalPlan = plan.repartition(10)
test("remove sortBy") {
val plan = testRelation.select('a, 'b).sortBy('a.asc, 'b.desc)
val planWithRepartition = repartition(plan)
...
}
}
class EliminateSortsInRepartitionByExpressionSuite extends
EliminateSortsInRepartitionSuite {
override def repartition(plan: LogicalPlan): LogicalPlan =
plan.distribute('a, 'b)(10)
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
