Re: Review Request 47242: HIVE-13726

2016-05-12 Thread Ashutosh Chauhan

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

(Updated May 12, 2016, 7:30 a.m.)


Review request for hive and Rui Li.


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


Repository: hive-git


Description
---

For insert overwrite significant amount might be spent in deleting existing 
files. This patch parallelizes this task.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/common/FileUtils.java b65c35b 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java b5e660b 

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


Testing
---

existing regression tests. No change in functionality.


Thanks,

Ashutosh Chauhan



Re: Review Request 47242: HIVE-13726

2016-05-12 Thread Ashutosh Chauhan

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

(Updated May 12, 2016, 6:07 a.m.)


Review request for hive and Rui Li.


Changes
---

Removed redundant FNFE. Kept non-static method as protected method is not 
currently used in sub classes.


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


Repository: hive-git


Description
---

For insert overwrite significant amount might be spent in deleting existing 
files. This patch parallelizes this task.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/common/FileUtils.java b65c35b 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java b5e660b 

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


Testing
---

existing regression tests. No change in functionality.


Thanks,

Ashutosh Chauhan



Re: Review Request 47242: HIVE-13726

2016-05-12 Thread Rui Li


> On May 12, 2016, 5:48 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java, line 3074
> > 
> >
> > It could have been. Feels cleaner to me as non-static. Just a matter of 
> > style. Is there any advantage of keeping it static?

Just thought the method is protected and may be accessed in sub-classes. If 
that's not a concern, I'm OK with non-static.


- Rui


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


On May 11, 2016, 4:36 p.m., Ashutosh Chauhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47242/
> ---
> 
> (Updated May 11, 2016, 4:36 p.m.)
> 
> 
> Review request for hive and Rui Li.
> 
> 
> Bugs: HIVE-13726
> https://issues.apache.org/jira/browse/HIVE-13726
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For insert overwrite significant amount might be spent in deleting existing 
> files. This patch parallelizes this task.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java 71c9188 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java dd14124 
> 
> Diff: https://reviews.apache.org/r/47242/diff/
> 
> 
> Testing
> ---
> 
> existing regression tests. No change in functionality.
> 
> 
> Thanks,
> 
> Ashutosh Chauhan
> 
>



Re: Review Request 47242: HIVE-13726

2016-05-11 Thread Ashutosh Chauhan

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




ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java (line 3074)


It could have been. Feels cleaner to me as non-static. Just a matter of 
style. Is there any advantage of keeping it static?



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java (line 3168)


yeah.. can remove that.


- Ashutosh Chauhan


On May 11, 2016, 4:36 p.m., Ashutosh Chauhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47242/
> ---
> 
> (Updated May 11, 2016, 4:36 p.m.)
> 
> 
> Review request for hive and Rui Li.
> 
> 
> Bugs: HIVE-13726
> https://issues.apache.org/jira/browse/HIVE-13726
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For insert overwrite significant amount might be spent in deleting existing 
> files. This patch parallelizes this task.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java 71c9188 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java dd14124 
> 
> Diff: https://reviews.apache.org/r/47242/diff/
> 
> 
> Testing
> ---
> 
> existing regression tests. No change in functionality.
> 
> 
> Thanks,
> 
> Ashutosh Chauhan
> 
>



Re: Review Request 47242: HIVE-13726

2016-05-11 Thread Rui Li

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




ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java (line 3074)


Any reason why remove the static? I think the newly added method can be 
static too.



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java (line 3168)


I think FileNotFoundException is not needed here.


- Rui Li


On May 11, 2016, 4:36 p.m., Ashutosh Chauhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47242/
> ---
> 
> (Updated May 11, 2016, 4:36 p.m.)
> 
> 
> Review request for hive and Rui Li.
> 
> 
> Bugs: HIVE-13726
> https://issues.apache.org/jira/browse/HIVE-13726
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For insert overwrite significant amount might be spent in deleting existing 
> files. This patch parallelizes this task.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java 71c9188 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java dd14124 
> 
> Diff: https://reviews.apache.org/r/47242/diff/
> 
> 
> Testing
> ---
> 
> existing regression tests. No change in functionality.
> 
> 
> Thanks,
> 
> Ashutosh Chauhan
> 
>