You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Li Li <li...@cloudera.com> on 2015/12/22 22:47:27 UTC

Review Request 41662: SENTRY-826: TRUNCATE on empty partitioned table in Hive fails

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

Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

The SemanticException happened in HiveAuthzBinding.authorize method, which is caused by the empty outputHierarchy list. As there is no partition in the test tbl, the writeEntity set returned from Hive is empty. That's why the outputHierarchy list is also empty. To resolve it, we should manually add db.tbl in the outputHierarchy which will be validated in the hiveAuthzBinding.authorize method.


Diffs
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 994af8a04414f5093a0d5546744d6da727b69db4 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 56776db7c463e1f27cd39baa9e4c9d9709c53925 

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


Testing
-------

add test case for:
empty partitioned table and NOT empty partitioned table
user with permission and user WITHOUT permission


Thanks,

Li Li


Re: Review Request 41662: SENTRY-826: TRUNCATE on empty partitioned table in Hive fails

Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41662/#review113641
-----------------------------------------------------------

Ship it!


Ship It!

- Lenni Kuff


On Jan. 9, 2016, 3:31 p.m., Li Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41662/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2016, 3:31 p.m.)
> 
> 
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The SemanticException happened in HiveAuthzBinding.authorize method, which is caused by the empty outputHierarchy list. As there is no partition in the test tbl, the writeEntity set returned from Hive is empty. That's why the outputHierarchy list is also empty. To resolve it, we should manually add db.tbl in the outputHierarchy which will be validated in the hiveAuthzBinding.authorize method.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 994af8a04414f5093a0d5546744d6da727b69db4 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 56776db7c463e1f27cd39baa9e4c9d9709c53925 
> 
> Diff: https://reviews.apache.org/r/41662/diff/
> 
> 
> Testing
> -------
> 
> add test case for:
> empty partitioned table and NOT empty partitioned table
> user with permission and user WITHOUT permission
> 
> 
> Thanks,
> 
> Li Li
> 
>


Re: Review Request 41662: SENTRY-826: TRUNCATE on empty partitioned table in Hive fails

Posted by Li Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41662/
-----------------------------------------------------------

(Updated Jan. 9, 2016, 3:31 p.m.)


Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

The SemanticException happened in HiveAuthzBinding.authorize method, which is caused by the empty outputHierarchy list. As there is no partition in the test tbl, the writeEntity set returned from Hive is empty. That's why the outputHierarchy list is also empty. To resolve it, we should manually add db.tbl in the outputHierarchy which will be validated in the hiveAuthzBinding.authorize method.


Diffs (updated)
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 994af8a04414f5093a0d5546744d6da727b69db4 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 56776db7c463e1f27cd39baa9e4c9d9709c53925 

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


Testing
-------

add test case for:
empty partitioned table and NOT empty partitioned table
user with permission and user WITHOUT permission


Thanks,

Li Li


Re: Review Request 41662: SENTRY-826: TRUNCATE on empty partitioned table in Hive fails

Posted by Li Li <li...@cloudera.com>.

> On Jan. 7, 2016, 8:29 a.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java, line 520
> > <https://reviews.apache.org/r/41662/diff/2/?file=1181542#file1181542line520>
> >
> >     Add a test case for truncating an unpartitioned table if one does not already exist.

the test case for truncating an unpartitioned table already exists: testTruncateTable


> On Jan. 7, 2016, 8:29 a.m., Lenni Kuff wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 276
> > <https://reviews.apache.org/r/41662/diff/2/?file=1181541#file1181541line276>
> >
> >     Can you add a precondition check to ensure we have the expected number of children? Will we have this number of children for unpartitioned tables as well?

Sure. Yes.


- Li


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


On Jan. 5, 2016, 4:52 a.m., Li Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41662/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 4:52 a.m.)
> 
> 
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The SemanticException happened in HiveAuthzBinding.authorize method, which is caused by the empty outputHierarchy list. As there is no partition in the test tbl, the writeEntity set returned from Hive is empty. That's why the outputHierarchy list is also empty. To resolve it, we should manually add db.tbl in the outputHierarchy which will be validated in the hiveAuthzBinding.authorize method.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 994af8a04414f5093a0d5546744d6da727b69db4 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 56776db7c463e1f27cd39baa9e4c9d9709c53925 
> 
> Diff: https://reviews.apache.org/r/41662/diff/
> 
> 
> Testing
> -------
> 
> add test case for:
> empty partitioned table and NOT empty partitioned table
> user with permission and user WITHOUT permission
> 
> 
> Thanks,
> 
> Li Li
> 
>


Re: Review Request 41662: SENTRY-826: TRUNCATE on empty partitioned table in Hive fails

Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41662/#review113219
-----------------------------------------------------------



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 276)
<https://reviews.apache.org/r/41662/#comment173718>

    Can you add a precondition check to ensure we have the expected number of children? Will we have this number of children for unpartitioned tables as well?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java (line 510)
<https://reviews.apache.org/r/41662/#comment173719>

    Add a test case for truncating an unpartitioned table if one does not already exist.



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java (line 548)
<https://reviews.apache.org/r/41662/#comment173720>

    Also test "TRUNCATE TABLE tbl PARTITION(partitionSpec)"



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java (line 591)
<https://reviews.apache.org/r/41662/#comment173721>

    This test case seems very similar to the one above, expect that there is a partition added. Can you reduce the code duplicate or move some of the logic into shared helper methods?


- Lenni Kuff


On Jan. 5, 2016, 4:52 a.m., Li Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41662/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 4:52 a.m.)
> 
> 
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The SemanticException happened in HiveAuthzBinding.authorize method, which is caused by the empty outputHierarchy list. As there is no partition in the test tbl, the writeEntity set returned from Hive is empty. That's why the outputHierarchy list is also empty. To resolve it, we should manually add db.tbl in the outputHierarchy which will be validated in the hiveAuthzBinding.authorize method.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 994af8a04414f5093a0d5546744d6da727b69db4 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 56776db7c463e1f27cd39baa9e4c9d9709c53925 
> 
> Diff: https://reviews.apache.org/r/41662/diff/
> 
> 
> Testing
> -------
> 
> add test case for:
> empty partitioned table and NOT empty partitioned table
> user with permission and user WITHOUT permission
> 
> 
> Thanks,
> 
> Li Li
> 
>


Re: Review Request 41662: SENTRY-826: TRUNCATE on empty partitioned table in Hive fails

Posted by Li Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41662/
-----------------------------------------------------------

(Updated Jan. 5, 2016, 4:52 a.m.)


Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

The SemanticException happened in HiveAuthzBinding.authorize method, which is caused by the empty outputHierarchy list. As there is no partition in the test tbl, the writeEntity set returned from Hive is empty. That's why the outputHierarchy list is also empty. To resolve it, we should manually add db.tbl in the outputHierarchy which will be validated in the hiveAuthzBinding.authorize method.


Diffs (updated)
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 994af8a04414f5093a0d5546744d6da727b69db4 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 56776db7c463e1f27cd39baa9e4c9d9709c53925 

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


Testing
-------

add test case for:
empty partitioned table and NOT empty partitioned table
user with permission and user WITHOUT permission


Thanks,

Li Li


Re: Review Request 41662: SENTRY-826: TRUNCATE on empty partitioned table in Hive fails

Posted by Li Li <li...@cloudera.com>.

> On Jan. 3, 2016, 8:48 p.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 272
> > <https://reviews.apache.org/r/41662/diff/1/?file=1174771#file1174771line272>
> >
> >     Why not use currOutTab here, so logic in line 518 can be used and no extra logic need to be introduced in line 592?

Good Point!


- Li


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


On Dec. 22, 2015, 9:47 p.m., Li Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41662/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2015, 9:47 p.m.)
> 
> 
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The SemanticException happened in HiveAuthzBinding.authorize method, which is caused by the empty outputHierarchy list. As there is no partition in the test tbl, the writeEntity set returned from Hive is empty. That's why the outputHierarchy list is also empty. To resolve it, we should manually add db.tbl in the outputHierarchy which will be validated in the hiveAuthzBinding.authorize method.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 994af8a04414f5093a0d5546744d6da727b69db4 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 56776db7c463e1f27cd39baa9e4c9d9709c53925 
> 
> Diff: https://reviews.apache.org/r/41662/diff/
> 
> 
> Testing
> -------
> 
> add test case for:
> empty partitioned table and NOT empty partitioned table
> user with permission and user WITHOUT permission
> 
> 
> Thanks,
> 
> Li Li
> 
>


Re: Review Request 41662: SENTRY-826: TRUNCATE on empty partitioned table in Hive fails

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41662/#review112476
-----------------------------------------------------------



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 272)
<https://reviews.apache.org/r/41662/#comment172924>

    Why not use currOutTab here, so logic in line 518 can be used and no extra logic need to be introduced in line 592?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java (line 529)
<https://reviews.apache.org/r/41662/#comment172926>

    Change it to "verify all privileges on tab" to be more clear? Same as the following comments.



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java (line 630)
<https://reviews.apache.org/r/41662/#comment172925>

    Make it another line after if statement.


- Hao Hao


On Dec. 22, 2015, 9:47 p.m., Li Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41662/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2015, 9:47 p.m.)
> 
> 
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The SemanticException happened in HiveAuthzBinding.authorize method, which is caused by the empty outputHierarchy list. As there is no partition in the test tbl, the writeEntity set returned from Hive is empty. That's why the outputHierarchy list is also empty. To resolve it, we should manually add db.tbl in the outputHierarchy which will be validated in the hiveAuthzBinding.authorize method.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 994af8a04414f5093a0d5546744d6da727b69db4 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 56776db7c463e1f27cd39baa9e4c9d9709c53925 
> 
> Diff: https://reviews.apache.org/r/41662/diff/
> 
> 
> Testing
> -------
> 
> add test case for:
> empty partitioned table and NOT empty partitioned table
> user with permission and user WITHOUT permission
> 
> 
> Thanks,
> 
> Li Li
> 
>