[GitHub] [spark] MaxGekk commented on a change in pull request #28892: [MINOR][SQL] Simplify DateTimeUtils.cleanLegacyTimestampStr

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

MaxGekk commented on a change in pull request #28892:
URL: https://github.com/apache/spark/pull/28892#discussion_r443973612



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
##########
@@ -203,20 +203,10 @@ object DateTimeUtils {
     Math.multiplyExact(millis, MICROS_PER_MILLIS)
   }
 
+  private final val gmtUtf8 = UTF8String.fromString("GMT")
   // The method is called by JSON/CSV parser to clean up the legacy timestamp string by removing
-  // the "GMT" string.
-  def cleanLegacyTimestampStr(s: String): String = {
-    val indexOfGMT = s.indexOf("GMT")
-    if (indexOfGMT != -1) {
-      // ISO8601 with a weird time zone specifier (2000-01-01T00:00GMT+01:00)
-      val s0 = s.substring(0, indexOfGMT)
-      val s1 = s.substring(indexOfGMT + 3)
-      // Mapped to 2000-01-01T00:00+01:00
-      s0 + s1
-    } else {
-      s
-    }
-  }
+  // the "GMT" string. For example, it returns 2000-01-01T00:00+01:00 for 2000-01-01T00:00GMT+01:00.
+  def cleanLegacyTimestampStr(s: UTF8String): UTF8String = s.replace(gmtUtf8, UTF8String.EMPTY_UTF8)

Review comment:
       It has but look at how it is implemented via regexp. @JoshRosen implemented more effective replace in UTF8String https://github.com/apache/spark/pull/24707. That's why I took it. I hope it seems reasonable.




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