-
Notifications
You must be signed in to change notification settings - Fork 75
[NSE-770] [NSE-774] Fix runtime issues on spark 3.2 #773
Conversation
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/oap-project/native-sql-engine/issues Then could you also rename commit message and pull request title in the following format?
See also: |
(that canEqual this) && super.equals(that) | ||
case _ => false | ||
} | ||
|
||
// For spark3.2. | ||
override protected def withNewChildInternal(newChild: SparkPlan): ColumnarBroadcastExchangeAdaptor = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like no need to override here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, should remove it for spark 3.1 compatibility. Just fixed in a new commit.
The above commits have been cherry-picked into gazelle 1.3.1 branch. |
@@ -375,6 +375,12 @@ case class ColumnarPostOverrides() extends Rule[SparkPlan] { | |||
var isSupportAdaptive: Boolean = true | |||
|
|||
def replaceWithColumnarPlan(plan: SparkPlan): SparkPlan = plan match { | |||
case RowToColumnarExec(child: BroadcastQueryStageExec) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also make a note here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added. Thanks!
case RowToColumnarExec(child: BroadcastQueryStageExec) => | ||
logDebug(s"$child Due to a fallback of BHJ inserted into plan." + | ||
s" See above override in BroadcastQueryStageExec") | ||
val localBroadcastXchg = child.plan.asInstanceOf[BroadcastExchangeExec] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may need to guard this logic, like check if child.isInstanceOf[xxx] first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in new commit. Thanks!
e89fd4e
to
773414e
Compare
Note: Spark 321 is ok to run on AQE + DPP with this patch |
Signed-off-by: Yuan Zhou <[email protected]>
Signed-off-by: Yuan Zhou <[email protected]>
This reverts commit ddc8631.
This reverts commit 2268205.
Signed-off-by: Yuan Zhou <[email protected]>
773414e
to
50594bc
Compare
Just rebased the code. |
This PR includes some fixes for runtime issues on spark 3.2. Thanks Yuan's help in fixing the issues when AQE is turned on.