> On Oct. 14, 2017, 12:20 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SortedDynPartitionTimeGranularityOptimizer.java
> > Lines 285-288 (patched)
> > <https://reviews.apache.org/r/62981/diff/1/?file=1854682#file1854682line287>
> >
> >     Will be good to add a comment for a need to do a cast to timestamp 
> > first.
> >     Also, should it not be to ts w/ tz ?

I have added comments in the code to explain how this works. I copy-paste them 
here, if you have additional comments let me know.

      // Timestamp column type in Druid is timestamp with local time-zone, as 
it represents
      // a specific instant in time. Thus, we have this value and we need to 
extract the
      // granularity to split the data when we are storing it in Druid. 
However, Druid stores
      // the data in UTC. Thus, we need to apply the following logic on the 
data to extract
      // the granularity correctly:
      // 1) Read the timestamp with local time-zone value.
      // 2) Extract UTC epoch (millis) from timestamp with local time-zone.
      // 3) Cast the long to a timestamp.
      // 4) Apply the granularity function on the timestamp value.
      // That way, '2010-01-01 00:00:00 UTC' and '2009-12-31 16:00:00 PST' 
(same instant)
      // will end up in the same Druid segment.


> On Oct. 14, 2017, 12:20 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/functions/HiveSqlCountAggFunction.java
> > Lines 89 (patched)
> > <https://reviews.apache.org/r/62981/diff/1/?file=1854688#file1854688line90>
> >
> >     Good to add a reason for need to override.

Done, thanks! I will create a follow-up for this, the correct way to fix this 
would be to use proper nullability for these functions.


> On Oct. 14, 2017, 12:20 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTBuilder.java
> > Lines 285 (patched)
> > <https://reviews.apache.org/r/62981/diff/1/?file=1854699#file1854699line285>
> >
> >     good to add a comment on why we postfix UTC

Done, thanks!

      // Calcite stores timestamp with local time-zone in UTC internally, thus
      // when we bring it back, we need to add the UTC suffix.


> On Oct. 14, 2017, 12:20 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ExprNodeConverter.java
> > Lines 309 (patched)
> > <https://reviews.apache.org/r/62981/diff/1/?file=1854701#file1854701line316>
> >
> >     comment for UTC.

Same as above.


> On Oct. 14, 2017, 12:20 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g
> > Lines 371 (patched)
> > <https://reviews.apache.org/r/62981/diff/1/?file=1854709#file1854709line371>
> >
> >     We already support ts w/ tz in syntax. What is this change for?

This is for the literal values. This is useful when we can back from Calcite 
(through AST), Calcite might have folded a cast to timestamp with local 
time-zone over a text value, and now we need to generate a timestamp with local 
time-zone itself. We did not have support for it yet.


- Jesús


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


On Oct. 13, 2017, 4:52 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62981/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 4:52 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-17672
>     https://issues.apache.org/jira/browse/HIVE-17672
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-17672: Upgrade Calcite version to 1.14
> 
> 
> Diffs
> -----
> 
>   
> druid-handler/src/java/org/apache/hadoop/hive/druid/io/DruidOutputFormat.java 
> da7642c61a3c1b82e0d505d110bef63863e7be13 
>   druid-handler/src/java/org/apache/hadoop/hive/druid/serde/DruidSerDe.java 
> a43f62ed8203b74fde64e676e75385b4cd52570b 
>   
> druid-handler/src/java/org/apache/hadoop/hive/druid/serde/DruidSerDeUtils.java
>  86c325ba1fc728e452dea8440ab1ca0face45cc3 
>   
> druid-handler/src/test/org/apache/hadoop/hive/druid/serde/TestDruidSerDe.java 
> 655a96ad7b5a24de91707e35b93fb3eb99a2563a 
>   pom.xml ca088da3bfda76fd3da19d96fb6f7705a8adfe57 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
> 1d1d2a3e0bee177cd4b145c731b5ea3c75483c84 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> 68c94820c9368f504401c801f9b599a14f91ddf6 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
>  154ea6844175387e11802e6167343dac7c306112 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/SortedDynPartitionTimeGranularityOptimizer.java
>  4a248d6fe74234c532d32f11c7926971bb44e8a2 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java 
> f70aa7502793f691ab554e2b378f915b57ec52a1 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveConfPlannerContext.java
>  756b671a83f1be30b993c3b76913e5d4317ed2d5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelFactories.java 
> 823b099efd477a01b4facf670efc093732a8ad88 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveType.java 
> 31a628bba2ca5c83cceee7d77208acafaed45714 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveTypeSystemImpl.java
>  dad783d91dfa90fceefc2bd66e538bed0278971e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/functions/HiveSqlCountAggFunction.java
>  75b7ad2c8d4dd9da7357e6c811326bb29fcc02d7 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/functions/HiveSqlSumEmptyIsZeroAggFunction.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveAggregate.java
>  fa51b4e0b24b6fb1fde5e1a742dac5efa91c2246 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveGroupingID.java
>  2fb950867bccbdac571a0630b8ab4d8fc395e326 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveAggregateJoinTransposeRule.java
>  21b46cfca7661171e78b7e55cc8d14cf3d3a4573 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveAggregateProjectMergeRule.java
>  7c3b4b06ce5ecd8d53d73e43dc45e5726ba37a10 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java
>  b63ea022c9dd4c0d46ba5fc98761d4340c0d6291 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExpandDistinctAggregatesRule.java
>  81bb730fba9baeb3a86454b30210afe9874632f7 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
>  5b0a7d7293653ca87a0f4e4dd697ae20242b53aa 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelColumnsAlignment.java
>  f35bf2f2a9fd4b7123007085af45853f9412586b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java
>  b922faac5d522e44c1dc6f314877f22769d058be 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTBuilder.java
>  eab8f6f110610374f1ac5ab103302b1ee5ab15c5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java
>  d6b978fae3588f83b250ec3956d4f48e64733261 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ExprNodeConverter.java
>  3dcceaba694359e3206594cb409ea441323505cf 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveGBOpConvUtil.java
>  0f6c5b5355490f3a9b67a29bd3db9ef77a129f13 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java
>  471675b53bbb151eca5c1007a7f3573b5e3fe9c1 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/RexNodeConverter.java
>  f8a959d13cf74946ed5e0df21f837cfdd095f993 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java
>  31a088ba5fed51f195517a413d146d944fb777bd 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/TypeConverter.java
>  34886f367aba7a9665b44e45334bf51fc05f91d4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
> 65552693f776da5625526a7acb5a5d32b6a418e9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> 020a3008db5febd8070505ccc9c8b63dbd43ad6a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 0591fbf586d130857262c31c978057f169016d25 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 
> 391ee08f77eabaf8a190b94f1f6a62dba1965e0a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateFloor.java 
> 126ed44ebcb5c9db33e7adce48b1cbe05e53fd2b 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFEpochMilli.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateFormatGranularity.java 
> 03b2d42147b77f1ae844ef7a945f283989ce8ef3 
>   ql/src/test/queries/clientpositive/druid_timeseries.q 
> c0ad60fe1c2cd9ff00d8afefa2acdc7699e0d1d7 
>   ql/src/test/queries/clientpositive/druid_topn.q 
> b121b7e1207e40cd30374d4328fadf4944b07b3c 
>   ql/src/test/queries/clientpositive/timestamptz_3.q PRE-CREATION 
>   ql/src/test/results/clientpositive/cbo_rp_annotate_stats_groupby.q.out 
> a603cc60721951cb76d2d9227a4d4f80b12c94fb 
>   ql/src/test/results/clientpositive/druid_basic1.q.out 
> 91f79bb4246cb24f0d1a3d2ff9756937d0d1da1a 
>   ql/src/test/results/clientpositive/druid_basic2.q.out 
> 5d6862cc39e5b98f6288a915cf938c8cabdef791 
>   ql/src/test/results/clientpositive/druid_intervals.q.out 
> 63f28d5fd85f17d2be66059167f87a81cedb0aca 
>   ql/src/test/results/clientpositive/druid_timeseries.q.out 
> 04db1e075b8cf2ca101abab70b137dd9d360602b 
>   ql/src/test/results/clientpositive/druid_topn.q.out 
> 5c31f85df71a8c4ded179f88ca95917343ababb6 
>   ql/src/test/results/clientpositive/groupby_grouping_sets1.q.out 
> 13eac20418f4db9b571dc3dd97c98a5d8f251f0a 
>   ql/src/test/results/clientpositive/groupby_grouping_sets_limit.q.out 
> 02636eb26beeafd6ee3eefced23f677d1d657807 
>   ql/src/test/results/clientpositive/llap/vector_groupby_grouping_id2.q.out 
> f5144482fc2c0c5cb70486c8dad6ef14935a69c3 
>   ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets1.q.out 
> c7591705eb1c1d8e91ef31160c6f1957451fce84 
>   
> ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets_limit.q.out
>  15190b893f23b59abb99d6d5343b13d31ace60f7 
>   ql/src/test/results/clientpositive/timestamptz_3.q.out PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62981/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>

Reply via email to