hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ashutosh Chauhan <hashut...@apache.org>
Subject Re: Review Request 50787: Add a timezone-aware timestamp
Date Tue, 09 May 2017 23:05:44 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50787/#review174383
-----------------------------------------------------------




common/src/java/org/apache/hadoop/hive/common/type/TimestampTZ.java
Lines 48 (patched)
<https://reviews.apache.org/r/50787/#comment247508>

    Worth adding a comment on how value is represented when zone info is absent. From code
it seems, like it will be parsed as system's default zone and then normalized and stored as
UTC.



common/src/java/org/apache/hadoop/hive/common/type/TimestampTZ.java
Lines 158 (patched)
<https://reviews.apache.org/r/50787/#comment247509>

    May want to add a TODO that currently we assume zone as system default, in future this
may come from user specificed zone (via set time zone) command.



common/src/java/org/apache/hadoop/hive/common/type/TimestampTZ.java
Lines 190 (patched)
<https://reviews.apache.org/r/50787/#comment247510>

    TODO here to get zone from user provided set value in future.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/TypeConverter.java
Lines 204 (patched)
<https://reviews.apache.org/r/50787/#comment247512>

    Can you file a bug in Calcite that it should have sql type to represent TS w TZ?



ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g
Lines 133 (patched)
<https://reviews.apache.org/r/50787/#comment247513>

    Lets not use Timestamptz. Non-standard, with no advantage.



ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g
Lines 134 (patched)
<https://reviews.apache.org/r/50787/#comment247515>

    Add in release note as incompatiable change that time is now a reserved word. Should be
ok, since standard also has it as reserved.



ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToString.java
Lines 160 (patched)
<https://reviews.apache.org/r/50787/#comment247517>

    Add a comment that string reperesentation will return TS in UTC zone and not in original
TZ.



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToTimestampTZ.java
Lines 35 (patched)
<https://reviews.apache.org/r/50787/#comment247519>

    Comment about what happens if string has no zone info.



ql/src/test/queries/clientpositive/timestamptz_1.q
Lines 16 (patched)
<https://reviews.apache.org/r/50787/#comment247521>

    Can you add a test which adds string literal in TSTZ column which has no zone info?
    
    insert into tstz1 values (timestamp '2016-01-03 12:26:34.1');
    insert into tstz1 values ('2016-01-03 12:26:34.1');
    insert into tstz1 values (cast ('2016-01-03 12:26:34.1' as timestamp with time zone));
    
    create table t (ts timestamp);
    insert into t values (....);
    insert into tstz1 select ts from t;



ql/src/test/queries/clientpositive/timestamptz_1.q
Lines 21 (patched)
<https://reviews.apache.org/r/50787/#comment247520>

    Can you add a test which inserts TS column in TSTZ column?



serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java
Lines 1078 (patched)
<https://reviews.apache.org/r/50787/#comment247524>

    Instead of throwing exception, return null ?



serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java
Lines 1154 (patched)
<https://reviews.apache.org/r/50787/#comment247525>

    Instead of throwing exception, return null ?


- Ashutosh Chauhan


On May 8, 2017, 3:17 p.m., Rui Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50787/
> -----------------------------------------------------------
> 
> (Updated May 8, 2017, 3:17 p.m.)
> 
> 
> Review request for hive, pengcheng xiong and Xuefu Zhang.
> 
> 
> Bugs: HIVE-14412
>     https://issues.apache.org/jira/browse/HIVE-14412
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> The 1st patch to add timezone-aware timestamp.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/type/TimestampTZ.java PRE-CREATION 
>   common/src/test/org/apache/hadoop/hive/common/type/TestTimestampTZ.java PRE-CREATION

>   contrib/src/test/queries/clientnegative/serde_regex.q a676338 
>   contrib/src/test/queries/clientpositive/serde_regex.q d75d607 
>   contrib/src/test/results/clientnegative/serde_regex.q.out 58b1c02 
>   contrib/src/test/results/clientpositive/serde_regex.q.out 2984293 
>   hbase-handler/src/test/queries/positive/hbase_timestamp.q 0350afe 
>   hbase-handler/src/test/results/positive/hbase_timestamp.q.out 3918121 
>   itests/hive-blobstore/src/test/queries/clientpositive/orc_format_part.q 358eccd 
>   itests/hive-blobstore/src/test/queries/clientpositive/orc_nonstd_partitions_loc.q c462538

>   itests/hive-blobstore/src/test/queries/clientpositive/rcfile_format_part.q c563d3a

>   itests/hive-blobstore/src/test/queries/clientpositive/rcfile_nonstd_partitions_loc.q
d17c281 
>   itests/hive-blobstore/src/test/results/clientpositive/orc_format_part.q.out 5d1319f

>   itests/hive-blobstore/src/test/results/clientpositive/orc_nonstd_partitions_loc.q.out
70e72f7 
>   itests/hive-blobstore/src/test/results/clientpositive/rcfile_format_part.q.out bed10ab

>   itests/hive-blobstore/src/test/results/clientpositive/rcfile_nonstd_partitions_loc.q.out
c6442f9 
>   jdbc/src/java/org/apache/hive/jdbc/HiveBaseResultSet.java ade1900 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcColumn.java 38918f0 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 1b556ac 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/GroupByOperator.java f8b55da 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/SerializationUtilities.java 01a652d 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/TypeConverter.java
38308c9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 0cf9205 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 190b66b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ca639d3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 645ced9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java c3227c9 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java bda2050 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToBoolean.java 7cdf2c3 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToString.java 5cacd59 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java 68d98f5 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java 5a31e61 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToTimestampTZ.java PRE-CREATION

>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java
0dc6b19 
>   ql/src/test/queries/clientnegative/serde_regex.q c9cfc7d 
>   ql/src/test/queries/clientnegative/serde_regex2.q a29bb9c 
>   ql/src/test/queries/clientnegative/serde_regex3.q 4e91f06 
>   ql/src/test/queries/clientpositive/create_like.q bd39731 
>   ql/src/test/queries/clientpositive/join43.q 12c45a6 
>   ql/src/test/queries/clientpositive/serde_regex.q e21c6e1 
>   ql/src/test/queries/clientpositive/timestamptz.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/timestamptz_1.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/timestamptz_2.q PRE-CREATION 
>   ql/src/test/results/clientnegative/serde_regex.q.out a1ec5ca 
>   ql/src/test/results/clientnegative/serde_regex2.q.out 374675d 
>   ql/src/test/results/clientnegative/serde_regex3.q.out dc0a0e2 
>   ql/src/test/results/clientpositive/create_like.q.out ff2e752 
>   ql/src/test/results/clientpositive/join43.q.out e8c7278 
>   ql/src/test/results/clientpositive/serde_regex.q.out 7bebb0c 
>   ql/src/test/results/clientpositive/timestamptz.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/timestamptz_1.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/timestamptz_2.q.out PRE-CREATION 
>   serde/if/serde.thrift 1d40d5a 
>   serde/src/gen/thrift/gen-cpp/serde_constants.h 8785bd2 
>   serde/src/gen/thrift/gen-cpp/serde_constants.cpp 907acf2 
>   serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java
2578d3e 
>   serde/src/gen/thrift/gen-php/org/apache/hadoop/hive/serde/Types.php ea2dbbe 
>   serde/src/gen/thrift/gen-py/org_apache_hadoop_hive_serde/constants.py e3b24eb 
>   serde/src/gen/thrift/gen-rb/serde_constants.rb 15efaea 
>   serde/src/java/org/apache/hadoop/hive/serde2/SerDeUtils.java 5ecfbca 
>   serde/src/java/org/apache/hadoop/hive/serde2/binarysortable/BinarySortableSerDe.java
89e15c3 
>   serde/src/java/org/apache/hadoop/hive/serde2/io/TimestampTZWritable.java PRE-CREATION

>   serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyFactory.java 23dbe6a 
>   serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyTimestampTZ.java PRE-CREATION

>   serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyUtils.java 73c72e1 
>   serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyPrimitiveObjectInspectorFactory.java
5601734 
>   serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyTimestampTZObjectInspector.java
PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryFactory.java 52f3527

>   serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinarySerDe.java 56b4ca3

>   serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryTimestampTZ.java
PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryUtils.java 8237b64

>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorConverters.java
24b3d4e 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java
ba44bae 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/PrimitiveObjectInspector.java
70633f3 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/JavaTimestampTZObjectInspector.java
PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorConverter.java
e08ad43 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorFactory.java
2ed0843 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java
9642a7e 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/SettableTimestampTZObjectInspector.java
PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/TimestampTZObjectInspector.java
PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/WritableConstantTimestampTZObjectInspector.java
PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/WritableTimestampTZObjectInspector.java
PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/thrift/Type.java 0ad8c02 
>   serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoFactory.java 43c4819

>   serde/src/test/org/apache/hadoop/hive/serde2/io/TestTimestampTZWritable.java PRE-CREATION

>   service-rpc/if/TCLIService.thrift 824b049 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_constants.cpp 991cb2e 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 8accf66 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp b6995c4 
>   service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCLIServiceConstants.java
930bed7 
>   service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TProtocolVersion.java
18a7825 
>   service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TTypeId.java
a3735eb 
>   service-rpc/src/gen/thrift/gen-php/Types.php ee5acd2 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/constants.py c8d4f8f 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py e9faa2a 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_constants.rb 25adbb4 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 714309c 
>   service/src/java/org/apache/hive/service/cli/ColumnValue.java 76e8c03 
>   service/src/java/org/apache/hive/service/cli/TypeDescriptor.java d634bef 
> 
> 
> Diff: https://reviews.apache.org/r/50787/diff/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rui Li
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message