airframe-sql: Support in-expression sub-query in traverse and transform #2698
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2698 +/- ##
==========================================
- Coverage 82.20% 82.19% -0.01%
==========================================
Files 337 337
Lines 14076 14102 +26
Branches 2216 2223 +7
==========================================
+ Hits 11571 11591 +20
- Misses 2505 2511 +6
Continue to review full report at Codecov.
|
| createInstance(newArgs) | ||
| } | ||
|
|
||
| def traversePlan[U](rule: PartialFunction[LogicalPlan, U]): Unit = { |
There was a problem hiding this comment.
Simply changing LogicalPlan.traverse() to accept PartialFunction[TreeNode[_], U] works, but if we do so, traverse() will be able to handle Expression, not only LogicalPlan although LogicalPlan already have traverseExpressions().
I think adding traversePlan() is more consistent in terms of API design unless we drop methods for Expression.
06e4b56 to
c3fbf07
Compare
c3fbf07 to
62f7734
Compare
|
LGTM. I've checked Spark SQL's code, and it seems it also uses different transform methods for Expression and LogicalPlan. It seems merging two types of traversals will be unnecessary. |
|
Merged |
|
Thank you! |
This will make possible to traverse/transform
LogicalPlaninExpression.Another idea is moving all traverse and transform methods (including
mapChildren) to TreeNode with a signature changeLogicalPlan->TreeNode[_]. This will change the ability of those methods that can traverse/transform any nodes in the tree, not onlyLogicalPlan. Meaning, no need to have methods forLogicalPlanandExpressionindependently.I'm not sure which is better. 🤔