address_review_comments

Author: coderfender

native/spark-expr/src/static_invoke/char_varchar_utils/read_side_padding.rs

@@ -120,7 +120,7 @@ fn spark_read_side_padding_internal<T: OffsetSizeTrait>(
                         string.parse().unwrap(),
                         length.unwrap() as usize,
                         truncate,
-                    )),
+                    )?),
                     _ => builder.append_null(),
                 }
             }
@@ -140,7 +140,7 @@ fn spark_read_side_padding_internal<T: OffsetSizeTrait>(
                         string.parse().unwrap(),
                         length,
                         truncate,
-                    )),
+                    )?),
                     _ => builder.append_null(),
                 }
             }
@@ -149,7 +149,11 @@ fn spark_read_side_padding_internal<T: OffsetSizeTrait>(
     }
 }
 
-fn add_padding_string(string: String, length: usize, truncate: bool) -> String {
+fn add_padding_string(
+    string: String,
+    length: usize,
+    truncate: bool,
+) -> Result<String, DataFusionError> {
     // It looks Spark's UTF8String is closer to chars rather than graphemes
     // https://stackoverflow.com/a/46290728
     let space_string = " ".repeat(length);
@@ -161,11 +165,17 @@ fn add_padding_string(string: String, length: usize, truncate: bool) -> String {
                 .nth(length)
                 .map(|(i, _)| i)
                 .unwrap_or(string.len());
-            string[..idx].parse().unwrap()
+            match string[..idx].parse() {
+                Ok(string) => Ok(string),
+                Err(err) => Err(DataFusionError::Internal(format!(
+                    "Failed adding padding string {} error {:}",
+                    string, err
+                ))),
+            }
         } else {
-            string
+            Ok(string)
         }
     } else {
-        string + &space_string[char_len..]
+        Ok(string + &space_string[char_len..])
     }
 }

spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala

@@ -408,12 +408,9 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper {
     }
   }
   test("Verify rpad expr support for second arg instead of just literal") {
-    withTable("t1") {
-      val value = "IfIWasARoadIWouldBeBent"
-      sql("create table t1(c1 varchar(100), c2 int) using parquet")
-      sql(s"insert into t1 values('$value', 10)")
-      sql(s"insert into t1 values((${null}, 10))")
-      val res = sql("select rpad(c1,c2) , rpad(c1,5) from t1 order by c1")
+    val data = Seq(("IfIWasARoadIWouldBeBent", 10), ("తెలుగు", 2))
+    withParquetTable(data, "t1") {
+      val res = sql("select rpad(_1,_2) , rpad(_1,2) from t1 order by _1")
       checkSparkAnswerAndOperator(res)
     }
   }