You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Xiaomeng Huang <xi...@intel.com> on 2014/07/22 07:24:06 UTC

Review Request 23786: SENTRY-340: Database implement for "with grant option"

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

Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

This is database design and implement to support "with grant option".
Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}


Diffs
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 

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


Testing
-------


Thanks,

Xiaomeng Huang


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Xiaomeng Huang <xi...@intel.com>.

> On July 24, 2014, 1:20 a.m., Sravya Tirukkovalur wrote:
> > Thanks for the patch Xiaomeng! Mostly looks good to me. Few comments:
> > 1. You may have to rebase it on Sentry-339 after unique key constraint has been added.
> > 2. I am not completely sure if we require int for grant_option. More detailed comment on SENTRY-345 RB.

Hi, Sravya
Thanks for your comment. 
1. I will rebase it based on Sentry-339. The process is on tracking.
2. I will change the type of grantOption to Boolean in MSentryPrivilege, and CHAR(1) in database.


- Xiaomeng


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


On July 22, 2014, 6:29 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 22, 2014, 6:29 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23786/#review48558
-----------------------------------------------------------


Thanks for the patch Xiaomeng! Mostly looks good to me. Few comments:
1. You may have to rebase it on Sentry-339 after unique key constraint has been added.
2. I am not completely sure if we require int for grant_option. More detailed comment on SENTRY-345 RB.

- Sravya Tirukkovalur


On July 22, 2014, 6:29 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 22, 2014, 6:29 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Xiaomeng Huang <xi...@intel.com>.

> On July 22, 2014, 2:52 p.m., Arun Suresh wrote:
> > Xiomeng, thank you for working on this patch !!
> > 
> > Along with my other comments, I was wondering if you could remove the dependency of "PRIVILEGE_NAME" from your solution. Since the field itself is just a composite of Server, Db, Table, URI and action. This can be modeled as a aggregate key. Please take a look at SENTRY-339

Hi, Arun
Thanks for your review!
I have seen SENTRY-339. It's very good. I will update my patch depends on yours


> On July 22, 2014, 2:52 p.m., Arun Suresh wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java, line 55
> > <https://reviews.apache.org/r/23786/diff/3/?file=638815#file638815line55>
> >
> >     grantOption can be a Boolean i guess

Hi, this is a good point.
I explain it in my design doc.
1.In my design, grant option support priority. High priority can grant/revoke low priority.
2.For solving hive revoke privilege without "grant option" issue, we gave grantOption=-1 a special meaning. 
  If grantOption=-1, we will revoke all privileges with same privilegeName.


> On July 22, 2014, 2:52 p.m., Arun Suresh wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql, line 83
> > <https://reviews.apache.org/r/23786/diff/3/?file=638817#file638817line83>
> >
> >     Any particular reason you need "WITH_GRANT_OPTION" as part of your index ?

In database, it may have two privilege with same privilegeName and different grantOption.
e.g. user1 has privilege {privilegeName=server1+db1+tb1+select, grantOption=1}. user1 can grant this privilege to other role.
     user2 has privilege {privilegeName=server1+db1+tb1+select, grantOption=0}. user2 can't grant this privilege to other role.


> On July 22, 2014, 2:52 p.m., Arun Suresh wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql, line 29
> > <https://reviews.apache.org/r/23786/diff/3/?file=638818#file638818line29>
> >
> >     Can we use BIT/BOOLEAN datatype since this value is either 0 or 1

I explain it in first comment.
SentryStore is general for other components, and not only for hive.
In database design, we support it with priority. And value "-1" has a special meaning.


- Xiaomeng


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


On July 22, 2014, 6:29 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 22, 2014, 6:29 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23786/#review48363
-----------------------------------------------------------


Xiomeng, thank you for working on this patch !!

Along with my other comments, I was wondering if you could remove the dependency of "PRIVILEGE_NAME" from your solution. Since the field itself is just a composite of Server, Db, Table, URI and action. This can be modeled as a aggregate key. Please take a look at SENTRY-339


sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
<https://reviews.apache.org/r/23786/#comment84917>

    grantOption can be a Boolean i guess



sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql
<https://reviews.apache.org/r/23786/#comment84914>

    Any particular reason you need "WITH_GRANT_OPTION" as part of your index ?



sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql
<https://reviews.apache.org/r/23786/#comment84916>

    Can we use BIT/BOOLEAN datatype since this value is either 0 or 1


- Arun Suresh


On July 22, 2014, 6:29 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 22, 2014, 6:29 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Xiaomeng Huang <xi...@intel.com>.

> On July 28, 2014, 10:43 p.m., Sravya Tirukkovalur wrote:
> > Looks like the diff uploaded to review board also contains sentry-339.

This patch based on SENTRY-339. My branch is master, so I upload SENTRY-339 as parent patch.
View Diff or Download Diff will only see my modification based on SENTRY-339.
I also update my newest patch in https://issues.apache.org/jira/browse/SENTRY-340


- Xiaomeng


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


On July 29, 2014, 2:38 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 29, 2014, 2:38 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23786/#review48923
-----------------------------------------------------------


Looks like the diff uploaded to review board also contains sentry-339. 

- Sravya Tirukkovalur


On July 28, 2014, 7:07 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 28, 2014, 7:07 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Xiaomeng Huang <xi...@intel.com>.

> On July 29, 2014, 11:45 p.m., Sravya Tirukkovalur wrote:
> > Thanks for the patch  Xiaomeng! LGTM, just one question: Wonder how is null value of Boolean translated to a char(1) by jdo?

It's a good question. In our design, grantOption couldn't be null in db, it must be Y/N, and I restrain it NOT NULL in dbs.
We store the privilege to db just via grant command. And grant command will give grantOption(Boolean) yes(with grant option) or no(no grant option). And grantOption in MSentryPrivilege is initilized as false.
We just give grantOption in MSentryPrivilege null when Hive execute revoke command. If we got null in SentryStore, we will remove two privileges of grantOption=Y and N in db. And the revoke command will not store any items to db.


- Xiaomeng


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


On July 29, 2014, 2:38 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 29, 2014, 2:38 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> On July 29, 2014, 11:45 p.m., Sravya Tirukkovalur wrote:
> > Thanks for the patch  Xiaomeng! LGTM, just one question: Wonder how is null value of Boolean translated to a char(1) by jdo?
> 
> Xiaomeng Huang wrote:
>     It's a good question. In our design, grantOption couldn't be null in db, it must be Y/N, and I restrain it NOT NULL in dbs.
>     We store the privilege to db just via grant command. And grant command will give grantOption(Boolean) yes(with grant option) or no(no grant option). And grantOption in MSentryPrivilege is initilized as false.
>     We just give grantOption in MSentryPrivilege null when Hive execute revoke command. If we got null in SentryStore, we will remove two privileges of grantOption=Y and N in db. And the revoke command will not store any items to db.

Ah, makes sense. Thanks!


- Sravya


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


On July 29, 2014, 2:38 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 29, 2014, 2:38 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23786/#review49050
-----------------------------------------------------------

Ship it!


Thanks for the patch  Xiaomeng! LGTM, just one question: Wonder how is null value of Boolean translated to a char(1) by jdo?

- Sravya Tirukkovalur


On July 29, 2014, 2:38 a.m., Xiaomeng Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23786/
> -----------------------------------------------------------
> 
> (Updated July 29, 2014, 2:38 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is database design and implement to support "with grant option".
> Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
> And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
> 
> Diff: https://reviews.apache.org/r/23786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Xiaomeng Huang
> 
>


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23786/
-----------------------------------------------------------

(Updated July 29, 2014, 2:38 a.m.)


Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.


Changes
-------

fix a bug.


Repository: sentry


Description
-------

This is database design and implement to support "with grant option".
Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 

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


Testing
-------


Thanks,

Xiaomeng Huang


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23786/
-----------------------------------------------------------

(Updated July 28, 2014, 7:07 a.m.)


Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.


Changes
-------

1. update patch based on SENTRY-339.
2. change data type of grantOption from int to Boolean.


Repository: sentry


Description
-------

This is database design and implement to support "with grant option".
Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 

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


Testing
-------


Thanks,

Xiaomeng Huang


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23786/
-----------------------------------------------------------

(Updated July 22, 2014, 6:29 a.m.)


Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

This is database design and implement to support "with grant option".
Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 

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


Testing
-------


Thanks,

Xiaomeng Huang


Re: Review Request 23786: SENTRY-340: Database implement for "with grant option"

Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23786/
-----------------------------------------------------------

(Updated July 22, 2014, 5:37 a.m.)


Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

This is database design and implement to support "with grant option".
Add a field grantOption(int) to table SENTRY_DB_PRIVILEGE.
And modify the table SENTRY_DB_PRIVILEGE from a single index {privilegeName} to a composite index with member {privilegeName, grantOption}


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 

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


Testing
-------


Thanks,

Xiaomeng Huang