[GitHub] [spark] srowen commented on a change in pull request #30981: [SPARK-33947] String functions: Trim/Ltrim/Rtrim support byte array

[GitBox <gi...@apache.org>]

srowen commented on a change in pull request #30981:
URL: https://github.com/apache/spark/pull/30981#discussion_r552093857



##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/ByteArray.java
##########
@@ -101,4 +101,117 @@ public static long getPrefix(byte[] bytes) {
     }
     return result;
   }
+
+  /**
+   * Trims bytes representing spaces from both ends of this byte array.
+   *
+   * @param srcArr the source byte array
+   * @return this byte array with no bytes representing spaces at the start or end
+   */
+  public static byte[] trim(byte[] srcArr) {
+    return trim(srcArr, " ".getBytes());

Review comment:
       This could theoretically be locale-dependent. Why not just write in the byte 32 to be clear?

##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/ByteArray.java
##########
@@ -101,4 +101,117 @@ public static long getPrefix(byte[] bytes) {
     }
     return result;
   }
+
+  /**
+   * Trims bytes representing spaces from both ends of this byte array.
+   *
+   * @param srcArr the source byte array
+   * @return this byte array with no bytes representing spaces at the start or end
+   */
+  public static byte[] trim(byte[] srcArr) {
+    return trim(srcArr, " ".getBytes());
+  }
+
+  /**
+   * Trims instances of the given trim byte array from both ends of this byte array.
+   *
+   * @param srcArr the source byte array
+   * @param trimArr the trim byte array
+   * @return this byte array with no occurrences of the trim byte array at the start or end,
+   * or `null` if `trimArr` is `null`
+   */
+  public static byte[] trim(byte[] srcArr, byte[] trimArr) {
+    return trimLeft(trimRight(srcArr, trimArr), trimArr);
+  }
+
+  /**
+   * Trims bytes representing spaces from the start of this byte array.
+   *
+   * @param srcArr the source byte array
+   * @return this byte array with no bytes representing spaces at the start
+   */
+  public static byte[] trimLeft(byte[] srcArr) {
+    return trimLeft(srcArr, " ".getBytes());
+  }
+
+  /**
+   * Trims instances of the given trim byte array from the start of this byte array.
+   *
+   * @param srcArr the source byte array
+   * @param trimArr the trim byte array
+   * @return this byte array with no occurrences of the trim byte array at the start,
+   * or `null` if `trimArr` is `null`
+   */
+  public static byte[] trimLeft(byte[] srcArr, byte[] trimArr) {
+    if (trimArr == null) {
+      return null;
+    } else if (trimArr.length == 0 || srcArr.length == 0) {

Review comment:
       Nit: the two elses are redundant




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org