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

[kalyan kumar kalvagadda via Review Board]

---
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

[Na Li via Review Board]

---
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
> 
>

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

[Sergio Pena via Review Board]

> 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)
> > 
> >
> > 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)
> > 
> >
> > 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
> 
>