LuciferYang commented on code in PR #12981:
URL: https://github.com/apache/gluten/pull/12981#discussion_r3965727357
##########
backends-clickhouse/src/test/scala/org/apache/gluten/execution/hive/GlutenClickHouseHiveTableSuite.scala:
##########
@@ -1038,25 +1038,13 @@ class GlutenClickHouseHiveTableSuite
spark.sql(
s"CREATE FUNCTION my_add as " +
s"'org.apache.hadoop.hive.contrib.udf.example.UDFExampleAdd2' USING
JAR '$jarUrl'")
- if (isSparkVersionLE("3.3")) {
- runQueryAndCompare("select MY_ADD(id, id+1) from range(10)")(
- checkGlutenPlan[ProjectExecTransformer])
- } else {
- runQueryAndCompare("select MY_ADD(id, id+1) from range(10)", noFallBack
= false)(_ => {})
- }
+ runQueryAndCompare("select MY_ADD(id, id+1) from range(10)", noFallBack =
false)(_ => {})
}
+ // TODO: the expected operator counts at the call sites below only held on
Spark 3.3. Re-derive
Review Comment:
It is removable in the sense that it asserts nothing today: the whole body
was inside `if (spark33)`, so on 3.4+ it has been a no-op. What removing it
also drops is the expected operator count at each call site, which is the only
record of what the plan used to look like. I kept them behind the TODO for that
reason rather than because the helper is worth keeping.
So either direction works from my side, and I would rather @zzcclp picked:
delete the helper and its call sites, or re-derive the counts on 3.5 and turn
the assertion back on. Same question for the copy in
`GlutenFunctionValidateSuite`. I left both untouched here so the choice is not
buried in a version-check cleanup.
--
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]
