Re: Review Request 42671: Improve dynamic partition loading

2016-01-27 Thread j . prasanth . j 

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


Ship it!




Ship It!

- Prasanth_J


On Jan. 27, 2016, 7 a.m., Ashutosh Chauhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42671/
> ---
> 
> (Updated Jan. 27, 2016, 7 a.m.)
> 
> 
> Review request for hive, Gopal V and Prasanth_J.
> 
> 
> Bugs: HIVE-12897
> https://issues.apache.org/jira/browse/HIVE-12897
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Reduces number of calls to Metastore.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 97fe7bc 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> eee7f1b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> e044c73 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 3289cfc 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java efb50b2 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java
>  48105de 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java
>  2c2339a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSemanticAnalyzer.java 
> c1e9ec1 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java 95d5635 
> 
> Diff: https://reviews.apache.org/r/42671/diff/
> 
> 
> Testing
> ---
> 
> Regression test suite.
> 
> 
> Thanks,
> 
> Ashutosh Chauhan
> 
>

Re: Review Request 42671: Improve dynamic partition loading

2016-01-26 Thread j . prasanth . j 

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




metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 293)


nit: Putting positive condition first is little easier to read. Flipping 
the conditions in this case. Also isEmpty() can be used.



ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java (line 703)


nit: unwanted '+' between strings.



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 


Wonderful! Recurses through files but adds s.getPath().getParent(). You 
saved a kitten! :)



ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java (line 236)


Why HiveException? Why not wrap hive exception inside SemanticException in 
appropriate place instead of changing the signature.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 2637)


All these changes looks unnecessary. IMO SemanticAnalyzer should throw 
SemanticException.



ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java (line 80)


nit: use confVal.isEmpty() instead? since there is already a non-null check 
I think it will be safe. It will be much better if we put positive check first.

this.whiteListPatten = confVal == null || confVal.isEmpty() ? null : 
Pattern.compile(confVal);


- Prasanth_J


On Jan. 26, 2016, 5:42 p.m., Ashutosh Chauhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42671/
> ---
> 
> (Updated Jan. 26, 2016, 5:42 p.m.)
> 
> 
> Review request for hive, Gopal V and Prasanth_J.
> 
> 
> Bugs: HIVE-12897
> https://issues.apache.org/jira/browse/HIVE-12897
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Reduces number of calls to Metastore.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 97fe7bc 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> eee7f1b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> e044c73 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 3289cfc 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java efb50b2 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java
>  48105de 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> af1ee20 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 3fefbd7 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
> 1f30cbd 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java
>  2c2339a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSemanticAnalyzer.java 
> c1e9ec1 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ff90a6 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java 
> 5b4365c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java 95d5635 
> 
> Diff: https://reviews.apache.org/r/42671/diff/
> 
> 
> Testing
> ---
> 
> Regression test suite.
> 
> 
> Thanks,
> 
> Ashutosh Chauhan
> 
>

Re: Review Request 42671: Improve dynamic partition loading

2016-01-26 Thread Ashutosh Chauhan 

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

(Updated Jan. 27, 2016, 7 a.m.)


Review request for hive, Gopal V and Prasanth_J.


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


Repository: hive-git


Description
---

Reduces number of calls to Metastore.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 97fe7bc 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
eee7f1b 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java e044c73 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 3289cfc 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java efb50b2 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java
 48105de 
  
ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java
 2c2339a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSemanticAnalyzer.java 
c1e9ec1 
  ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java 95d5635 

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


Testing
---

Regression test suite.


Thanks,

Ashutosh Chauhan

Re: Review Request 42671: Improve dynamic partition loading

2016-01-26 Thread Ashutosh Chauhan 

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

(Updated Jan. 26, 2016, 5:42 p.m.)


Review request for hive, Gopal V and Prasanth_J.


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


Repository: hive-git


Description
---

Reduces number of calls to Metastore.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 97fe7bc 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
eee7f1b 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java e044c73 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 3289cfc 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java efb50b2 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java
 48105de 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java af1ee20 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 3fefbd7 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
1f30cbd 
  
ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java
 2c2339a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSemanticAnalyzer.java 
c1e9ec1 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ff90a6 
  ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java 
5b4365c 
  ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java 95d5635 

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


Testing
---

Regression test suite.


Thanks,

Ashutosh Chauhan