You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sergio Pena via Review Board <no...@reviews.apache.org> on 2018/09/07 17:01:11 UTC

Review Request 68669: SENTRY-2395: ALTER VIEW AS SELECT is asking for CREATE privileges instead of ALTER

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

Review request for sentry.


Bugs: SENTRY-2395
    https://issues.apache.org/jira/browse/SENTRY-2395


Repository: sentry


Description
-------

This patch checks in the pre-analyze if the operation will be an ALTER VIEW AS SELECT, and it then switches the CREATEVIEW for ALTERVIEW_AS in the post-analyze.
It also adds the new required privilege for the ALTERVIEW_AS operation.


Diffs
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 0ab636a97cf158cf0a219f1b0f1da35b332441a4 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java 0973e9832b334b83f4acb3013dc35583e5c0173a 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java feb77ad6e925b5df3e06cbadbda260e01e5e94c5 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 42971d846f9344282f434b9cb720c7c74c592135 


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


Testing
-------


Thanks,

Sergio Pena


Re: Review Request 68669: SENTRY-2395: ALTER VIEW AS SELECT is asking for CREATE privileges instead of ALTER

Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.

> On Sept. 7, 2018, 5:51 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
> > Lines 152 (patched)
> > <https://reviews.apache.org/r/68669/diff/1/?file=2086587#file2086587line152>
> >
> >     I don't think this logic is applicable for all the above cases. We should consider limiting this code to cases that are relavent.

Then I will need to repeat code on the TOK_ALTERVIEW case. The isAlterViewAsOperation() checks the tree is correct the same way it does it in this switch. The logic is the same.


> On Sept. 7, 2018, 5:51 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
> > Lines 302 (patched)
> > <https://reviews.apache.org/r/68669/diff/1/?file=2086587#file2086587line302>
> >
> >     I'm consufed. This comment says ALTER VIEW AS SELECT to CREATEVIEW but the API "isAlterViewAsOperation" is not invoked for case where ast.getToken().getType() is equals to HiveParser.TOK_CREATEVIEW.
> >     
> >     Could you clarify?

The isAlterViewAs is set on the preAnalyze where we can check if the tree has such operation. The postAnalyze uses CREATEVIEW which the only way to know it as an ALTERVIEW_AS was by setting the boolean value in the preAnalyze.


- Sergio


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


On Sept. 7, 2018, 5:01 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68669/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2018, 5:01 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2395
>     https://issues.apache.org/jira/browse/SENTRY-2395
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This patch checks in the pre-analyze if the operation will be an ALTER VIEW AS SELECT, and it then switches the CREATEVIEW for ALTERVIEW_AS in the post-analyze.
> It also adds the new required privilege for the ALTERVIEW_AS operation.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 0ab636a97cf158cf0a219f1b0f1da35b332441a4 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java 0973e9832b334b83f4acb3013dc35583e5c0173a 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java feb77ad6e925b5df3e06cbadbda260e01e5e94c5 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 42971d846f9344282f434b9cb720c7c74c592135 
> 
> 
> Diff: https://reviews.apache.org/r/68669/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 68669: SENTRY-2395: ALTER VIEW AS SELECT is asking for CREATE privileges instead of ALTER

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68669/#review208452
-----------------------------------------------------------




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
Lines 152 (patched)
<https://reviews.apache.org/r/68669/#comment292375>

    I don't think this logic is applicable for all the above cases. We should consider limiting this code to cases that are relavent.



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
Lines 302 (patched)
<https://reviews.apache.org/r/68669/#comment292376>

    I'm consufed. This comment says ALTER VIEW AS SELECT to CREATEVIEW but the API "isAlterViewAsOperation" is not invoked for case where ast.getToken().getType() is equals to HiveParser.TOK_CREATEVIEW.
    
    Could you clarify?


- kalyan kumar kalvagadda


On Sept. 7, 2018, 5:01 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68669/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2018, 5:01 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2395
>     https://issues.apache.org/jira/browse/SENTRY-2395
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This patch checks in the pre-analyze if the operation will be an ALTER VIEW AS SELECT, and it then switches the CREATEVIEW for ALTERVIEW_AS in the post-analyze.
> It also adds the new required privilege for the ALTERVIEW_AS operation.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 0ab636a97cf158cf0a219f1b0f1da35b332441a4 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java 0973e9832b334b83f4acb3013dc35583e5c0173a 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java feb77ad6e925b5df3e06cbadbda260e01e5e94c5 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 42971d846f9344282f434b9cb720c7c74c592135 
> 
> 
> Diff: https://reviews.apache.org/r/68669/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 68669: SENTRY-2395: ALTER VIEW AS SELECT is asking for CREATE privileges instead of ALTER

Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68669/#review208458
-----------------------------------------------------------




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
Lines 152 (patched)
<https://reviews.apache.org/r/68669/#comment292382>

    Then I will need to repeat code on the TOK_ALTERVIEW case. The isAlterViewAsOperation() checks the tree is correct the same way it does it in this switch. The logic is the same.



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
Lines 302 (patched)
<https://reviews.apache.org/r/68669/#comment292383>

    The isAlterViewAs is set on the preAnalyze where we can check if the tree has such operation. The postAnalyze uses CREATEVIEW which the only way to know it as an ALTERVIEW_AS was by setting the boolean value in the preAnalyze.


- Sergio Pena


On Sept. 7, 2018, 5:01 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68669/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2018, 5:01 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2395
>     https://issues.apache.org/jira/browse/SENTRY-2395
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This patch checks in the pre-analyze if the operation will be an ALTER VIEW AS SELECT, and it then switches the CREATEVIEW for ALTERVIEW_AS in the post-analyze.
> It also adds the new required privilege for the ALTERVIEW_AS operation.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 0ab636a97cf158cf0a219f1b0f1da35b332441a4 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java 0973e9832b334b83f4acb3013dc35583e5c0173a 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java feb77ad6e925b5df3e06cbadbda260e01e5e94c5 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 42971d846f9344282f434b9cb720c7c74c592135 
> 
> 
> Diff: https://reviews.apache.org/r/68669/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 68669: SENTRY-2395: ALTER VIEW AS SELECT is asking for CREATE privileges instead of ALTER

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68669/#review208497
-----------------------------------------------------------


Ship it!




Ship It!

- kalyan kumar kalvagadda


On Sept. 7, 2018, 5:01 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68669/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2018, 5:01 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2395
>     https://issues.apache.org/jira/browse/SENTRY-2395
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This patch checks in the pre-analyze if the operation will be an ALTER VIEW AS SELECT, and it then switches the CREATEVIEW for ALTERVIEW_AS in the post-analyze.
> It also adds the new required privilege for the ALTERVIEW_AS operation.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 0ab636a97cf158cf0a219f1b0f1da35b332441a4 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java 0973e9832b334b83f4acb3013dc35583e5c0173a 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java feb77ad6e925b5df3e06cbadbda260e01e5e94c5 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 42971d846f9344282f434b9cb720c7c74c592135 
> 
> 
> Diff: https://reviews.apache.org/r/68669/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 68669: SENTRY-2395: ALTER VIEW AS SELECT is asking for CREATE privileges instead of ALTER

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68669/#review208496
-----------------------------------------------------------


Ship it!




Ship It!

- Na Li


On Sept. 7, 2018, 5:01 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68669/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2018, 5:01 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2395
>     https://issues.apache.org/jira/browse/SENTRY-2395
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This patch checks in the pre-analyze if the operation will be an ALTER VIEW AS SELECT, and it then switches the CREATEVIEW for ALTERVIEW_AS in the post-analyze.
> It also adds the new required privilege for the ALTERVIEW_AS operation.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 0ab636a97cf158cf0a219f1b0f1da35b332441a4 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java 0973e9832b334b83f4acb3013dc35583e5c0173a 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java feb77ad6e925b5df3e06cbadbda260e01e5e94c5 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 42971d846f9344282f434b9cb720c7c74c592135 
> 
> 
> Diff: https://reviews.apache.org/r/68669/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>