hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Szehon Ho" <sze...@cloudera.com>
Subject Re: Review Request 38216: HIVE-11745: Alter table Exchange partition with multiple partition_spec is not working
Date Fri, 11 Sep 2015 04:20:28 GMT


> On Sept. 10, 2015, 6:02 p.m., Szehon Ho wrote:
> > Please make sure that file permission inheritance works for this feature (see HadoopShims.getFullFileStatus
and HadoopShims.setFullFileStatus).  And please add a test to FolderPermissionBase after you
verified it?

Please make sure to add a unit test in this file, to verify permission inheritance works.


> On Sept. 10, 2015, 6:02 p.m., Szehon Ho wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 2552
> > <https://reviews.apache.org/r/38216/diff/1/?file=1065987#file1065987line2552>
> >
> >     I think this whole method can be moved to FileUtils for organization.  Also
please check if there's any method there already.
> 
> Yongzhi Chen wrote:
>     I think it may be better as a private method in the HiveMetaStore class for it will
using its private variable wh (hdfs warehouse) .

Actually looking more into the code, this method should not be necessary.  You can just call
wh.mkdirs directly.  The underlying FileSystem.mkdirs has the same semantics as -p, there
should be no file system that violates this.  If there were, many other partition codes would
break..


> On Sept. 10, 2015, 6:02 p.m., Szehon Ho wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 2548
> > <https://reviews.apache.org/r/38216/diff/1/?file=1065987#file1065987line2548>
> >
> >     Please make sure we follow the normal code standards (properly indent the comment,
make sure there are spaces after "if", and "catch" does not have to be a new line.
> 
> Yongzhi Chen wrote:
>     Fixed the indent of the comment. If and catch statements.

Thanks, please also put parenthesis around all the if-statements for consistency.


- Szehon


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


On Sept. 10, 2015, 7:36 p.m., Yongzhi Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38216/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2015, 7:36 p.m.)
> 
> 
> Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-11745
>     https://issues.apache.org/jira/browse/HIVE-11745
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Alter table Exchange partition with multiple partition_spec does not work in cluster
mode because in rename, the parent folder for destination path does not physically exist.
Some files system(hdfs for instance) does not support(or allow) this. Fix by create parent
folder first.
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties bed621d3eb74f01e54110552f68538afd228018d

>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1840e76cc567e95e1942d912b8ab0db516d63a3b

>   ql/src/test/queries/clientpositive/exchgpartition2lel.q PRE-CREATION 
>   ql/src/test/results/clientpositive/exchgpartition2lel.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38216/diff/
> 
> 
> Testing
> -------
> 
> Add minimr unit test.
> 
> 
> Thanks,
> 
> Yongzhi Chen
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message