[GLUTEN-1640] Support judging whether the execution plan has a fallback #1641
Conversation
|
Run Gluten Clickhouse CI |
| object FallbackUtil extends Logging { | ||
|
|
||
| def skip(plan: SparkPlan): Boolean = { | ||
| var skip = false |
There was a problem hiding this comment.
nit: looks unnecessary to use a variable.
| skip = true | ||
| case InputAdapter(_) => | ||
| skip = true | ||
| case BroadcastExchangeExec(_, _) => |
There was a problem hiding this comment.
Should we really need to skip BroadcastExchangeExec? I think we should do a consistent tackling as ShuffleExchangeExec.
There was a problem hiding this comment.
I think we should also skip vanilla spark's transition node: C2R/R2C, assuming transition node is not cared about by us in checking fallback. Right?
There was a problem hiding this comment.
In gluten , the processing of BroadcastExchangeExec and ShuffleExchangeExec is different. I think that BroadcastExchangeExec appears because of BroadcastHashJoinExec, so we ignore it. We only need to pay attention to BroadcastHashJoinExec. Don't worry about this now， i do not skip BroadcastExchangeExec now,
vanilla spark's transition node: C2R/R2C will be cast to GlutenColumnarToRowExecBase/GlutenRowToColumnarExec in gluten that has been ignored
| skip = true | ||
| case AdaptiveSparkPlanExec(_, _, _, _, _) => | ||
| skip = true | ||
| case _ => |
There was a problem hiding this comment.
ShuffleQueryStageExec & BroadcastQueryStageExec should also be skipped, right?
There was a problem hiding this comment.
add QueryStageExec
| return skip | ||
| } | ||
|
|
||
| def isFallback(plan: SparkPlan): Boolean = { |
There was a problem hiding this comment.
Nit: naming as hasFallback looks better.
gluten-core/src/main/scala/io/glutenproject/utils/FallbackUtil.scala
Outdated
Show resolved
Hide resolved
zheniantoushipashi
force-pushed
the
GLUTEN-1640
branch
from
df2ac73 to
f02c370
Compare
|
Run Gluten Clickhouse CI |
|
@PHILO-HE pls review again |
| object FallbackUtil extends Logging with AdaptiveSparkPlanHelper { | ||
|
|
||
| def skip(plan: SparkPlan): Boolean = { | ||
| return plan match { |
| true | ||
| case _: GlutenRowToColumnarExec => | ||
| true | ||
| case _: BaseSubqueryExec => |
There was a problem hiding this comment.
I note we have a plan named ColumnarSubqueryBroadcastExec which extending BaseSubqueryExec. Should we skip this one?
There was a problem hiding this comment.
i think wo shoud skip ColumnarSubqueryBroadcastExec, We should pay attention to the subclasses of TransformSupport, because these classes will call doValidate to determine whether to fallback， ColumnarSubqueryBroadcastExec is not a subClasses of TransformSupport
There was a problem hiding this comment.
zheniantoushipashi
force-pushed
the
GLUTEN-1640
branch
from
f02c370 to
43221a8
Compare
|
Run Gluten Clickhouse CI |
What changes were proposed in this pull request?
Support judging whether the execution plan has a fallback
How was this patch tested?
UT