You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergey Shelukhin <se...@hortonworks.com> on 2014/10/08 00:22:23 UTC

Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

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

(Updated Oct. 7, 2014, 10:22 p.m.)


Review request for hive, Ashutosh Chauhan and John Pullokkaran.


Repository: hive-git


Description
-------

see JIRA


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 19110ce 
  ql/src/test/queries/clientpositive/cbo_correctness.q f7f0722 
  ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
  ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 

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


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Oct. 22, 2014, 2:46 a.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 384
> > <https://reviews.apache.org/r/25550/diff/6/?file=726844#file726844line384>
> >
> >     Why not use //?

it's a javadoc


- Sergey


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


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review57734
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98573>

    Why not use //?


- John Pullokkaran


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Oct. 22, 2014, 6:40 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 12401
> > <https://reviews.apache.org/r/25550/diff/6/?file=726844#file726844line12401>
> >
> >     Wouldn't it be better to change this method to be verbose always.
> >     
> >     May be change the signature to
> >     
> >     boolean canHandleQuery(QB, topLevelQB, StringBuffer){
> >     
> >     Use the implementation under verbose
> >     }

if logging is disabled, there's nowhere to write the verbose message, so there's no reason to generate it?


- Sergey


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


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review57850
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98725>

    Wouldn't it be better to change this method to be verbose always.
    
    May be change the signature to
    
    boolean canHandleQuery(QB, topLevelQB, StringBuffer){
    
    Use the implementation under verbose
    }


- John Pullokkaran


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Oct. 22, 2014, 2:44 a.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 372
> > <https://reviews.apache.org/r/25550/diff/6/?file=726844#file726844line372>
> >
> >     Some tests reuses semanticanalyzer possibly from different threads.
> >     
> >     Its more defensive code than encountered problem.

I don't think semantic analyzer is used from multiple threads... given how it's organized it would lead to disaster...
Anyway, there's no threads communicating via this variable, and boolean assignment is atomic, so volatile should not be needed.


- Sergey


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


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review57733
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98569>

    Some tests reuses semanticanalyzer possibly from different threads.
    
    Its more defensive code than encountered problem.


- John Pullokkaran


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Oct. 23, 2014, 7:47 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 9960
> > <https://reviews.apache.org/r/25550/diff/7/?file=728978#file728978line9960>
> >
> >     Seems like we could refactor the code here for ease of maintanence:
> >     1. Modify PreCboCtx to include error msg
> >     2. processStmtForCBO(PreCboCtx, qb)
> >     3. Move all of the logic below to #2
> >           // Note: for now, we don't actually pass the queryForCbo to CBO, because it accepts qb, not
> >           //    AST, and can also access all the private stuff in SA. We rely on the fact that CBO
> >           //    ignores the unknown tokens (create table, destination), so if the query is otherwise ok,
> >           //    it is as if we did remove those and gave CBO the proper AST. That is kinda hacky.
> >            queryForCbo = ast;
> >            if (cboCtx.type == PreCboCtx.Type.CTAS) {
> >              // nodeOfInterest is the query
> >              queryForCbo = cboCtx.nodeOfInterest;
> >            }
> >            int root = queryForCbo.getToken().getType();
> >            boolean isSupportedRoot =
> >                root == HiveParser.TOK_QUERY || root == HiveParser.TOK_EXPLAIN || qb.isCTAS();
> >            // Assumption: If top level QB is query then everything below it must also be Query
> >            // Can there be an insert or CTAS that wouldn't
> >           //        be supported and would require additional checks similar to IsQuery?
> >            boolean isSupportedType =
> >                qb.getIsQuery() || qb.isCTAS() || cboCtx.type == PreCboCtx.Type.INSERT;
> >            runCBO = runCBO && isSupportedRoot && isSupportedType && createVwDesc == null;
> >     
> >     .3 If PreCboCtx err msg is empty then check if CBO can handle Select Query
> >     rename canHandleQuery to canHandleSelectQuery

refactored all the checks into the method, and all the non-checks separately


- Sergey


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


On Oct. 22, 2014, 8:34 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2014, 8:34 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review58073
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98968>

    Seems like we could refactor the code here for ease of maintanence:
    1. Modify PreCboCtx to include error msg
    2. processStmtForCBO(PreCboCtx, qb)
    3. Move all of the logic below to #2
          // Note: for now, we don't actually pass the queryForCbo to CBO, because it accepts qb, not
          //    AST, and can also access all the private stuff in SA. We rely on the fact that CBO
          //    ignores the unknown tokens (create table, destination), so if the query is otherwise ok,
          //    it is as if we did remove those and gave CBO the proper AST. That is kinda hacky.
           queryForCbo = ast;
           if (cboCtx.type == PreCboCtx.Type.CTAS) {
             // nodeOfInterest is the query
             queryForCbo = cboCtx.nodeOfInterest;
           }
           int root = queryForCbo.getToken().getType();
           boolean isSupportedRoot =
               root == HiveParser.TOK_QUERY || root == HiveParser.TOK_EXPLAIN || qb.isCTAS();
           // Assumption: If top level QB is query then everything below it must also be Query
           // Can there be an insert or CTAS that wouldn't
          //        be supported and would require additional checks similar to IsQuery?
           boolean isSupportedType =
               qb.getIsQuery() || qb.isCTAS() || cboCtx.type == PreCboCtx.Type.INSERT;
           runCBO = runCBO && isSupportedRoot && isSupportedType && createVwDesc == null;
    
    .3 If PreCboCtx err msg is empty then check if CBO can handle Select Query
    rename canHandleQuery to canHandleSelectQuery


- John Pullokkaran


On Oct. 22, 2014, 8:34 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2014, 8:34 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review58075
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98970>

    Expanding on the previous comment: something like  below
    
        if (runCBO) {
        	processStmtForCbo(cboCtx, qb);
        	if (cboCtx.getErrMsg().isEmpty()) {
        	   canHandleQuery(cboCtx, qb, true);
        	}
        	if (cboCtx.getErrMsg().isEmpty()) {
                    disableJoinMerge = true;
                  } else {
                	  runCBO = false;
                	  if (LOG.isInfoEnabled()) {
                		  LOG.info("Not invoking CBO because the statement " + msg));
                  }
        	}


- John Pullokkaran


On Oct. 22, 2014, 8:34 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2014, 8:34 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Oct. 23, 2014, 7:33 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 9981
> > <https://reviews.apache.org/r/25550/diff/7/?file=728978#file728978line9981>
> >
> >     Shouldn't this check be for msg.isEmpty as opposed to null. Since in verbose mode msg will never be empty.

see structure of the "if". In case all conditions is satisfied, it will always return null. Verbose is only checked later.


> On Oct. 23, 2014, 7:33 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 12419
> > <https://reviews.apache.org/r/25550/diff/7/?file=728978#file728978line12419>
> >
> >     Why don't we remove this if block & remove verbose flag.
> >     
> >     Always return the reason for not running CBO.
> >     Caller based on log level can decide to log the reason.

building the reason can be expensive...


> On Oct. 23, 2014, 7:33 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 12437
> > <https://reviews.apache.org/r/25550/diff/7/?file=728978#file728978line12437>
> >
> >     As pointed out last time, wouldn't this cause CBO to never run in Log level info.
> >     
> >     Caller code is
> >             msg = canHandleQuery(qb, true, LOG.isInfoEnabled());
> >              runCBO = runCBO && (msg == null);

no, verbose is only checked when CBO already cannot run. I'll make code more clear


- Sergey


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


On Oct. 22, 2014, 8:34 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2014, 8:34 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review58065
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98965>

    Shouldn't this check be for msg.isEmpty as opposed to null. Since in verbose mode msg will never be empty.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98963>

    Why don't we remove this if block & remove verbose flag.
    
    Always return the reason for not running CBO.
    Caller based on log level can decide to log the reason.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98964>

    As pointed out last time, wouldn't this cause CBO to never run in Log level info.
    
    Caller code is
            msg = canHandleQuery(qb, true, LOG.isInfoEnabled());
             runCBO = runCBO && (msg == null);


- John Pullokkaran


On Oct. 22, 2014, 8:34 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2014, 8:34 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Oct. 24, 2014, 5:10 p.m., John Pullokkaran wrote:
> > ql/src/test/queries/clientpositive/ctas_colname.q, line 9
> > <https://reviews.apache.org/r/25550/diff/8/?file=731255#file731255line9>
> >
> >     Why this change

see HIVE-8512, the original query is not valid and should fail


- Sergey


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


On Oct. 23, 2014, 9:11 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 9:11 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d8c50e3 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review58290
-----------------------------------------------------------



ql/src/test/queries/clientpositive/ctas_colname.q
<https://reviews.apache.org/r/25550/#comment99237>

    Why this change


- John Pullokkaran


On Oct. 23, 2014, 9:11 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 9:11 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d8c50e3 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Oct. 23, 2014, 10:51 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 6440
> > <https://reviews.apache.org/r/25550/diff/8/?file=731253#file731253line6440>
> >
> >     Can CTAS in non cbo route handle qualified column name?
> >     
> >     create table as select t1.key, t1.value from t1;

Yes... CBO processing of AST inserts explicit aliases at some point, which doesn't happen on non-CBO path


- Sergey


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


On Oct. 23, 2014, 9:11 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 9:11 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d8c50e3 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review58148
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment99077>

    Can CTAS in non cbo route handle qualified column name?
    
    create table as select t1.key, t1.value from t1;



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment99065>

    This logging is not needed anymore right, since caller logs it.


- John Pullokkaran


On Oct. 23, 2014, 9:11 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 9:11 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d8c50e3 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review58363
-----------------------------------------------------------

Ship it!


Ship It!

- John Pullokkaran


On Oct. 24, 2014, 8:35 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 8:35 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d8c50e3 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/
-----------------------------------------------------------

(Updated Oct. 24, 2014, 8:35 p.m.)


Review request for hive, Ashutosh Chauhan and John Pullokkaran.


Repository: hive-git


Description
-------

see JIRA


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d8c50e3 
  ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
  ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
  ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
  ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
  ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
  ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
  ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 

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


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/
-----------------------------------------------------------

(Updated Oct. 23, 2014, 9:11 p.m.)


Review request for hive, Ashutosh Chauhan and John Pullokkaran.


Repository: hive-git


Description
-------

see JIRA


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d8c50e3 
  ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
  ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
  ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
  ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
  ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
  ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
  ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 

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


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review58063
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98962>

    Seems not used


- John Pullokkaran


On Oct. 22, 2014, 8:34 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2014, 8:34 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review58082
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98973>

    Seems like it would be better to move CTAS/Insert specific fix up to a seperate function and call that from here.
    
    try {
            // 1. Gen Optimized AST
            ASTNode newAST = optiqPlanner.getOptimizedAST(prunedPartitions);
    init(false);
    newAST = fixUpQueryForCTASInsert(cboCtx, newAST)


- John Pullokkaran


On Oct. 22, 2014, 8:34 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2014, 8:34 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/
-----------------------------------------------------------

(Updated Oct. 22, 2014, 8:34 p.m.)


Review request for hive, Ashutosh Chauhan and John Pullokkaran.


Repository: hive-git


Description
-------

see JIRA


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b 
  ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
  ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
  ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
  ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
  ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
  ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
  ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 

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


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review57853
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98729>

    Seems to be missing if ((!isInTest || conf.getVar(ConfVars.HIVEMAPREDMODE).equalsIgnoreCase("nonstrict"))


- John Pullokkaran


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review57856
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98731>

    !isInTest is missing from topLevelQB && queryProperties.getJoinCount() <= 1
    
    I know usually tests won't set that log level.


- John Pullokkaran


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Oct. 22, 2014, 6:48 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 12427
> > <https://reviews.apache.org/r/25550/diff/6/?file=726844#file726844line12427>
> >
> >     This seems wrong; because of this CBO wouldn't get excersised in verbose mode.
> >     
> >     Am i missing something?

it checks all the conditions in the first if, so this else if is only executed when CBO cannot run. 
I will add comments


- Sergey


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


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/#review57857
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/25550/#comment98732>

    This seems wrong; because of this CBO wouldn't get excersised in verbose mode.
    
    Am i missing something?


- John Pullokkaran


On Oct. 21, 2014, 1:13 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25550/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:13 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see JIRA
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
>   ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
>   ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
>   ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
>   ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
>   ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
>   ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25550: HIVE-8021 CBO: support CTAS and insert ... select

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25550/
-----------------------------------------------------------

(Updated Oct. 21, 2014, 1:13 a.m.)


Review request for hive, Ashutosh Chauhan and John Pullokkaran.


Repository: hive-git


Description
-------

see JIRA


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java dee7d7e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 37cbf7f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1b7a41d 
  ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 
  ql/src/test/queries/clientpositive/ctas_colname.q 5322626 
  ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c 
  ql/src/test/queries/clientpositive/insert0.q PRE-CREATION 
  ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 
  ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e 
  ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION 

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


Testing
-------


Thanks,

Sergey Shelukhin