Re: Review Request 50525: HIVE-14341: Altered skewed location is not respected for list bucketing

2016-09-20 Thread Aihua Xu

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




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


Actually the original one is also not overwrite but move from a temp file 
to the final location. While that would fail if an previous file already 
exists. Then we should try to rename to '_copy_1' and so.



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


Yes. That's where this issue comes from. Without this change, even if you 
alter the skew location which will be saved in locationMap, it would get the 
old location and save it to overwrite the newer location. That is not correct.

The new logic is: if we have the mapped location for the skew keys, then 
use it. Otherwise, get the default skew location.


- Aihua Xu


On Sept. 19, 2016, 9:02 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50525/
> ---
> 
> (Updated Sept. 19, 2016, 9:02 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14341: Altered skewed location is not respected for list bucketing
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java e386717 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java da46854 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
>  ba4f6a7 
>   ql/src/test/queries/clientpositive/create_alter_list_bucketing_table1.q 
> bf89e8f 
>   ql/src/test/results/clientpositive/create_alter_list_bucketing_table1.q.out 
> 216d3be 
> 
> Diff: https://reviews.apache.org/r/50525/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>



Re: Review Request 50525: HIVE-14341: Altered skewed location is not respected for list bucketing

2016-09-20 Thread Yongzhi Chen

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




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


Any reason you change the logic from replace(overwrite) to something 
like(insert into)?


- Yongzhi Chen


On Sept. 19, 2016, 9:02 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50525/
> ---
> 
> (Updated Sept. 19, 2016, 9:02 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14341: Altered skewed location is not respected for list bucketing
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java e386717 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java da46854 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
>  ba4f6a7 
>   ql/src/test/queries/clientpositive/create_alter_list_bucketing_table1.q 
> bf89e8f 
>   ql/src/test/results/clientpositive/create_alter_list_bucketing_table1.q.out 
> 216d3be 
> 
> Diff: https://reviews.apache.org/r/50525/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>



Re: Review Request 50525: HIVE-14341: Altered skewed location is not respected for list bucketing

2016-09-20 Thread Yongzhi Chen

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




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


You change old logic here a little bit in following case:
When locationMap has skewedValsCandidate, but
allSkewedVals.contains(skewedValsCandidate) == false

Before your change, it uses defaultKey in locationMap while after the 
change, skewedValsCandidate is used. 
Is that safe?


- Yongzhi Chen


On Sept. 19, 2016, 9:02 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50525/
> ---
> 
> (Updated Sept. 19, 2016, 9:02 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14341: Altered skewed location is not respected for list bucketing
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java e386717 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java da46854 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
>  ba4f6a7 
>   ql/src/test/queries/clientpositive/create_alter_list_bucketing_table1.q 
> bf89e8f 
>   ql/src/test/results/clientpositive/create_alter_list_bucketing_table1.q.out 
> 216d3be 
> 
> Diff: https://reviews.apache.org/r/50525/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>



Re: Review Request 50525: HIVE-14341: Altered skewed location is not respected for list bucketing

2016-09-19 Thread Aihua Xu

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

(Updated Sept. 19, 2016, 9:02 p.m.)


Review request for hive.


Changes
---

Made the changes so the desc command will show skewed location for those 
locations not updated explicitly.
With this patch, we will not automatically collect the skew mapping from the 
directory since that would cause the issue if the location is updated 
explicitly.
Rather, given a query like select * from list_bucket_single where key=1, if the 
skew location for key 1 is updated explicitly, then we will have the new 
location from HMS, otherwise, we will check the default location 
/list_bucket_single/key=1.


Repository: hive-git


Description
---

HIVE-14341: Altered skewed location is not respected for list bucketing


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java e386717 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java da46854 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
 ba4f6a7 
  ql/src/test/queries/clientpositive/create_alter_list_bucketing_table1.q 
bf89e8f 
  ql/src/test/results/clientpositive/create_alter_list_bucketing_table1.q.out 
216d3be 

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


Testing
---


Thanks,

Aihua Xu