JoshRosen commented on code in PR #36654:
URL: https://github.com/apache/spark/pull/36654#discussion_r889431243
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -479,21 +479,24 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
* first to this node, then this node's subqueries and finally this node's
children.
* When the partial function does not apply to a given node, it is left
unchanged.
*/
- def transformDownWithSubqueries(f: PartialFunction[PlanType, PlanType]):
PlanType = {
+ def transformDownWithSubqueries(
Review Comment:
Instead of modifying this method's signature, I slightly prefer adding an
overload named `transformDownWithSubqueriesAndPruning` in order to remain
consistent with the naming conventions for other transform methods.
Adding a new method also avoids introducing source or binary compatibility
issues for third-party code that calls Catalyst APIs. Technically speaking,
Catalyst APIs are considered internal to Spark and are subject to change
between minor releases (see
[source](https://github.com/apache/spark/blob/bb51add5c79558df863d37965603387d40cc4387/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/package.scala#L20-L24)),
but I think it's nice to try to avoid API breakage when feasible.
I also ran into problems when trying to call `transformDownWithSubqueries`
without supplying any arguments in the first argument group: calling
`transformDownWithSubqueries() { f }` resulted in confusing compilation errors.
As a result, I'm going to submit a followup to this PR to split this into
two methods as I've suggested above.
--
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]
