This is an automated email from the ASF dual-hosted git repository.

gengliang pushed a commit to branch branch-3.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.4 by this push:
     new f4e53fa9910 [SPARK-43336][SQL] Casting between Timestamp and 
TimestampNTZ requires timezone
f4e53fa9910 is described below

commit f4e53fa9910780934b49b2fcf6a22dba7b337a11
Author: Gengliang Wang <gengli...@apache.org>
AuthorDate: Tue May 2 13:09:45 2023 -0700

    [SPARK-43336][SQL] Casting between Timestamp and TimestampNTZ requires 
timezone
    
    ### What changes were proposed in this pull request?
    
    Casting between Timestamp and TimestampNTZ requires a timezone since the 
timezone id is used in the evaluation.
    This PR updates the method `Cast.needsTimeZone` to include the conversion 
between Timestamp and TimestampNTZ. As a result:
    * Casting between Timestamp and TimestampNTZ is considered as unresolved 
unless the timezone is defined
    * Canonicalizing cast will include the time zone
    
    ### Why are the changes needed?
    
    Timezone id is used in the evaluation between Timestamp and TimestampNTZ, 
thus we should mark such conversion as "needsTimeZone"
    
    ### Does this PR introduce _any_ user-facing change?
    
    No. It is more like a fix for potential bugs that the casting between 
Timestamp and TimestampNTZ is marked as resolved before resolving the timezone.
    
    ### How was this patch tested?
    
    New UT
    
    Closes #41010 from gengliangwang/fixCast.
    
    Authored-by: Gengliang Wang <gengli...@apache.org>
    Signed-off-by: Gengliang Wang <gengli...@apache.org>
    (cherry picked from commit a1c7035294ecdc10f507c58a41cb0ed7137717ca)
    Signed-off-by: Gengliang Wang <gengli...@apache.org>
---
 .../org/apache/spark/sql/catalyst/expressions/Cast.scala   |  2 ++
 .../spark/sql/catalyst/expressions/CanonicalizeSuite.scala | 14 +++++++++++++-
 .../spark/sql/catalyst/expressions/CastSuiteBase.scala     |  7 +++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
index ae0dc3dbf03..3a93cc58a5f 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
@@ -271,6 +271,8 @@ object Cast extends QueryErrorsBase {
     case (TimestampType | DateType, StringType) => true
     case (DateType, TimestampType) => true
     case (TimestampType, DateType) => true
+    case (TimestampType, TimestampNTZType) => true
+    case (TimestampNTZType, TimestampType) => true
     case (ArrayType(fromType, _), ArrayType(toType, _)) => 
needsTimeZone(fromType, toType)
     case (MapType(fromKey, fromValue, _), MapType(toKey, toValue, _)) =>
       needsTimeZone(fromKey, toKey) || needsTimeZone(fromValue, toValue)
diff --git 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CanonicalizeSuite.scala
 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CanonicalizeSuite.scala
index f2a9eac8216..0e22b0d2876 100644
--- 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CanonicalizeSuite.scala
+++ 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CanonicalizeSuite.scala
@@ -25,7 +25,7 @@ import org.apache.spark.sql.catalyst.dsl.plans._
 import org.apache.spark.sql.catalyst.plans.logical.Range
 import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.sql.internal.SQLConf.MULTI_COMMUTATIVE_OP_OPT_THRESHOLD
-import org.apache.spark.sql.types.{BooleanType, Decimal, DecimalType, 
IntegerType, LongType, StringType, StructField, StructType}
+import org.apache.spark.sql.types.{BooleanType, Decimal, DecimalType, 
IntegerType, LongType, StringType, StructField, StructType, TimestampNTZType, 
TimestampType}
 
 class CanonicalizeSuite extends SparkFunSuite {
 
@@ -99,6 +99,18 @@ class CanonicalizeSuite extends SparkFunSuite {
     assert(castWithTimeZoneId.semanticEquals(cast))
   }
 
+  test("SPARK-43336: Canonicalize Cast between Timestamp and TimestampNTZ 
should consider " +
+    "timezone") {
+    val timestampLiteral = Literal.create(1L, TimestampType)
+    val timestampNTZLiteral = Literal.create(1L, TimestampNTZType)
+    Seq(
+      Cast(timestampLiteral, TimestampNTZType),
+      Cast(timestampNTZLiteral, TimestampType)
+    ).foreach { cast =>
+      
assert(!cast.semanticEquals(cast.withTimeZone(SQLConf.get.sessionLocalTimeZone)))
+    }
+  }
+
   test("SPARK-32927: Bitwise operations are commutative") {
     Seq(BitwiseOr(_, _), BitwiseAnd(_, _), BitwiseXor(_, _)).foreach { f =>
       val e1 = f($"a", f($"b", $"c"))
diff --git 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuiteBase.scala
 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuiteBase.scala
index bad85ca4176..7c11ccf6a0d 100644
--- 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuiteBase.scala
+++ 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuiteBase.scala
@@ -1391,4 +1391,11 @@ abstract class CastSuiteBase extends SparkFunSuite with 
ExpressionEvalHelper {
     assert(expr.sql == cast.sql)
     assert(expr.toString == cast.toString)
   }
+
+  test("SPARK-43336: Casting between Timestamp and TimestampNTZ requires 
timezone") {
+    val timestampLiteral = Literal.create(1L, TimestampType)
+    val timestampNTZLiteral = Literal.create(1L, TimestampNTZType)
+    assert(!Cast(timestampLiteral, TimestampNTZType).resolved)
+    assert(!Cast(timestampNTZLiteral, TimestampType).resolved)
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to