andygrove commented on code in PR #4111:
URL: https://github.com/apache/datafusion-comet/pull/4111#discussion_r3289680116
##########
spark/src/main/scala/org/apache/comet/serde/strings.scala:
##########
@@ -33,15 +33,21 @@ import
org.apache.comet.serde.QueryPlanSerde.{createBinaryExpr, exprToProtoInter
object CometStringRepeat extends CometExpressionSerde[StringRepeat] {
- override def getCompatibleNotes(): Seq[String] = Seq(
- "A negative argument for the number of times to repeat throws an
exception" +
- " instead of returning an empty string as Spark does")
-
override def convert(
expr: StringRepeat,
inputs: Seq[Attribute],
binding: Boolean): Option[ExprOuterClass.Expr] = {
val children = expr.children
+ children(1) match {
+ case Literal(count: Number, _) if count.longValue() <= 0 =>
+ // Match Spark: repeat(s, n) with n <= 0 returns "" (or NULL if s is
NULL)
+ val ifExpr = If(
+ IsNull(children(0)),
+ Literal.create(null, StringType),
+ Literal(UTF8String.EMPTY_UTF8, StringType))
+ return exprToProtoInternal(ifExpr, inputs, binding)
+ case _ =>
Review Comment:
Seems like negative repeat edge case is fixed in DF v53 ?
https://github.com/apache/datafusion/blame/4a41173ba3df9b5d47638599c819a1e6e46ad92b/datafusion/functions/src/string/repeat.rs#L165
--
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]
