peter-toth commented on PR #42223: URL: https://github.com/apache/spark/pull/42223#issuecomment-1666495642
> > But honestly, I don't like the idea that we duplicate the code of MergeScalarSubqueries.tryMergePlans() with some > > modifications into this new rule just because we don't want to fix agregate merging there. IMO if we all are on the same > > page about the discussed solution in [#42223 (comment)](https://github.com/apache/spark/pull/42223#issuecomment-1665208447) then we > > should update MergeScalarSubqueries.tryMergePlans() (in #37630) and use the > > updated common merging logic in this PR. > > I'm also don't like the idea as you said. But it seems the code is different between this PR and #37630. Maybe we should advance these two PRs separately until a shared opportunity is discovered, and then reconstruct both. IMO the codes are very similar and the basic algorightm is identical. Both methods are just traversing the plan and comparing nodes if/how they can be merged. I'm off for a week, but once returned I can update https://github.com/apache/spark/pull/42223 with the discussed and maybe remove the physical scan equality check part. Although that part is useful as in some cases we don't need to use any heuristics to allow merging (https://github.com/apache/spark/pull/42223#issuecomment-1665208447), it makes my PR complex. Maybe we can add it back later... BTW this PR still doesn't use the "condition expression cost is low" heuristics that we discussed. The current "isLikelySelective" doesn't make sense (https://github.com/apache/spark/pull/42223#discussion_r1281846237) and has nothing to do with expression cost. -- 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]
