LuciferYang commented on code in PR #12981:
URL: https://github.com/apache/gluten/pull/12981#discussion_r3964910043
##########
gluten-core/src/main/scala/org/apache/spark/util/SparkTaskUtil.scala:
##########
@@ -64,25 +64,8 @@ object SparkTaskUtil {
ctors.head
}
Review Comment:
Changed to `require` with a message in e68b841. You are right that the
elided case matters here: with `-Xdisable-assertions` the check disappears and
`ctors.head` silently picks whichever constructor comes first. Gluten does not
set that flag today, but a precondition on a reflective lookup should not
depend on that.
##########
gluten-ut/test/src/test/scala/org/apache/spark/sql/GlutenImplicitsTest.scala:
##########
@@ -140,8 +140,7 @@ class GlutenImplicitsTest extends GlutenQueryTest with
SharedSparkSession {
// Spark 3.3 counts one Gluten node here. Since 3.4 the CTAS is
executed as an
// ExecutedCommandExec, which collectFallbackNodes walks past without
counting anything,
// so the summary reports neither a Gluten node nor a fallback node.
Review Comment:
Fixed in e68b841. The 3.3 clause is gone; the comment now only explains the
current behaviour, that the CTAS runs as an `ExecutedCommandExec` which
`collectFallbackNodes` walks past without counting. Good catch: this was the
exact class of leftover the PR is meant to remove, and I edited the assertion
below it without touching the comment 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]
