Fix comment

facebookincubator · Dec 27, 2024 · 6b1dcd2 · 6b1dcd2
1 parent 2aa1262
commit 6b1dcd2
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 21 deletions.
diff --git a/velox/functions/sparksql/specialforms/SparkCastHooks.cpp b/velox/functions/sparksql/specialforms/SparkCastHooks.cpp
@@ -83,11 +83,6 @@ StringView SparkCastHooks::removeWhiteSpaces(const StringView& view) const {
   return output;
 }
 
-const TimestampToStringOptions& SparkCastHooks::timestampToStringOptions()
-    const {
-  return timestampToStringOptions_;
-}
-
 exec::PolicyType SparkCastHooks::getPolicy() const {
   return exec::PolicyType::SparkCastPolicy;
 }

diff --git a/velox/functions/sparksql/specialforms/SparkCastHooks.h b/velox/functions/sparksql/specialforms/SparkCastHooks.h
@@ -52,10 +52,9 @@ class SparkCastHooks : public exec::CastHooks {
   /// whitespaces before cast.
   StringView removeWhiteSpaces(const StringView& view) const override;
 
-  /// 1) Does not follow 'isLegacyCast' and session timezone. 2) The conversion
-  /// precision is microsecond. 3) Does not append trailing zeros. 4) Adds a
-  /// positive sign at first if the year exceeds 9999.
-  const TimestampToStringOptions& timestampToStringOptions() const override;
+  const TimestampToStringOptions& timestampToStringOptions() const override {
+    return timestampToStringOptions_;
+  }
 
   bool truncate() const override {
     return true;
@@ -65,6 +64,10 @@ class SparkCastHooks : public exec::CastHooks {
 
  private:
   const core::QueryConfig& config_;
+  /// 1) Does not follow 'isLegacyCast' and session timezone. 2) The conversion
+  /// precision is microsecond. 3) Does not append trailing zeros. 4) Adds a
+  /// positive sign at first if the year exceeds 9999. 5) The configured session
+  /// timezone is respected.
   TimestampToStringOptions timestampToStringOptions_ = {
       .precision = TimestampToStringOptions::Precision::kMicroseconds,
       .leadingPositiveSign = true,

diff --git a/velox/functions/sparksql/tests/SparkCastExprTest.cpp b/velox/functions/sparksql/tests/SparkCastExprTest.cpp
@@ -581,31 +581,60 @@ TEST_F(SparkCastExprTest, timestampToString) {
           std::nullopt,
       });
 
+  std::vector<std::optional<Timestamp>> input = {
+      Timestamp(-946684800, 0),
+      Timestamp(-7266, 0),
+      Timestamp(0, 0),
+      Timestamp(61, 10),
+      Timestamp(3600, 0),
+      Timestamp(946684800, 0),
+
+      Timestamp(946729316, 0),
+      Timestamp(946729316, 123),
+      Timestamp(946729316, 100000000),
+      Timestamp(946729316, 129900000),
+      Timestamp(946729316, 123456789),
+      Timestamp(7266, 0),
+      std::nullopt,
+  };
+
   setTimezone("America/Los_Angeles");
   testCast<Timestamp, std::string>(
       "string",
+      input,
       {
-          Timestamp(0, 0),
-          Timestamp(3600, 0),
-          Timestamp(61, 10),
-      },
-      {
+          "1940-01-01 16:00:00",
+          "1969-12-31 13:58:54",
           "1969-12-31 16:00:00",
-          "1969-12-31 17:00:00",
           "1969-12-31 16:01:01",
+          "1969-12-31 17:00:00",
+          "1999-12-31 16:00:00",
+          "2000-01-01 04:21:56",
+          "2000-01-01 04:21:56",
+          "2000-01-01 04:21:56.1",
+          "2000-01-01 04:21:56.1299",
+          "2000-01-01 04:21:56.123456",
+          "1969-12-31 18:01:06",
+          std::nullopt,
       });
   setTimezone("Asia/Shanghai");
   testCast<Timestamp, std::string>(
       "string",
+      input,
       {
-          Timestamp(0, 0),
-          Timestamp(3600, 0),
-          Timestamp(61, 10),
-      },
-      {
+          "1940-01-02 08:00:00",
+          "1970-01-01 05:58:54",
           "1970-01-01 08:00:00",
-          "1970-01-01 09:00:00",
           "1970-01-01 08:01:01",
+          "1970-01-01 09:00:00",
+          "2000-01-01 08:00:00",
+          "2000-01-01 20:21:56",
+          "2000-01-01 20:21:56",
+          "2000-01-01 20:21:56.1",
+          "2000-01-01 20:21:56.1299",
+          "2000-01-01 20:21:56.123456",
+          "1970-01-01 10:01:06",
+          std::nullopt,
       });
 }