[SPARK-20557] [SQL] Only support TIMESTAMP WITH TIME ZONE for Oracle Dialect #19939
Conversation
|
Test build #84692 has finished for PR 19939 at commit
|
|
Jenkins, retest this please |
|
Test build #84693 has finished for PR 19939 at commit
|
|
retest this please |
|
Test build #84697 has finished for PR 19939 at commit
|
| } | ||
|
|
||
| val dfRead = sqlContext.read.jdbc(jdbcUrl, "ts_with_timezone", new Properties) | ||
| withTimeZone("PST") { |
There was a problem hiding this comment.
how do you make sure sesson time zone is same with JVM time zone?
There was a problem hiding this comment.
oh i see, session time zone is dynamically evaluated with JVM timezone if not set, so we need to guarantee that session time zone is not set here.
There was a problem hiding this comment.
I feel it's safer if we also set session local time zone in withTimeZone
|
LGTM |
|
retest this please |
|
Test build #84722 has finished for PR 19939 at commit
|
| @@ -151,6 +151,8 @@ class PostgresIntegrationSuite extends DockerJDBCIntegrationSuite { | |||
|
|
|||
| test("SPARK-20557: column type TIMESTAMP with TIME ZONE and TIME with TIME ZONE " + | |||
| "should be recognized") { | |||
| // When using JDBC to read the columns of TIMESTAMP with TIME ZONE and TIME with TIME ZONE | |||
There was a problem hiding this comment.
Out of curiosity, why do we not need something similar for postgres: https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala#L61?
There was a problem hiding this comment.
That is for the support of ArrayType. We can revisit that one in the next PR.
|
Test build #84731 has finished for PR 19939 at commit
|
|
Since the test failure in SparkR is not related to this PR, I merge this to master. Thanks for the review! |
What changes were proposed in this pull request?
In the previous PRs, #17832 and #17835 , we convert
TIMESTAMP WITH TIME ZONEandTIME WITH TIME ZONEtoTIMESTAMPfor all the JDBC sources. However, this conversion could be risky since it does not respect our SQL configurationspark.sql.session.timeZone.In addition, each vendor might have different semantics for these two types. For example, Postgres simply returns
TIMESTAMPtypes forTIMESTAMP WITH TIME ZONE. For such supports, we should do it case by case. This PR reverts the general support ofTIMESTAMP WITH TIME ZONEandTIME WITH TIME ZONEfor JDBC sources, except ORACLE Dialect.When supporting the ORACLE's
TIMESTAMP WITH TIME ZONE, we only support it when the JVM default timezone is the same as the user-specified configurationspark.sql.session.timeZone(whose default is the JVM default timezone). Now, we still treatTIMESTAMP WITH TIME ZONEasTIMESTAMPwhen fetching the values via the Oracle JDBC connector, whose client converts the timestamp values with time zone to the timestamp values using the local JVM default timezone (a test case is added toOracleIntegrationSuite.scalain this PR for showing the behavior). Thus, to avoid any future behavior change, we will not support it if JVM default timezone is different fromspark.sql.session.timeZoneNo regression because the previous two PRs were just merged to be unreleased master branch.
How was this patch tested?
Added the test cases