sankarh commented on a change in pull request #2447:
URL: https://github.com/apache/hive/pull/2447#discussion_r665477114
##########
File path:
common/src/java/org/apache/hadoop/hive/common/type/TimestampTZUtil.java
##########
@@ -186,4 +186,8 @@ public static Timestamp convertTimestampToZone(Timestamp
ts, ZoneId fromZone, Zo
return
Timestamp.ofEpochSecond(localDateTimeAtToZone.toEpochSecond(ZoneOffset.UTC),
localDateTimeAtToZone.getNano());
}
+
+ public static double convertTimestampTZToDouble(TimestampTZ timestampTZ) {
+ return timestampTZ.getEpochSecond() + timestampTZ.getNanos() / 1000000000;
Review comment:
Can use DateUtils.NANOS_PER_SEC instead of hardcoding.
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToByte.java
##########
@@ -211,7 +217,10 @@ public ByteWritable evaluate(TimestampWritableV2 i) {
if (i == null) {
return null;
} else {
- final long longValue = i.getSeconds();
+ ZoneId zone = SessionState.get() == null ?
+ new HiveConf().getLocalTimeZone() :
SessionState.get().getConf().getLocalTimeZone();
+ TimestampTZ timestamp = TimestampTZUtil.convert(i.getTimestamp(), zone);
Review comment:
Duplicate code. Can write an utility method in TimestampTZUtil that
takes timestamp as input, convert and return TimestampTZ.
##########
File path: ql/src/test/results/clientpositive/llap/timestamp_1.q.out
##########
@@ -257,7 +329,7 @@ POSTHOOK: query: select cast(t as double) from timestamp_1
limit 1
POSTHOOK: type: QUERY
POSTHOOK: Input: default@timestamp_1
#### A masked pattern was here ####
-1.2938436611E9
+1.293843661E9
Review comment:
Does this precision values match with Hive 1.2?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToBoolean.java
##########
@@ -213,7 +219,10 @@ public BooleanWritable evaluate(TimestampWritableV2 i) {
if (i == null) {
return null;
} else {
- booleanWritable.set(i.getSeconds() != 0 || i.getNanos() != 0);
+ ZoneId zone = SessionState.get() == null ?
+ new HiveConf().getLocalTimeZone() :
SessionState.get().getConf().getLocalTimeZone();
+ TimestampTZ timestamp = TimestampTZUtil.convert(i.getTimestamp(), zone);
+ booleanWritable.set(timestamp.getEpochSecond() != 0 ||
timestamp.getNanos() != 0);
Review comment:
Nit: Use () around each conditions.
##########
File path: ql/src/test/queries/clientpositive/timestamp_2.q
##########
@@ -17,6 +17,19 @@ select cast(t as float) from timestamp_2 limit 1;
select cast(t as double) from timestamp_2 limit 1;
select cast(t as string) from timestamp_2 limit 1;
+set hive.local.time.zone=Asia/Bangkok;
+
+select cast(t as boolean) from timestamp_2 limit 1;
Review comment:
I don't see any diff between timestamp_1 and timestamp_2 testcases. Is
it duplicate?
##########
File path: ql/src/test/results/clientpositive/llap/timestamp_1.q.out
##########
@@ -101,6 +101,78 @@ POSTHOOK: type: QUERY
POSTHOOK: Input: default@timestamp_1
#### A masked pattern was here ####
2011-01-01 01:01:01
+PREHOOK: query: select cast(t as boolean) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as boolean) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+true
+PREHOOK: query: select cast(t as tinyint) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as tinyint) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+NULL
+PREHOOK: query: select cast(t as smallint) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as smallint) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+NULL
+PREHOOK: query: select cast(t as int) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as int) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+1293843661
+PREHOOK: query: select cast(t as bigint) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as bigint) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+1293818461
+PREHOOK: query: select cast(t as float) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as float) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+1.2938185E9
Review comment:
Output of cast(float) and cast(double) mismatch with UTC one.
##########
File path: ql/src/test/results/clientpositive/llap/timestamp_1.q.out
##########
@@ -101,6 +101,78 @@ POSTHOOK: type: QUERY
POSTHOOK: Input: default@timestamp_1
#### A masked pattern was here ####
2011-01-01 01:01:01
+PREHOOK: query: select cast(t as boolean) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as boolean) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+true
+PREHOOK: query: select cast(t as tinyint) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as tinyint) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+NULL
+PREHOOK: query: select cast(t as smallint) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as smallint) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+NULL
+PREHOOK: query: select cast(t as int) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as int) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+1293843661
+PREHOOK: query: select cast(t as bigint) from timestamp_1 limit 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+POSTHOOK: query: select cast(t as bigint) from timestamp_1 limit 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@timestamp_1
+#### A masked pattern was here ####
+1293818461
Review comment:
Why the output mismatch with cast(int)?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToBoolean.java
##########
@@ -213,7 +219,10 @@ public BooleanWritable evaluate(TimestampWritableV2 i) {
if (i == null) {
return null;
} else {
- booleanWritable.set(i.getSeconds() != 0 || i.getNanos() != 0);
+ ZoneId zone = SessionState.get() == null ?
Review comment:
nit: Use () around condition.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
