You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by pengcheng xiong <px...@hortonworks.com> on 2016/09/09 01:41:03 UTC

Review Request 51755: Support Intersect Distinct

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

Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-12765


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelFactories.java cf93ed8 
  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/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/translator/ASTConverter.java 9f5e733 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ff94160 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 7ceb005 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g df596ff 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9ba1865 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QBExpr.java cccf0f6 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 943d9d7 
  ql/src/test/queries/clientpositive/intersect.q PRE-CREATION 
  ql/src/test/results/clientpositive/intersect.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/51755/diff/


Testing
-------


Thanks,

pengcheng xiong

Re: Review Request 51755: Support Intersect Except

Posted by pengcheng xiong <px...@hortonworks.com>.

> On Oct. 21, 2016, 2:23 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java, line 42
> > <https://reviews.apache.org/r/51755/diff/6/?file=1542500#file1542500line42>
> >
> >     It seems this rule is not called from anywhere. Does that mean we will generate suboptimal plans where it was applicable now?

it is called in Calcite planner {code} perfLogger.PerfLogBegin(this.getClass().getName(), PerfLogger.OPTIMIZER);
      basePlan = hepPlan(basePlan, true, mdProvider, null, HepMatchOrder.BOTTOM_UP,
          HiveProjectOverIntersectRemoveRule.INSTANCE, HiveIntersectMergeRule.INSTANCE);
      perfLogger.PerfLogEnd(this.getClass().getName(), PerfLogger.OPTIMIZER,
          "Calcite: HiveIntersectMerge rule");{code}


> On Oct. 21, 2016, 2:23 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectOverIntersectRemoveRule.java, line 52
> > <https://reviews.apache.org/r/51755/diff/6/?file=1542502#file1542502line52>
> >
> >     I presume you want if() here instead of assert.

I think assert is better. it will throw out error if it is not trivial. This rule should only match for the () for the intersect.


> On Oct. 21, 2016, 2:23 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java, line 1196
> > <https://reviews.apache.org/r/51755/diff/6/?file=1542505#file1542505line1196>
> >
> >     No intersect-merge rule?

there is.


- pengcheng


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


On Oct. 20, 2016, 6:12 p.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51755/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2016, 6:12 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-12765
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties 09833ff 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f308832 
>   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/HiveProjectOverIntersectRemoveRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java 3ec9dac 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java 63aa086 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java d32a0a7 
>   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 9d58193 
>   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/except_all.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/except_distinct.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_all.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_distinct.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_merge.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51755/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>

Re: Review Request 51755: Support Intersect Except

Posted by pengcheng xiong <px...@hortonworks.com>.

> On Oct. 21, 2016, 2:23 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectOverIntersectRemoveRule.java, line 52
> > <https://reviews.apache.org/r/51755/diff/6/?file=1542502#file1542502line52>
> >
> >     I presume you want if() here instead of assert.
> 
> pengcheng xiong wrote:
>     I think assert is better. it will throw out error if it is not trivial. This rule should only match for the () for the intersect.

I have changed my mind... i will put a if there and it will throw error if it is not trivial.


- pengcheng


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


On Oct. 20, 2016, 6:12 p.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51755/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2016, 6:12 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-12765
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties 09833ff 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f308832 
>   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/HiveProjectOverIntersectRemoveRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java 3ec9dac 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java 63aa086 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java d32a0a7 
>   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 9d58193 
>   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/except_all.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/except_distinct.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_all.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_distinct.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_merge.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51755/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>

Re: Review Request 51755: Support Intersect Except

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/#review153543
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java (line 956)
<https://reviews.apache.org/r/51755/#comment222938>

    Can all these new methods in this class be protected?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveExcept.java (line 30)
<https://reviews.apache.org/r/51755/#comment222944>

    whitespace



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java (line 42)
<https://reviews.apache.org/r/51755/#comment222945>

    It seems this rule is not called from anywhere. Does that mean we will generate suboptimal plans where it was applicable now?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 90)
<https://reviews.apache.org/r/51755/#comment222946>

    Whitespaces in the file.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectOverIntersectRemoveRule.java (line 52)
<https://reviews.apache.org/r/51755/#comment222937>

    I presume you want if() here instead of assert.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectOverIntersectRemoveRule.java (line 56)
<https://reviews.apache.org/r/51755/#comment222943>

    private method



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java (line 71)
<https://reviews.apache.org/r/51755/#comment222939>

    Is unordered() needed here? If so, please add comments.



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (line 1196)
<https://reviews.apache.org/r/51755/#comment222941>

    No intersect-merge rule?



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (line 1201)
<https://reviews.apache.org/r/51755/#comment222940>

    Update this string.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 515)
<https://reviews.apache.org/r/51755/#comment222942>

    Can reuse newly added ErrorMsg.


- Ashutosh Chauhan


On Oct. 20, 2016, 6:12 p.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51755/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2016, 6:12 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-12765
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties 09833ff 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f308832 
>   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/HiveProjectOverIntersectRemoveRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java 3ec9dac 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java 63aa086 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java d32a0a7 
>   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 9d58193 
>   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/except_all.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/except_distinct.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_all.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_distinct.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_merge.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51755/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>

Re: Review Request 51755: Support Intersect Except

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/
-----------------------------------------------------------

(Updated Oct. 22, 2016, 2:55 a.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-12765


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties 09833ff 
  ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f308832 
  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/HiveProjectOverIntersectRemoveRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java 3ec9dac 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java 63aa086 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java d32a0a7 
  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 9d58193 
  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/except_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/except_distinct.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_distinct.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_merge.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/51755/diff/


Testing
-------


Thanks,

pengcheng xiong

Re: Review Request 51755: Support Intersect Except

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/
-----------------------------------------------------------

(Updated Oct. 22, 2016, 12:31 a.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-12765


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties 09833ff 
  ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f308832 
  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/HiveProjectOverIntersectRemoveRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java 3ec9dac 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java 63aa086 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java d32a0a7 
  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 9d58193 
  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/except_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/except_distinct.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_distinct.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_merge.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/51755/diff/


Testing
-------


Thanks,

pengcheng xiong

Re: Review Request 51755: Support Intersect Except

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/
-----------------------------------------------------------

(Updated Oct. 20, 2016, 6:12 p.m.)


Review request for hive and Ashutosh Chauhan.


Summary (updated)
-----------------

Support Intersect Except


Repository: hive-git


Description
-------

HIVE-12765


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties 09833ff 
  ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f308832 
  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/HiveProjectOverIntersectRemoveRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java 3ec9dac 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java 63aa086 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java d32a0a7 
  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 9d58193 
  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/except_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/except_distinct.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_distinct.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_merge.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/51755/diff/


Testing
-------


Thanks,

pengcheng xiong

Re: Review Request 51755: Support Intersect Distinct

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/
-----------------------------------------------------------

(Updated Oct. 19, 2016, 1:36 a.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-12765


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties 8aee7f5 
  ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f308832 
  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/translator/ASTConverter.java 63aa086 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java d32a0a7 
  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 9d58193 
  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/except_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/except_distinct.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_distinct.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/intersect_merge.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/51755/diff/


Testing
-------


Thanks,

pengcheng xiong

Re: Review Request 51755: Support Intersect Distinct

Posted by pengcheng xiong <px...@hortonworks.com>.

> On Oct. 18, 2016, 3:31 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java, line 41
> > <https://reviews.apache.org/r/51755/diff/4/?file=1538671#file1538671line41>
> >
> >     Any reason for adding Minus as reserved word?

As except is a reserved keyword in SQL2011, although minus is an oracle defined operator, i think it makes sense to make it reserved.


- pengcheng


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


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

Re: Review Request 51755: Support Intersect Distinct

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/#review153098
-----------------------------------------------------------




itests/src/test/resources/testconfiguration.properties (line 53)
<https://reviews.apache.org/r/51755/#comment222376>

    Lets do it other way round. Write all new tests on MiniLlapLocalDriver and have one on MR.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SimpleFetchOptimizer.java (lines 130 - 132)
<https://reviews.apache.org/r/51755/#comment222377>

    Is this needed?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java (lines 162 - 170)
<https://reviews.apache.org/r/51755/#comment222378>

    Now this is covered in different jira, can be removed.



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (line 1528)
<https://reviews.apache.org/r/51755/#comment222380>

    Add opcode in error message.



ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java (line 41)
<https://reviews.apache.org/r/51755/#comment222381>

    Any reason for adding Minus as reserved word?



ql/src/test/results/clientpositive/auto_join8.q.out (line 94)
<https://reviews.apache.org/r/51755/#comment222372>

    Not folding null constant anymore?



ql/src/test/results/clientpositive/input25.q.out (line 122)
<https://reviews.apache.org/r/51755/#comment222375>

    Shuffling unnecssary constant column?



ql/src/test/results/clientpositive/join8.q.out (line 97)
<https://reviews.apache.org/r/51755/#comment222373>

    Not folding null constant anymore?



ql/src/test/results/clientpositive/smb_mapjoin_25.q.out (line 116)
<https://reviews.apache.org/r/51755/#comment222374>

    Shuffling unnecessary column?


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

Re: Review Request 51755: Support Intersect Except

Posted by pengcheng xiong <px...@hortonworks.com>.

> On Oct. 18, 2016, 11:09 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java, line 956
> > <https://reviews.apache.org/r/51755/diff/4/?file=1538656#file1538656line956>
> >
> >     Does this need to be public?

Yes, it is used by two rules and it may be reused in the future.


- pengcheng


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


On Oct. 20, 2016, 6:12 p.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51755/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2016, 6:12 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-12765
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties 09833ff 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f308832 
>   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/HiveProjectOverIntersectRemoveRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSortLimitPullUpConstantsRule.java 3ec9dac 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java 63aa086 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java d32a0a7 
>   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 9d58193 
>   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/except_all.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/except_distinct.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_all.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_distinct.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/intersect_merge.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51755/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>

Re: Review Request 51755: Support Intersect Distinct

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
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
> 
>

Re: Review Request 51755: Support Intersect Distinct

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
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 (updated)
-----

  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

Re: Review Request 51755: Support Intersect Distinct

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/
-----------------------------------------------------------

(Updated Oct. 13, 2016, 12:01 a.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-12765


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties 97e310d 
  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/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/except_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/except_distinct.q.out PRE-CREATION 
  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/tez/intersect_merge.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/51755/diff/


Testing
-------


Thanks,

pengcheng xiong

Re: Review Request 51755: Support Intersect Distinct

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/
-----------------------------------------------------------

(Updated Oct. 11, 2016, 10:42 p.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-12765


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties fbba0cd 
  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/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 bef3acf 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 6ae731f 
  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/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/except_all.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/except_distinct.q.out PRE-CREATION 
  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/tez/intersect_merge.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/51755/diff/


Testing
-------


Thanks,

pengcheng xiong

Re: Review Request 51755: Support Intersect Distinct

Posted by pengcheng xiong <px...@hortonworks.com>.

> On Oct. 7, 2016, 2:57 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java, line 59
> > <https://reviews.apache.org/r/51755/diff/1/?file=1494884#file1494884line59>
> >
> >     Is this check not required on bottomIntersect?

We need this. if top is distinct, we can always merge whether bottom is distinct or not. if top is all, we can only merge if bottom is also all. that is to say, we should bail out if top is all and bottom is distinct. (I made some modifications.)


> On Oct. 7, 2016, 2:57 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g, line 93
> > <https://reviews.apache.org/r/51755/diff/1/?file=1494889#file1494889line93>
> >
> >     Good to call this TOK_EXCEPTALL since KW is except not minus.

We have both of them... actually both except and minus are kws. anyway, im going to change it to except.


- pengcheng


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


On Sept. 9, 2016, 1:41 a.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51755/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2016, 1:41 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-12765
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelFactories.java cf93ed8 
>   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/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/translator/ASTConverter.java 9f5e733 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ff94160 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 7ceb005 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g df596ff 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9ba1865 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBExpr.java cccf0f6 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 943d9d7 
>   ql/src/test/queries/clientpositive/intersect.q PRE-CREATION 
>   ql/src/test/results/clientpositive/intersect.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51755/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>

Re: Review Request 51755: Support Intersect Distinct

Posted by Ashutosh Chauhan <ha...@apache.org>.

> On Oct. 7, 2016, 2:57 a.m., Ashutosh Chauhan wrote:
> >

Took first pass look. You may wait to address these comments till I finish full review.


- Ashutosh


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


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

Re: Review Request 51755: Support Intersect Distinct

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51755/#review148788
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelFactories.java (line 209)
<https://reviews.apache.org/r/51755/#comment216301>

    Error message: Union or intersect



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveIntersect.java (line 30)
<https://reviews.apache.org/r/51755/#comment220241>

    whitespace



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java (line 31)
<https://reviews.apache.org/r/51755/#comment216303>

    It will be good to add a comment describing transformation we are trying to achieve here. Better will be in terms of sql level transform, if possible.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java (line 59)
<https://reviews.apache.org/r/51755/#comment216304>

    Is this check not required on bottomIntersect?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java (line 65)
<https://reviews.apache.org/r/51755/#comment216306>

    This assert should be before if()



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 64)
<https://reviews.apache.org/r/51755/#comment220242>

    Good to add more details on how this rule is transforming operator tree.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 102)
<https://reviews.apache.org/r/51755/#comment220243>

    Will be useful to explain why Project is needed here.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 107)
<https://reviews.apache.org/r/51755/#comment220244>

    Remove TODO



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 108)
<https://reviews.apache.org/r/51755/#comment220245>

    rethrow after logging.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 148)
<https://reviews.apache.org/r/51755/#comment220246>

    Add comments on why we need to have project here which is not doing any transformations.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 150)
<https://reviews.apache.org/r/51755/#comment220247>

    TODO



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 151)
<https://reviews.apache.org/r/51755/#comment220248>

    log and rethrow.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 180)
<https://reviews.apache.org/r/51755/#comment220249>

    It is a deterministic function.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 189)
<https://reviews.apache.org/r/51755/#comment220250>

    TODO



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 190)
<https://reviews.apache.org/r/51755/#comment220251>

    log and rethrow.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 212)
<https://reviews.apache.org/r/51755/#comment220252>

    remove TODO



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java (line 213)
<https://reviews.apache.org/r/51755/#comment220253>

    log and rethrow.



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (line 2090)
<https://reviews.apache.org/r/51755/#comment220254>

    ws



ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g (line 93)
<https://reviews.apache.org/r/51755/#comment220255>

    Good to call this TOK_EXCEPTALL since KW is except not minus.



ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g (line 94)
<https://reviews.apache.org/r/51755/#comment220258>

    TOK_EXCEPTDISTINCT



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 516)
<https://reviews.apache.org/r/51755/#comment220256>

    insideView not needed anymore?



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 523)
<https://reviews.apache.org/r/51755/#comment220257>

    insideView not needed anymore?



ql/src/test/queries/clientpositive/intersect.q (line 40)
<https://reviews.apache.org/r/51755/#comment220259>

    Add a test where there is a GBy on different columns with different aggregates on two branches of intersect.


- Ashutosh Chauhan


On Sept. 9, 2016, 1:41 a.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51755/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2016, 1:41 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-12765
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelFactories.java cf93ed8 
>   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/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/translator/ASTConverter.java 9f5e733 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ff94160 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 7ceb005 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g df596ff 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9ba1865 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBExpr.java cccf0f6 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 943d9d7 
>   ql/src/test/queries/clientpositive/intersect.q PRE-CREATION 
>   ql/src/test/results/clientpositive/intersect.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51755/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>