Re: Review Request 69077: HIVE-20748

2018-12-19 Thread Ashutosh Chauhan 

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




ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
Lines 1275 (patched)


We can remove this statement, doesnt provide any extra info. And just keep 
first line.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
Line 70 (original), 64 (patched)


Better name : resultCacheInvalidReason;



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Line 385 (original), 385 (patched)


Name : canCBOHandleReason.



ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java
Line 558 (original), 558 (patched)


Can we drop public? And leave it as protected or default.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Line 400 (original), 400 (patched)


invalidResultCacheReason.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Lines 13722 (patched)


can remove this sentence.


- Ashutosh Chauhan


On Oct. 19, 2018, 11:26 p.m., Jesús Camacho Rodríguez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69077/
> ---
> 
> (Updated Oct. 19, 2018, 11:26 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-20748
> https://issues.apache.org/jira/browse/HIVE-20748
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20748
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 807f159daa98d40e667914adc6c53fb8ecabf998 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
>  df216e7555bff4756130f5e097bdb6b0e5e7eef5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
> 22f3266c87f1d42c254893b424b68e757fb2953b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 
> be1c59f93272352705731c8c7a02433c7ac3d6dc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
> eed875e7a4475f207727d5d536521fdba0c329fb 
>   ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite_2.q 
> PRE-CREATION 
>   
> ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite.q
>  PRE-CREATION 
>   
> ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite_2.q
>  PRE-CREATION 
>   ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite_2.q.out 
> PRE-CREATION 
>   
> ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite.q.out
>  PRE-CREATION 
>   
> ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite_2.q.out
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69077/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>

Re: Review Request 69077: HIVE-20748

2018-10-19 Thread Jesús Camacho Rodríguez 

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

(Updated Oct. 19, 2018, 11:26 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
---

HIVE-20748


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
807f159daa98d40e667914adc6c53fb8ecabf998 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
 df216e7555bff4756130f5e097bdb6b0e5e7eef5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
22f3266c87f1d42c254893b424b68e757fb2953b 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 
be1c59f93272352705731c8c7a02433c7ac3d6dc 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
eed875e7a4475f207727d5d536521fdba0c329fb 
  ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite.q 
PRE-CREATION 
  ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite_2.q 
PRE-CREATION 
  
ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite.q 
PRE-CREATION 
  
ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite_2.q
 PRE-CREATION 
  ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite.q.out 
PRE-CREATION 
  ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite_2.q.out 
PRE-CREATION 
  
ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite.q.out
 PRE-CREATION 
  
ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite_2.q.out
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69077/diff/2/

Changes: https://reviews.apache.org/r/69077/diff/1-2/


Testing
---


Thanks,

Jesús Camacho Rodríguez

Re: Review Request 69077: HIVE-20748

2018-10-19 Thread Jesús Camacho Rodríguez 


> On Oct. 19, 2018, 6:01 a.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite.q.out
> > Lines 22 (patched)
> > 
> >
> > Can we provide better error message here? That rewriting can't be 
> > enabled because it contains outer join.

I improved the propagation from the Calcite planner.


- Jesús


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


On Oct. 19, 2018, 12:17 a.m., Jesús Camacho Rodríguez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69077/
> ---
> 
> (Updated Oct. 19, 2018, 12:17 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-20748
> https://issues.apache.org/jira/browse/HIVE-20748
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20748
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 807f159daa98d40e667914adc6c53fb8ecabf998 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
>  df216e7555bff4756130f5e097bdb6b0e5e7eef5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOptAutomaticRewritingMaterializationValidator.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
> 22f3266c87f1d42c254893b424b68e757fb2953b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 
> be1c59f93272352705731c8c7a02433c7ac3d6dc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
> eed875e7a4475f207727d5d536521fdba0c329fb 
>   ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite_2.q 
> PRE-CREATION 
>   
> ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite.q
>  PRE-CREATION 
>   
> ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite_2.q
>  PRE-CREATION 
>   ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite_2.q.out 
> PRE-CREATION 
>   
> ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite.q.out
>  PRE-CREATION 
>   
> ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite_2.q.out
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69077/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>

Re: Review Request 69077: HIVE-20748

2018-10-19 Thread Ashutosh Chauhan 

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




ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 1768-1779 (patched)


Is there a reason to do these validations in seperate passes? If they can 
be combined in 1 pass, that will help keep latency of compiler low.



ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite.q.out
Lines 22 (patched)


Can we provide better error message here? That rewriting can't be enabled 
because it contains outer join.



ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite_2.q.out
Lines 36 (patched)


better error message.


- Ashutosh Chauhan


On Oct. 19, 2018, 12:17 a.m., Jesús Camacho Rodríguez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69077/
> ---
> 
> (Updated Oct. 19, 2018, 12:17 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-20748
> https://issues.apache.org/jira/browse/HIVE-20748
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20748
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 807f159daa98d40e667914adc6c53fb8ecabf998 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
>  df216e7555bff4756130f5e097bdb6b0e5e7eef5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOptAutomaticRewritingMaterializationValidator.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
> 22f3266c87f1d42c254893b424b68e757fb2953b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 
> be1c59f93272352705731c8c7a02433c7ac3d6dc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
> eed875e7a4475f207727d5d536521fdba0c329fb 
>   ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite_2.q 
> PRE-CREATION 
>   
> ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite.q
>  PRE-CREATION 
>   
> ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite_2.q
>  PRE-CREATION 
>   ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite_2.q.out 
> PRE-CREATION 
>   
> ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite.q.out
>  PRE-CREATION 
>   
> ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite_2.q.out
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69077/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>

Re: Review Request 69077: HIVE-20748

2018-10-18 Thread Jesús Camacho Rodríguez 

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

(Updated Oct. 19, 2018, 12:17 a.m.)


Review request for hive and Ashutosh Chauhan.


Summary (updated)
-

HIVE-20748


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


Repository: hive-git


Description (updated)
---

HIVE-20748


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
807f159daa98d40e667914adc6c53fb8ecabf998 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
 df216e7555bff4756130f5e097bdb6b0e5e7eef5 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOptAutomaticRewritingMaterializationValidator.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
22f3266c87f1d42c254893b424b68e757fb2953b 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 
be1c59f93272352705731c8c7a02433c7ac3d6dc 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
eed875e7a4475f207727d5d536521fdba0c329fb 
  ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite.q 
PRE-CREATION 
  ql/src/test/queries/clientnegative/materialized_view_no_cbo_rewrite_2.q 
PRE-CREATION 
  
ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite.q 
PRE-CREATION 
  
ql/src/test/queries/clientnegative/materialized_view_no_supported_op_rewrite_2.q
 PRE-CREATION 
  ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite.q.out 
PRE-CREATION 
  ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite_2.q.out 
PRE-CREATION 
  
ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite.q.out
 PRE-CREATION 
  
ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite_2.q.out
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69077/diff/1/


Testing
---


Thanks,

Jesús Camacho Rodríguez