peter-toth commented on code in PR #37851:
URL: https://github.com/apache/spark/pull/37851#discussion_r968733560
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Canonicalize.scala:
##########
@@ -65,6 +78,15 @@ object Canonicalize {
val newChildren = orderCommutative(l, { case Least(children) => children
})
Least(newChildren)
- case _ => e.withNewChildren(e.children.map(reorderCommutativeOperators))
+ case bc: EqualTo => orderBinaryComparison(bc, EqualTo)
+ case bc: EqualNullSafe => orderBinaryComparison(bc, EqualNullSafe)
+
+ case bc: GreaterThan => orderBinaryComparison(bc, LessThan)
+ case bc: LessThan => orderBinaryComparison(bc, GreaterThan)
+
+ case bc: GreaterThanOrEqual => orderBinaryComparison(bc, LessThanOrEqual)
+ case bc: LessThanOrEqual => orderBinaryComparison(bc, GreaterThanOrEqual)
+
+ case _ =>
e.mapChildren(preCanonicalizeAndReorderOperators).preCanonicalized
Review Comment:
Unfortunately
https://github.com/apache/spark/pull/37851/commits/88cb00616c06b9b9ae69b4c9c6f553b5ee949b4e
didn't work because `HigherOrderFunction.preCanonicalized` modifies its
children's `NamedLambdaVariable`s and reorder should happen after these
modifications.
But in
https://github.com/apache/spark/pull/37851/commits/99f8b61610421d21c71e72491cf71230c081b114
and
https://github.com/apache/spark/pull/37851/commits/6a38a0028f66f8d636fda4c7302f259bd051471d
I'm proposing a change that we break the full tree traversal in
`Canonicalize.reorderOperators`, and instead, we could let non commutative
operator's `preCanonicalized` to call their children's`canonicalized` to
initialte `Canonicalize.reorderOperators` on their children.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
