address_review_comments

Author: Bhargava Vadlamani

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 getIncompatibleReasons(): 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 _ =>
+    }
     val leftCast = Cast(children(0), StringType)
     val rightCast = Cast(children(1), LongType)
     val leftExpr = exprToProtoInternal(leftCast, inputs, binding)

spark/src/main/scala/org/apache/comet/serde/unixtime.scala

@@ -38,6 +38,8 @@ object CometFromUnixTime extends CometExpressionSerde[FromUnixTime] {
 
   override def getSupportLevel(expr: FromUnixTime): SupportLevel = {
     expr.format match {
+      case Literal(null, _) =>
+        Compatible(None)
       case Literal(fmt, _) =>
         val formatStr = fmt.toString
         val defaultPattern = TimestampFormatter.defaultPattern
@@ -55,6 +57,12 @@ object CometFromUnixTime extends CometExpressionSerde[FromUnixTime] {
       expr: FromUnixTime,
       inputs: Seq[Attribute],
       binding: Boolean): Option[ExprOuterClass.Expr] = {
+    expr.format match {
+      case Literal(null, _) =>
+        // from_unixtime is null-intolerant, so NULL format yields NULL
+        return exprToProtoInternal(Literal.create(null, StringType), inputs, binding)
+      case _ =>
+    }
     val secExpr = exprToProtoInternal(expr.sec, inputs, binding)
     // TODO: DataFusion toChar does not support Spark datetime pattern format
     // https://github.com/apache/datafusion/issues/16577

spark/src/test/resources/sql-tests/expressions/datetime/from_unix_time.sql

@@ -33,3 +33,13 @@ SELECT from_unixtime(0)
 
 query expect_fallback(Datetime pattern format: yyyy-MM-dd is unsupported)
 SELECT from_unixtime(1718451045, 'yyyy-MM-dd')
+
+-- null format literal
+query
+SELECT from_unixtime(t, NULL) FROM test_from_unix_time
+
+query
+SELECT from_unixtime(1718451045, NULL)
+
+query
+SELECT from_unixtime(1718451045, CAST(NULL AS STRING))

spark/src/test/resources/sql-tests/expressions/string/string_repeat.sql

@@ -35,3 +35,11 @@ SELECT repeat('hi', n) FROM test_repeat
 -- literal + literal
 query
 SELECT repeat('hi', 3), repeat('', 5), repeat('a', 0), repeat(NULL, 3)
+
+-- non-positive literal count
+query
+SELECT repeat('namaste', -1), repeat('namaste', -100), repeat('a', 0), repeat(NULL, -1)
+
+-- non-positive literal count over a column
+query
+SELECT repeat(s, -1), repeat(s, 0) FROM test_repeat