Github user tejasapatil commented on a diff in the pull request:
https://github.com/apache/spark/pull/19257#discussion_r153343898
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/EnsureRequirements.scala
---
@@ -265,6 +268,7 @@ case class EnsureRequirements(conf: SQLConf) extends
Rule[SparkPlan] {
if (childPartitioning.guarantees(partitioning)) child else
operator
case _ => operator
}
- case operator: SparkPlan => ensureDistributionAndOrdering(operator)
+ case operator: SparkPlan =>
+ ensureDistributionAndOrdering(reorderJoinPredicates.apply(operator))
--- End diff --
It feels like having a rule invoked in such fashion is in-consistent
compared to rest of the codebase .... from point of view of someone new to
codebase, it will look odd. I removed the rule and instead moved this methods
inside `EnsureRequirements`. Let me know how you feel about the changed version
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
