----------------------------------------------------------- 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 > >
