PHILO-HE commented on code in PR #9394:
URL: https://github.com/apache/incubator-gluten/pull/9394#discussion_r2054552886
##########
cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc:
##########
@@ -232,6 +232,18 @@ bool SubstraitToVeloxPlanValidator::validateScalarFunction(
return true;
}
+bool isValidArrayCast(const TypePtr& fromType, const TypePtr& toType) {
+ const auto& fromElem = fromType->asArray().elementType();
Review Comment:
This variable is not used. Better to remove it?
##########
cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc:
##########
@@ -232,6 +232,18 @@ bool SubstraitToVeloxPlanValidator::validateScalarFunction(
return true;
}
+bool isValidArrayCast(const TypePtr& fromType, const TypePtr& toType) {
Review Comment:
Maybe, better to name it like `isSupportedArrayCast`. "valid" give me a
feeling that some cast cases are acceptable in Spark semantic and some are not.
But here, we just check whether it is supported in native execution.
##########
gluten-ut/spark33/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -626,6 +626,10 @@ class ClickHouseTestSettings extends BackendTestSettings {
.exclude("SPARK-36924: Cast YearMonthIntervalType to IntegralType")
.exclude("SPARK-36924: Cast IntegralType to YearMonthIntervalType")
.exclude("Cast should output null for invalid strings when ANSI is not
enabled.")
+ .exclude("cast array from IntegerType to StringType")
+ .exclude("cast array from DoubleType to StringType")
+ .exclude("cast array from BooleanType to StringType")
+ .exclude("cast array from DateType to StringType")
Review Comment:
Why we need exclude these tests for CH backend? In theory, the execution
should fall back for CH backend.
##########
gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenCastSuite.scala:
##########
@@ -43,6 +43,56 @@ class GlutenCastSuite extends CastSuite with
GlutenTestsTrait {
UDTRegistration.register(classOf[IExampleBaseType].getName,
classOf[ExampleBaseTypeUDT].getName)
UDTRegistration.register(classOf[IExampleSubType].getName,
classOf[ExampleSubTypeUDT].getName)
+ test("cast array from IntegerType to StringType") {
Review Comment:
Are these tests designed by yourself? Can we put these new tests into
`gluten-ut/test`? Generally, we only put revised Spark test in GlutenXXXSuite
which is under spark32/33/34/35. And by putting them into `gluten-ut/test`, all
Spark version tests and both backends will be covered.
--
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]
