You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Devaraj Das <dd...@apache.org> on 2011/04/13 00:12:32 UTC
Review Request: HIVE-1943
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/586/
-----------------------------------------------------------
Review request for hive.
Summary
-------
This is a patch to handle the issue described in HIVE-1943
This addresses bug HIVE-1943.
https://issues.apache.org/jira/browse/HIVE-1943
Diffs
-----
http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1090038
http://svn.apache.org/repos/asf/hive/trunk/conf/hive-default.xml 1090038
http://svn.apache.org/repos/asf/hive/trunk/metastore/build.xml 1090038
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1090038
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 1090038
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 1090038
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java PRE-CREATION
http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 1090038
http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 1090038
http://svn.apache.org/repos/asf/hive/trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java 1090038
http://svn.apache.org/repos/asf/hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1090038
Diff: https://reviews.apache.org/r/586/diff
Testing
-------
Unit tests added.
Thanks,
Devaraj
Re: Review Request: HIVE-1943
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/586/#review459
-----------------------------------------------------------
Ship it!
+1
Please upload the patch to jira.
- Amareshwari
On 2011-04-14 00:57:35, Devaraj Das wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/586/
> -----------------------------------------------------------
>
> (Updated 2011-04-14 00:57:35)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> This is a patch to handle the issue described in HIVE-1943
>
>
> This addresses bug HIVE-1943.
> https://issues.apache.org/jira/browse/HIVE-1943
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1091578
> http://svn.apache.org/repos/asf/hive/trunk/conf/hive-default.xml 1091578
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091578
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 1091578
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 1091578
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java PRE-CREATION
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 1091578
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 1091578
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java 1091578
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1091578
>
> Diff: https://reviews.apache.org/r/586/diff
>
>
> Testing
> -------
>
> Unit tests added.
>
>
> Thanks,
>
> Devaraj
>
>
Re: Review Request: HIVE-1943
Posted by Devaraj Das <dd...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/586/
-----------------------------------------------------------
(Updated 2011-04-14 00:57:35.688239)
Review request for hive.
Changes
-------
The new patch addresses Amareshwari's comments. The testcase is modified to do checks for write permissions for everyone in this patch (that will avoid the confusion that was created in the earlier patch where i was checking the owner write permissions only).
Summary
-------
This is a patch to handle the issue described in HIVE-1943
This addresses bug HIVE-1943.
https://issues.apache.org/jira/browse/HIVE-1943
Diffs (updated)
-----
http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1091578
http://svn.apache.org/repos/asf/hive/trunk/conf/hive-default.xml 1091578
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091578
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 1091578
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 1091578
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java PRE-CREATION
http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 1091578
http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 1091578
http://svn.apache.org/repos/asf/hive/trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java 1091578
http://svn.apache.org/repos/asf/hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1091578
Diff: https://reviews.apache.org/r/586/diff
Testing
-------
Unit tests added.
Thanks,
Devaraj
Re: Review Request: HIVE-1943
Posted by Devaraj Das <dd...@apache.org>.
> On 2011-04-13 06:21:06, Amareshwari Sriramadasu wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/build.xml, lines 25-46
> > <https://reviews.apache.org/r/586/diff/1/?file=15648#file15648line25>
> >
> > Did not understand why this change is required?
Right. This is not needed (it was needed in an earlier version of the TestMetaStoreAuthorization).
> On 2011-04-13 06:21:06, Amareshwari Sriramadasu wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 649
> > <https://reviews.apache.org/r/586/diff/1/?file=15649#file15649line649>
> >
> > change Table metadata to databse metadata?
Copy-paste problem :(
> On 2011-04-13 06:21:06, Amareshwari Sriramadasu wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 1399
> > <https://reviews.apache.org/r/586/diff/1/?file=15649#file15649line1399>
> >
> > change Table metadata to partition metadata?
Copy-paste problem :(
> On 2011-04-13 06:21:06, Amareshwari Sriramadasu wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 1411
> > <https://reviews.apache.org/r/586/diff/1/?file=15649#file15649line1411>
> >
> > change Table metadata to partition metadata?
Copy-paste problem :(
> On 2011-04-13 06:21:06, Amareshwari Sriramadasu wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java, lines 63-65
> > <https://reviews.apache.org/r/586/diff/1/?file=15652#file15652line63>
> >
> > Can this be changed to assert statement instead of throwing exception?
Okay, will make it an assert.
> On 2011-04-13 06:21:06, Amareshwari Sriramadasu wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java, lines 67-69
> > <https://reviews.apache.org/r/586/diff/1/?file=15652#file15652line67>
> >
> > Can this be changed to assert statement instead of throwing exception?
Okay, will make it an assert.
> On 2011-04-13 06:21:06, Amareshwari Sriramadasu wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java, line 73
> > <https://reviews.apache.org/r/586/diff/1/?file=15652#file15652line73>
> >
> > Can we add the test for group writable also?
That's a bit tricky. The reason being that the test might give out false positives, since the isWritable call doesn't take any argument to signify for which entity (owner or group) we are doing the check. For example, isWritable(foo) will return true for both 570 and 750 permissions on foo. We really can't say whether the test passed because the check for the owner part returned true, or, the check for the group part returned true. If i break up the method isWritable to three methods that do checks on owner, group, and, others, respectively, i can do the tests for all three, but have a feeling that that will be an overkill.. Thoughts?
- Devaraj
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/586/#review437
-----------------------------------------------------------
On 2011-04-12 22:12:32, Devaraj Das wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/586/
> -----------------------------------------------------------
>
> (Updated 2011-04-12 22:12:32)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> This is a patch to handle the issue described in HIVE-1943
>
>
> This addresses bug HIVE-1943.
> https://issues.apache.org/jira/browse/HIVE-1943
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/conf/hive-default.xml 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/build.xml 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java PRE-CREATION
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1090038
>
> Diff: https://reviews.apache.org/r/586/diff
>
>
> Testing
> -------
>
> Unit tests added.
>
>
> Thanks,
>
> Devaraj
>
>
Re: Review Request: HIVE-1943
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/586/#review437
-----------------------------------------------------------
http://svn.apache.org/repos/asf/hive/trunk/metastore/build.xml
<https://reviews.apache.org/r/586/#comment851>
Did not understand why this change is required?
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/586/#comment850>
change Table metadata to databse metadata?
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/586/#comment849>
change Table metadata to partition metadata?
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/586/#comment848>
change Table metadata to partition metadata?
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java
<https://reviews.apache.org/r/586/#comment845>
Can this be changed to assert statement instead of throwing exception?
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java
<https://reviews.apache.org/r/586/#comment846>
Can this be changed to assert statement instead of throwing exception?
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java
<https://reviews.apache.org/r/586/#comment847>
Can we add the test for group writable also?
- Amareshwari
On 2011-04-12 22:12:32, Devaraj Das wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/586/
> -----------------------------------------------------------
>
> (Updated 2011-04-12 22:12:32)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> This is a patch to handle the issue described in HIVE-1943
>
>
> This addresses bug HIVE-1943.
> https://issues.apache.org/jira/browse/HIVE-1943
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/conf/hive-default.xml 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/build.xml 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java PRE-CREATION
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java 1090038
> http://svn.apache.org/repos/asf/hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1090038
>
> Diff: https://reviews.apache.org/r/586/diff
>
>
> Testing
> -------
>
> Unit tests added.
>
>
> Thanks,
>
> Devaraj
>
>