Re: Review Request 20737: HIVE-6031: explain subquery rewrite for where clause predicates

2014-04-28 Thread Harish Butani 


> On April 28, 2014, 6:28 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java, line 
> > 137
> > 
> >
> > will be good to add a comment here for this replacement.

done


> On April 28, 2014, 6:28 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java,
> >  lines 53-56
> > 
> >
> > Is this needed ? Looks redundant code. If not, good to put some 
> > comments here.

removed


> On April 28, 2014, 6:28 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g, line 297
> > 
> >
> > I think we should not have explain subquery rewrite .. rather explain 
> > rewrite .. because later on we may want to extend this explain 
> > functionality beyond just subquery rewrites.

done


> On April 28, 2014, 6:28 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java, line 
> > 207
> > 
> >
> > Also, good to add a note here saying its non-explain path.

done


- Harish


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


On April 29, 2014, 12:43 a.m., Harish Butani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20737/
> ---
> 
> (Updated April 29, 2014, 12:43 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-6031
> https://issues.apache.org/jira/browse/HIVE-6031
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> explain subquery rewrite for where clause predicates
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 679c6ec 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 864e692 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a8b436e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java b7c9e65 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0e0395e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> f6b70d8 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java 07d32ed 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainSQRewriteWork.java 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/subquery_exists_explain_rewrite.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/subquery_in_explain_rewrite.q 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/subquery_exists_explain_rewrite.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/subquery_in_explain_rewrite.q.out 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20737/diff/
> 
> 
> Testing
> ---
> 
> new tests added
> 
> 
> Thanks,
> 
> Harish Butani
> 
>

Re: Review Request 20737: HIVE-6031: explain subquery rewrite for where clause predicates

2014-04-28 Thread Harish Butani 

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

(Updated April 29, 2014, 12:43 a.m.)


Review request for hive and Ashutosh Chauhan.


Changes
---

address review comments


Bugs: HIVE-6031
https://issues.apache.org/jira/browse/HIVE-6031


Repository: hive-git


Description
---

explain subquery rewrite for where clause predicates


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 679c6ec 
  
ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 864e692 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a8b436e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java b7c9e65 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0e0395e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
f6b70d8 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java 07d32ed 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainSQRewriteWork.java 
PRE-CREATION 
  ql/src/test/queries/clientpositive/subquery_exists_explain_rewrite.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/subquery_in_explain_rewrite.q PRE-CREATION 
  ql/src/test/results/clientpositive/subquery_exists_explain_rewrite.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/subquery_in_explain_rewrite.q.out 
PRE-CREATION 

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


Testing
---

new tests added


Thanks,

Harish Butani

Re: Review Request 20737: HIVE-6031: explain subquery rewrite for where clause predicates

2014-04-28 Thread Ashutosh Chauhan 

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


Few comments. Major one is to drop subquery from explain subquery rewrite 
syntax.


ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java


will be good to add a comment here for this replacement.



ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java


Is this needed ? Looks redundant code. If not, good to put some comments 
here.



ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g


I think we should not have explain subquery rewrite .. rather explain 
rewrite .. because later on we may want to extend this explain functionality 
beyond just subquery rewrites. 



ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java


Doesn't make lot of difference, but QBSQRewrite extends QBSQRewriteNoop 
seems more natural.



ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java


Also, good to add a note here saying its non-explain path.


- Ashutosh Chauhan


On April 25, 2014, 10:57 p.m., Harish Butani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20737/
> ---
> 
> (Updated April 25, 2014, 10:57 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-6031
> https://issues.apache.org/jira/browse/HIVE-6031
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> explain subquery rewrite for where clause predicates
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 679c6ec 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 864e692 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a8b436e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java b7c9e65 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 3b33dc2 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> e7d0359 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java 07d32ed 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainSQRewriteWork.java 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/subquery_exists_explain_rewrite.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/subquery_in_explain_rewrite.q 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/subquery_exists_explain_rewrite.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/subquery_in_explain_rewrite.q.out 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20737/diff/
> 
> 
> Testing
> ---
> 
> new tests added
> 
> 
> Thanks,
> 
> Harish Butani
> 
>

Review Request 20737: HIVE-6031: explain subquery rewrite for where clause predicates

2014-04-25 Thread Harish Butani 

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

Review request for hive and Ashutosh Chauhan.


Bugs: HIVE-6031
https://issues.apache.org/jira/browse/HIVE-6031


Repository: hive-git


Description
---

explain subquery rewrite for where clause predicates


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 679c6ec 
  
ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 864e692 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a8b436e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java b7c9e65 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 3b33dc2 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
e7d0359 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java 07d32ed 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainSQRewriteWork.java 
PRE-CREATION 
  ql/src/test/queries/clientpositive/subquery_exists_explain_rewrite.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/subquery_in_explain_rewrite.q PRE-CREATION 
  ql/src/test/results/clientpositive/subquery_exists_explain_rewrite.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/subquery_in_explain_rewrite.q.out 
PRE-CREATION 

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


Testing
---

new tests added


Thanks,

Harish Butani