----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51755/#review153186 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java (line 956) <https://reviews.apache.org/r/51755/#comment222467> Does this need to be public? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java (line 967) <https://reviews.apache.org/r/51755/#comment222469> public? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java (line 1000) <https://reviews.apache.org/r/51755/#comment222471> need to be public? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java (line 73) <https://reviews.apache.org/r/51755/#comment222474> Add comment about why we only have 2 branches? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java (line 82) <https://reviews.apache.org/r/51755/#comment222472> Nice comments! I think for completeness, also add comments on how NULLs are handled. e.g., If they make any difference for all vs distinct. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java (line 144) <https://reviews.apache.org/r/51755/#comment222475> Why can't Gby takes its input from union? If it indeed is necessary, add a comment why. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java (line 256) <https://reviews.apache.org/r/51755/#comment222476> Add a comment on why we need this pass through project? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java (line 285) <https://reviews.apache.org/r/51755/#comment222477> determinsitic = true here, isnt it? ql/src/test/queries/clientpositive/except_all.q (line 56) <https://reviews.apache.org/r/51755/#comment222493> Add tests which has aggregates in branches of except so that we can test ReduceSinkDedup optimizations with these features. ql/src/test/queries/clientpositive/except_distinct.q (line 55) <https://reviews.apache.org/r/51755/#comment222494> Add tests which has aggregates in branches of except so that we can test ReduceSinkDedup optimizations with these features. ql/src/test/queries/clientpositive/intersect_all.q (line 40) <https://reviews.apache.org/r/51755/#comment222496> Add tests which has aggregates in branches of intersect so that we can test ReduceSinkDedup optimizations with these features. ql/src/test/queries/clientpositive/intersect_distinct.q (line 39) <https://reviews.apache.org/r/51755/#comment222497> Add tests which has aggregates in branches of intersect so that we can test ReduceSinkDedup optimizations with these features. - Ashutosh Chauhan On Oct. 15, 2016, 4:10 a.m., pengcheng xiong wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51755/ > ----------------------------------------------------------- > > (Updated Oct. 15, 2016, 4:10 a.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Repository: hive-git > > > Description > ------- > > HIVE-12765 > > > Diffs > ----- > > itests/src/test/resources/testconfiguration.properties 97e310d > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SimpleFetchOptimizer.java > eb0ba7b > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java > c527e58 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelFactories.java > cf93ed8 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveExcept.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveIntersect.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java > cc318db > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java > 8d738aa > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java e6ab947 > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 5d3fa6a > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 5c16c55 > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 50987c3 > ql/src/java/org/apache/hadoop/hive/ql/parse/QBExpr.java cccf0f6 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 747f387 > > ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java > a427803 > ql/src/test/queries/clientpositive/except_all.q PRE-CREATION > ql/src/test/queries/clientpositive/except_distinct.q PRE-CREATION > ql/src/test/queries/clientpositive/intersect_all.q PRE-CREATION > ql/src/test/queries/clientpositive/intersect_distinct.q PRE-CREATION > ql/src/test/queries/clientpositive/intersect_merge.q PRE-CREATION > ql/src/test/results/clientpositive/auto_join8.q.out ccbafba > ql/src/test/results/clientpositive/except_all.q.out PRE-CREATION > ql/src/test/results/clientpositive/except_distinct.q.out PRE-CREATION > ql/src/test/results/clientpositive/input25.q.out 465173d > ql/src/test/results/clientpositive/intersect_all.q.out PRE-CREATION > ql/src/test/results/clientpositive/intersect_distinct.q.out PRE-CREATION > ql/src/test/results/clientpositive/join8.q.out 47b821a > ql/src/test/results/clientpositive/list_bucket_query_oneskew_2.q.out > 45f2481 > ql/src/test/results/clientpositive/smb_mapjoin_25.q.out aba899e > ql/src/test/results/clientpositive/spark/auto_join8.q.out 6909113 > ql/src/test/results/clientpositive/spark/join8.q.out 4903c90 > ql/src/test/results/clientpositive/spark/smb_mapjoin_25.q.out eacb438 > ql/src/test/results/clientpositive/spark/union_top_level.q.out e1c7fc7 > ql/src/test/results/clientpositive/tez/intersect_merge.q.out PRE-CREATION > ql/src/test/results/clientpositive/timestamp.q.out 7c08ec8 > ql/src/test/results/clientpositive/vector_null_projection.q.out 779f787 > > Diff: https://reviews.apache.org/r/51755/diff/ > > > Testing > ------- > > > Thanks, > > pengcheng xiong > >
