You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by shen guoquan <gu...@intel.com> on 2015/08/26 11:35:52 UTC
Review Request 37794: SENTRY-847: [column level privilege] if grant
column level privilege to user,
show columns in table shouldn't require extra table level privilege
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/
-----------------------------------------------------------
Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
grant select(s) on table test_tb to role test_role;
show columns in test_tb;
Error: Error while compiling statement: FAILED: SemanticException No valid privileges
Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
It should show s column
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
Diff: https://reviews.apache.org/r/37794/diff/
Testing
-------
Run local unit case
Thanks,
shen guoquan
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
> On 八月 27, 2015, 5:14 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 758
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053060#file1053060line758>
> >
> > how about just use columnName here?
OK, I will change it. Thanks
> On 八月 27, 2015, 5:14 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 123
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053059#file1053059line123>
> >
> > add a space after // ?
I will change it. thanks
> On 八月 27, 2015, 5:14 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 763
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053060#file1053060line763>
> >
> > remove ; ?
Because Colunmn level privilege will not any WriteEntity. So it doesn't need to check. I have removed it. Thanks
> On 八月 27, 2015, 5:14 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 53
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053059#file1053059line53>
> >
> > How about removing the default constructor?
I will change it. Thanks
> On 八月 27, 2015, 5:14 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 95
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053059#file1053059line95>
> >
> > This is also same with the parent one and can be removed. Please correct me if I am wrong.
I have change this code. Thanks for your comment.
> On 八月 27, 2015, 5:14 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 51
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053059#file1053059line51>
> >
> > stmtOperation maynot be initialized when we call getStmtOperation() (eg. in line 129). How about setting it to null in construcor?
I will move the setting in the construct function. Thanks
> On 八月 27, 2015, 5:14 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 56
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053059#file1053059line56>
> >
> > Since setHiveAuthzBinding and setSubject should only be called once, how about moving them in the constructor method, and remove setHiveAuthzBinding and setSubject methods?
> > Besides, it maybe better to verify hiveAuthzBing and subject are not null. eg. If subject is null, it will throw runtime exception in line 129: getSubject().getName().
Good advice. I will change it. Thanks
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96637
-----------------------------------------------------------
On 八月 27, 2015, 5:15 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated 八月 27, 2015, 5:15 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by Li Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96637
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 51)
<https://reviews.apache.org/r/37794/#comment152236>
stmtOperation maynot be initialized when we call getStmtOperation() (eg. in line 129). How about setting it to null in construcor?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 53)
<https://reviews.apache.org/r/37794/#comment152230>
How about removing the default constructor?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 56)
<https://reviews.apache.org/r/37794/#comment152233>
Since setHiveAuthzBinding and setSubject should only be called once, how about moving them in the constructor method, and remove setHiveAuthzBinding and setSubject methods?
Besides, it maybe better to verify hiveAuthzBing and subject are not null. eg. If subject is null, it will throw runtime exception in line 129: getSubject().getName().
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 95)
<https://reviews.apache.org/r/37794/#comment152244>
This is also same with the parent one and can be removed. Please correct me if I am wrong.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 123)
<https://reviews.apache.org/r/37794/#comment152232>
add a space after // ?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 758)
<https://reviews.apache.org/r/37794/#comment152245>
how about just use columnName here?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 763)
<https://reviews.apache.org/r/37794/#comment152231>
remove ; ?
- Li Li
On Aug. 26, 2015, 9:35 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2015, 9:35 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
> On 八月 26, 2015, 5:13 p.m., Sravya Tirukkovalur wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 122
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053059#file1053059line122>
> >
> > Nit: typo
I am sorry for the spell error. I will fix it
> On 八月 26, 2015, 5:13 p.m., Sravya Tirukkovalur wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 734
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053060#file1053060line734>
> >
> > Use ColumnMetaDataPrivilege from HiveAuthzPrivilegesMap?
Good advice. I will change it.
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96546
-----------------------------------------------------------
On 八月 26, 2015, 9:35 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated 八月 26, 2015, 9:35 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96546
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 122)
<https://reviews.apache.org/r/37794/#comment152012>
Nit: typo
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 734)
<https://reviews.apache.org/r/37794/#comment152010>
Use ColumnMetaDataPrivilege from HiveAuthzPrivilegesMap?
- Sravya Tirukkovalur
On Aug. 26, 2015, 9:35 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2015, 9:35 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
> On 八月 27, 2015, 5:56 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, lines 144-145
> > <https://reviews.apache.org/r/37794/diff/3/?file=1055769#file1055769line144>
> >
> > use O(n^2) to filter the columns is not effient, Maybe we can just pass List<FieldSchema> to HiveAuthzBindingHook.filterShowColumns.
> > Also, we can use an iterator of the list to remove the one which user does not have privileges, so that do not need to create and copy list many times.
Good advice. I will change it. Thanks
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96652
-----------------------------------------------------------
On 八月 27, 2015, 5:37 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated 八月 27, 2015, 5:37 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by Li Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96652
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (lines 144 - 145)
<https://reviews.apache.org/r/37794/#comment152269>
use O(n^2) to filter the columns is not effient, Maybe we can just pass List<FieldSchema> to HiveAuthzBindingHook.filterShowColumns.
Also, we can use an iterator of the list to remove the one which user does not have privileges, so that do not need to create and copy list many times.
- Li Li
On Aug. 27, 2015, 5:37 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2015, 5:37 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96843
-----------------------------------------------------------
Ship it!
Ship It!
- Colin Ma
On Aug. 27, 2015, 8:21 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2015, 8:21 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java 526a0e0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by Li Li <li...@cloudera.com>.
> On Aug. 28, 2015, 1:47 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 79
> > <https://reviews.apache.org/r/37794/diff/4/?file=1055835#file1055835line79>
> >
> > As showColumns is public method in its parent class, it may have potential security problem when client directly call SentryFilterDDLTask.showColumns(..). Maybe it is better to just override showColumns like the one used in your first version?
>
> shen guoquan wrote:
> The reason why I change this source code is I found all of show function in the DDLTask is private except showColumn. I don't want to override the only showColumn function. I will do some next jiras about show metadata such as "describe table"
> and "show tables". I want to keep code consistency. So I change it. Thanks for your comment.
I am just thinking about if there is potential security situation when client bypass execute method and directly call showColumns, then it can get all columns even if it does not have the permission.
- Li
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96815
-----------------------------------------------------------
On Aug. 27, 2015, 8:21 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2015, 8:21 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java 526a0e0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
> On 八月 28, 2015, 1:47 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 107
> > <https://reviews.apache.org/r/37794/diff/4/?file=1055835#file1055835line107>
> >
> > Shall we use the same way in DDLTask.showColumns?
> > if (dbName == null) {
> > table = db.getTable(tableName);
> > } else {
> > table = db.getTable(dbName, tableName);
> > }
>
> shen guoquan wrote:
> I am sorry. I can't found this part code:
> if (dbName == null) {
> table = db.getTable(tableName);
> } else {
> table = db.getTable(dbName, tableName);
> }
Currently Sentry depends on the Opensource Hive-1.1.0 version. I see the code in this hive version. I can't see your mentioned code.
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96815
-----------------------------------------------------------
On 八月 27, 2015, 8:21 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated 八月 27, 2015, 8:21 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java 526a0e0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
> On 八月 28, 2015, 1:47 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 79
> > <https://reviews.apache.org/r/37794/diff/4/?file=1055835#file1055835line79>
> >
> > As showColumns is public method in its parent class, it may have potential security problem when client directly call SentryFilterDDLTask.showColumns(..). Maybe it is better to just override showColumns like the one used in your first version?
The reason why I change this source code is I found all of show function in the DDLTask is private except showColumn. I don't want to override the only showColumn function. I will do some next jiras about show metadata such as "describe table"
and "show tables". I want to keep code consistency. So I change it. Thanks for your comment.
> On 八月 28, 2015, 1:47 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 107
> > <https://reviews.apache.org/r/37794/diff/4/?file=1055835#file1055835line107>
> >
> > Shall we use the same way in DDLTask.showColumns?
> > if (dbName == null) {
> > table = db.getTable(tableName);
> > } else {
> > table = db.getTable(dbName, tableName);
> > }
I am sorry. I can't found this part code:
if (dbName == null) {
table = db.getTable(tableName);
} else {
table = db.getTable(dbName, tableName);
}
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96815
-----------------------------------------------------------
On 八月 27, 2015, 8:21 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated 八月 27, 2015, 8:21 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java 526a0e0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by Li Li <li...@cloudera.com>.
> On Aug. 28, 2015, 1:47 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 107
> > <https://reviews.apache.org/r/37794/diff/4/?file=1055835#file1055835line107>
> >
> > Shall we use the same way in DDLTask.showColumns?
> > if (dbName == null) {
> > table = db.getTable(tableName);
> > } else {
> > table = db.getTable(dbName, tableName);
> > }
>
> shen guoquan wrote:
> I am sorry. I can't found this part code:
> if (dbName == null) {
> table = db.getTable(tableName);
> } else {
> table = db.getTable(dbName, tableName);
> }
>
> shen guoquan wrote:
> Currently Sentry depends on the Opensource Hive-1.1.0 version. I see the code in this hive version. I can't see your mentioned code.
I see, thanks for your clarification!
- Li
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96815
-----------------------------------------------------------
On Aug. 27, 2015, 8:21 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2015, 8:21 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java 526a0e0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
> On 八月 28, 2015, 1:47 a.m., Li Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java, line 79
> > <https://reviews.apache.org/r/37794/diff/4/?file=1055835#file1055835line79>
> >
> > As showColumns is public method in its parent class, it may have potential security problem when client directly call SentryFilterDDLTask.showColumns(..). Maybe it is better to just override showColumns like the one used in your first version?
>
> shen guoquan wrote:
> The reason why I change this source code is I found all of show function in the DDLTask is private except showColumn. I don't want to override the only showColumn function. I will do some next jiras about show metadata such as "describe table"
> and "show tables". I want to keep code consistency. So I change it. Thanks for your comment.
>
> Li Li wrote:
> I am just thinking about if there is potential security situation when client bypass execute method and directly call showColumns, then it can get all columns even if it does not have the permission.
As far as I know, Hive always execute the Task.execute method to do some operation, such as get metadatas or run mapreduce jobs. There is no chance to bypass execute method. Thanks for your comments.
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96815
-----------------------------------------------------------
On 八月 27, 2015, 8:21 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated 八月 27, 2015, 8:21 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java 526a0e0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by Li Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96815
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 79)
<https://reviews.apache.org/r/37794/#comment152481>
As showColumns is public method in its parent class, it may have potential security problem when client directly call SentryFilterDDLTask.showColumns(..). Maybe it is better to just override showColumns like the one used in your first version?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 107)
<https://reviews.apache.org/r/37794/#comment152479>
Shall we use the same way in DDLTask.showColumns?
if (dbName == null) {
table = db.getTable(tableName);
} else {
table = db.getTable(dbName, tableName);
}
- Li Li
On Aug. 27, 2015, 8:21 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2015, 8:21 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java 526a0e0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/
-----------------------------------------------------------
(Updated 八月 27, 2015, 8:21 a.m.)
Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
grant select(s) on table test_tb to role test_role;
show columns in test_tb;
Error: Error while compiling statement: FAILED: SemanticException No valid privileges
Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
It should show s column
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java 526a0e0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
Diff: https://reviews.apache.org/r/37794/diff/
Testing
-------
Run local unit case
Thanks,
shen guoquan
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/
-----------------------------------------------------------
(Updated 八月 27, 2015, 5:37 a.m.)
Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
grant select(s) on table test_tb to role test_role;
show columns in test_tb;
Error: Error while compiling statement: FAILED: SemanticException No valid privileges
Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
It should show s column
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
Diff: https://reviews.apache.org/r/37794/diff/
Testing
-------
Run local unit case
Thanks,
shen guoquan
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/
-----------------------------------------------------------
(Updated 八月 27, 2015, 5:15 a.m.)
Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
grant select(s) on table test_tb to role test_role;
show columns in test_tb;
Error: Error while compiling statement: FAILED: SemanticException No valid privileges
Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
It should show s column
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
Diff: https://reviews.apache.org/r/37794/diff/
Testing
-------
Run local unit case
Thanks,
shen guoquan
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by shen guoquan <gu...@intel.com>.
> On 八月 27, 2015, 2:53 a.m., Colin Ma wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 728
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053060#file1053060line728>
> >
> > can we put this method to SentryFilterDDLTask?
It is similar to filterShowTables.
> On 八月 27, 2015, 2:53 a.m., Colin Ma wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 742
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053060#file1053060line742>
> >
> > Define the database and table outof loop.
Thanks Colin for your advice. I will change it
> On 八月 27, 2015, 2:53 a.m., Colin Ma wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 347
> > <https://reviews.apache.org/r/37794/diff/1/?file=1053060#file1053060line347>
> >
> > The logic should be merged with the following code in this method:
> > .....
> > for (Task<? extends Serializable> task : rootTasks) {
> > if (task instanceof SentryGrantRevokeTask) {
> > SentryGrantRevokeTask sentryTask = (SentryGrantRevokeTask)task;
> > ......
I will have a try. If it doesn't success. I will follow the previous code design. Thanks
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96622
-----------------------------------------------------------
On 八月 26, 2015, 9:35 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated 八月 26, 2015, 9:35 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 37794: SENTRY-847: [column level privilege] if
grant column level privilege to user,
show columns in table shouldn't require extra table level privilege
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37794/#review96622
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 85)
<https://reviews.apache.org/r/37794/#comment152214>
Why override this method, no difference with the parent class.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java (line 90)
<https://reviews.apache.org/r/37794/#comment152222>
Why override this method, no difference with the parent class.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 347)
<https://reviews.apache.org/r/37794/#comment152213>
The logic should be merged with the following code in this method:
.....
for (Task<? extends Serializable> task : rootTasks) {
if (task instanceof SentryGrantRevokeTask) {
SentryGrantRevokeTask sentryTask = (SentryGrantRevokeTask)task;
......
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 728)
<https://reviews.apache.org/r/37794/#comment152215>
can we put this method to SentryFilterDDLTask?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 742)
<https://reviews.apache.org/r/37794/#comment152212>
Define the database and table outof loop.
- Colin Ma
On Aug. 26, 2015, 9:35 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37794/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2015, 9:35 a.m.)
>
>
> Review request for sentry, Colin Ma, Dapeng Sun, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> grant select(s) on table test_tb to role test_role;
>
> show columns in test_tb;
>
> Error: Error while compiling statement: FAILED: SemanticException No valid privileges
> Required privileges for this query: Server=server1->Db=test_db->Table=test_tb->action=insert;Server=server1->Db=test_db->Table=test_tb->action=select; (state=42000,code=40000)
>
> It should show s column
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ddfb222
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java 8cd82ef
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 0291b6c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 159b9d9
>
> Diff: https://reviews.apache.org/r/37794/diff/
>
>
> Testing
> -------
>
> Run local unit case
>
>
> Thanks,
>
> shen guoquan
>
>