Re: Review Request 58686: HIVE-16147: Rename a partitioned table should not drop its partition columns stats

2017-04-28 Thread Chaoyu Tang 


> On April 28, 2017, 6:27 p.m., pengcheng xiong wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
> > Lines 7386 (patched)
> > 
> >
> > The other parts look good to me. Could u explain with an example why we 
> > swallow the exception here? thanks.

The colNames in validateTableCols(table, colNames) are the partition columns. 
Under some circumstances, the partition columns might be different from those 
defined for its parent table. It is because Hive allows to change partition 
columns directly without changing its table's columns by using DDL like alter 
table ... partition (...) change colName1 colName2 colType. For this case, the 
validateTableCols will fail when we try to get partition column stats by 
calling ObjectStore.getMPartitionColumnStatistics. However, the discrepancies 
in cloumns between table and its partition should not be common, therefore I 
raise the warning here.
We invalidated (delete) partition columns stats and did not need to call 
getMPartitionColumnStatistics when we change a partition column, therefore we 
did not run into this issue.

I think it is not right to validate partition columns against its table. We 
should validate them against the partition instead. However it is a separate 
issue. The original author particularly put the comment "// We are not going to 
verify SD for each partition. Just verify for the table." in the code. I need 
contact him for the reasons and to see if it is necessary to file a separate 
JIRA for this issue.


- Chaoyu


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


On April 28, 2017, 6:19 p.m., Chaoyu Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58686/
> ---
> 
> (Updated April 28, 2017, 6:19 p.m.)
> 
> 
> Review request for hive and pengcheng xiong.
> 
> 
> Bugs: HIVE-16147
> https://issues.apache.org/jira/browse/HIVE-16147
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The patch is to
> 1. preserve the column stats after a partitioned table is renamed
> 2. rename the alter_table_invalidate_column_stats.q to 
> alter_table_column_stats.q
> 
> 
> Diffs
> -
> 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
> 77b3541 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 1b701e0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 6b21751 
>   ql/src/test/queries/clientpositive/alter_table_column_stats.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/alter_table_invalidate_column_stats.q 
> a478451 
>   ql/src/test/results/clientpositive/alter_table_column_stats.q.out 
> PRE-CREATION 
>   
> ql/src/test/results/clientpositive/alter_table_invalidate_column_stats.q.out 
> 85d7dc4 
> 
> 
> Diff: https://reviews.apache.org/r/58686/diff/2/
> 
> 
> Testing
> ---
> 
> manual tests
> qtests
> 
> 
> Thanks,
> 
> Chaoyu Tang
> 
>

Re: Review Request 58686: HIVE-16147: Rename a partitioned table should not drop its partition columns stats

2017-04-28 Thread pengcheng xiong 

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




metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 7386 (patched)


The other parts look good to me. Could u explain with an example why we 
swallow the exception here? thanks.


- pengcheng xiong


On April 28, 2017, 6:19 p.m., Chaoyu Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58686/
> ---
> 
> (Updated April 28, 2017, 6:19 p.m.)
> 
> 
> Review request for hive and pengcheng xiong.
> 
> 
> Bugs: HIVE-16147
> https://issues.apache.org/jira/browse/HIVE-16147
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The patch is to
> 1. preserve the column stats after a partitioned table is renamed
> 2. rename the alter_table_invalidate_column_stats.q to 
> alter_table_column_stats.q
> 
> 
> Diffs
> -
> 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
> 77b3541 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 1b701e0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 6b21751 
>   ql/src/test/queries/clientpositive/alter_table_column_stats.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/alter_table_invalidate_column_stats.q 
> a478451 
>   ql/src/test/results/clientpositive/alter_table_column_stats.q.out 
> PRE-CREATION 
>   
> ql/src/test/results/clientpositive/alter_table_invalidate_column_stats.q.out 
> 85d7dc4 
> 
> 
> Diff: https://reviews.apache.org/r/58686/diff/2/
> 
> 
> Testing
> ---
> 
> manual tests
> qtests
> 
> 
> Thanks,
> 
> Chaoyu Tang
> 
>

Re: Review Request 58686: HIVE-16147: Rename a partitioned table should not drop its partition columns stats

2017-04-28 Thread Chaoyu Tang 

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

(Updated April 28, 2017, 6:19 p.m.)


Review request for hive and pengcheng xiong.


Changes
---

Furhter changes to fix the test failures:
1. partititon view does not have SD, so we need check if SD is null or not 
before further calling its getCols()
2. ObjectStore.getMPartitionColumnStatistics validated the partition columns 
against Table which I thought is not correct. Partition might have a different 
column definition from Table. In this case, we should raise some warning 
instead of throwing out the exception.


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


Repository: hive-git


Description
---

The patch is to
1. preserve the column stats after a partitioned table is renamed
2. rename the alter_table_invalidate_column_stats.q to 
alter_table_column_stats.q


Diffs (updated)
-

  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
77b3541 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
1b701e0 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b21751 
  ql/src/test/queries/clientpositive/alter_table_column_stats.q PRE-CREATION 
  ql/src/test/queries/clientpositive/alter_table_invalidate_column_stats.q 
a478451 
  ql/src/test/results/clientpositive/alter_table_column_stats.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/alter_table_invalidate_column_stats.q.out 
85d7dc4 


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

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


Testing
---

manual tests
qtests


Thanks,

Chaoyu Tang

Re: Review Request 58686: HIVE-16147: Rename a partitioned table should not drop its partition columns stats

2017-04-24 Thread pengcheng xiong 

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


Ship it!




Ship It!

- pengcheng xiong


On April 24, 2017, 10:29 p.m., Chaoyu Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58686/
> ---
> 
> (Updated April 24, 2017, 10:29 p.m.)
> 
> 
> Review request for hive and pengcheng xiong.
> 
> 
> Bugs: HIVE-16147
> https://issues.apache.org/jira/browse/HIVE-16147
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The patch is to
> 1. preserve the column stats after a partitioned table is renamed
> 2. rename the alter_table_invalidate_column_stats.q to 
> alter_table_column_stats.q
> 
> 
> Diffs
> -
> 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
> 77b3541 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 1b701e0 
>   ql/src/test/queries/clientpositive/alter_table_column_stats.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/alter_table_invalidate_column_stats.q 
> a478451 
>   ql/src/test/results/clientpositive/alter_table_column_stats.q.out 
> PRE-CREATION 
>   
> ql/src/test/results/clientpositive/alter_table_invalidate_column_stats.q.out 
> 85d7dc4 
> 
> 
> Diff: https://reviews.apache.org/r/58686/diff/1/
> 
> 
> Testing
> ---
> 
> manual tests
> qtests
> 
> 
> Thanks,
> 
> Chaoyu Tang
> 
>

Review Request 58686: HIVE-16147: Rename a partitioned table should not drop its partition columns stats

2017-04-24 Thread Chaoyu Tang 

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

Review request for hive and pengcheng xiong.


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


Repository: hive-git


Description
---

The patch is to
1. preserve the column stats after a partitioned table is renamed
2. rename the alter_table_invalidate_column_stats.q to 
alter_table_column_stats.q


Diffs
-

  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
77b3541 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
1b701e0 
  ql/src/test/queries/clientpositive/alter_table_column_stats.q PRE-CREATION 
  ql/src/test/queries/clientpositive/alter_table_invalidate_column_stats.q 
a478451 
  ql/src/test/results/clientpositive/alter_table_column_stats.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/alter_table_invalidate_column_stats.q.out 
85d7dc4 


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


Testing
---

manual tests
qtests


Thanks,

Chaoyu Tang