rui-mo commented on code in PR #9107:
URL: https://github.com/apache/incubator-gluten/pull/9107#discussion_r2451179498
##########
gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -848,6 +856,14 @@ class ClickHouseTestSettings extends BackendTestSettings {
.includeCH("length check for input string values: nested in both map key
and value")
.includeCH("length check for input string values: nested in array of
struct")
.includeCH("length check for input string values: nested in array of
array")
+ .excludeGlutenTest("length check for input string values: top-level
columns")
Review Comment:
Would you please add comments above to illustrate the reason to exclude
these tests (because they are rewritten due to ...)?
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala:
##########
@@ -144,6 +144,66 @@ object ExpressionConverter extends SQLConfHelper with
Logging {
DecimalArithmeticExpressionTransformer(substraitName, leftChild,
rightChild, resultType, b)
}
+ private def replaceStaticInvokeWithExpressionTransformer(
+ i: StaticInvoke,
+ attributeSeq: Seq[Attribute],
+ expressionsMap: Map[Class[_], String]): ExpressionTransformer = {
+ def validateAndTransform(
+ exprName: String,
+ childTransformers: => Seq[ExpressionTransformer]):
ExpressionTransformer = {
+ if (!BackendsApiManager.getValidatorApiInstance.doExprValidate(exprName,
i)) {
+ throw new GlutenNotSupportException(
+ s"Not supported to map current ${i.getClass} call on function:
${i.functionName}.")
+ }
+ GenericExpressionTransformer(exprName, childTransformers, i)
+ }
+
+ i.functionName match {
+ case "encode" | "decode" if i.objectName.endsWith("UrlCodec") =>
+ validateAndTransform(
+ "url_" + i.functionName,
+ Seq(replaceWithExpressionTransformer0(i.arguments.head,
attributeSeq, expressionsMap))
+ )
+
+ case "isLuhnNumber" =>
+ validateAndTransform(
+ ExpressionNames.LUHN_CHECK,
+ Seq(replaceWithExpressionTransformer0(i.arguments.head,
attributeSeq, expressionsMap))
+ )
+
+ case "encode" | "decode" if i.objectName.endsWith("Base64") =>
+ if
(!BackendsApiManager.getValidatorApiInstance.doExprValidate(ExpressionNames.BASE64,
i)) {
+ throw new GlutenNotSupportException(
+ s"Not supported to map current ${i.getClass} call on function:
${i.functionName}.")
+ }
+
BackendsApiManager.getSparkPlanExecApiInstance.genBase64StaticInvokeTransformer(
+ ExpressionNames.BASE64,
+ replaceWithExpressionTransformer0(i.arguments.head, attributeSeq,
expressionsMap),
+ i
+ )
+
+ case fn
+ if i.objectName.endsWith("CharVarcharCodegenUtils") && Set(
+ "varcharTypeWriteSideCheck",
+ "charTypeWriteSideCheck",
+ "readSidePadding").contains(fn) =>
+ val exprName = fn match {
+ case "varcharTypeWriteSideCheck" =>
ExpressionNames.VARCHAR_TYPE_WRITE_SIDE_CHECK
+ case "charTypeWriteSideCheck" =>
ExpressionNames.CHAR_TYPE_WRITE_SIDE_CHECK
+ case "readSidePadding" => ExpressionNames.READ_SIDE_PADDING
Review Comment:
The mappings from Spark's name to Velox's name are more commonly to be put
at
https://github.com/apache/incubator-gluten/blob/main/cpp/velox/substrait/SubstraitParser.cc#L387
to allow compatibility among different backends.
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala:
##########
@@ -144,6 +144,66 @@ object ExpressionConverter extends SQLConfHelper with
Logging {
DecimalArithmeticExpressionTransformer(substraitName, leftChild,
rightChild, resultType, b)
}
+ private def replaceStaticInvokeWithExpressionTransformer(
+ i: StaticInvoke,
+ attributeSeq: Seq[Attribute],
+ expressionsMap: Map[Class[_], String]): ExpressionTransformer = {
+ def validateAndTransform(
+ exprName: String,
+ childTransformers: => Seq[ExpressionTransformer]):
ExpressionTransformer = {
+ if (!BackendsApiManager.getValidatorApiInstance.doExprValidate(exprName,
i)) {
+ throw new GlutenNotSupportException(
+ s"Not supported to map current ${i.getClass} call on function:
${i.functionName}.")
+ }
+ GenericExpressionTransformer(exprName, childTransformers, i)
+ }
+
+ i.functionName match {
+ case "encode" | "decode" if i.objectName.endsWith("UrlCodec") =>
+ validateAndTransform(
+ "url_" + i.functionName,
+ Seq(replaceWithExpressionTransformer0(i.arguments.head,
attributeSeq, expressionsMap))
+ )
+
+ case "isLuhnNumber" =>
+ validateAndTransform(
+ ExpressionNames.LUHN_CHECK,
+ Seq(replaceWithExpressionTransformer0(i.arguments.head,
attributeSeq, expressionsMap))
+ )
+
+ case "encode" | "decode" if i.objectName.endsWith("Base64") =>
+ if
(!BackendsApiManager.getValidatorApiInstance.doExprValidate(ExpressionNames.BASE64,
i)) {
+ throw new GlutenNotSupportException(
+ s"Not supported to map current ${i.getClass} call on function:
${i.functionName}.")
+ }
Review Comment:
Thanks for the refactor. This validations seem to be newly introduced, would
you please clarify a bit?
--
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]
