ahshahid commented on PR #53658: URL: https://github.com/apache/spark/pull/53658#issuecomment-3713964328
Also pls note that, this change of transform down is only for the new Not created as children of Junction op.. that is basically processing the newly added Not nodes, right there, as otherwise it will get processed in next iteration of the rule. On Tue, Jan 6, 2026, 1:29 AM Asif Shahid ***@***.***> wrote: > I believe it's safe.. > If the original logic is modified such that instead of transform up , > transform down is used, then this bug would be fixed, but other cases like > that mentioned in Constant folding suite will break in idempotency. > To take care of both the cases, use of transform up and transform down is > needed...as in the pr > > On Tue, Jan 6, 2026, 12:31 AM Peter Toth ***@***.***> wrote: > >> ***@***.**** commented on this pull request. >> ------------------------------ >> >> In >> sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala >> <https://github.com/apache/spark/pull/53658#discussion_r2664100330>: >> >> > >> - case Not(a LessThan b) => GreaterThanOrEqual(a, b) >> - case Not(a LessThanOrEqual b) => GreaterThan(a, b) >> + case Not(a Or b) => >> + And(Not(a), Not(b)).transformDownWithPruning(_.containsPattern(NOT), ruleId) { >> >> Is this safe? I mean, before this PR the simplification logic of >> actualExprTransformer was called with transformUp..., but now you call >> it with transformDown... (please note that a Not node can be deep down >> in a or b). Is there any reason why we invoke the logic with transformUp >> or could the whole rule use transformDown on expression trees? >> >> — >> Reply to this email directly, view it on GitHub >> <https://github.com/apache/spark/pull/53658#pullrequestreview-3629937420>, >> or unsubscribe >> <https://github.com/notifications/unsubscribe-auth/AC6XG2DUVG2AXN2P4L5ADAT4FNXHFAVCNFSM6AAAAACQPKSJM6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMMRZHEZTONBSGA> >> . >> You are receiving this because you authored the thread.Message ID: >> ***@***.***> >> > -- 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]
