You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li <li...@cloudera.com> on 2017/09/01 21:53:59 UTC

Re: Review Request 61793: SENTRY-1894: Update field size in package.jdo for dataNucleus to match size in sql

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

(Updated Sept. 1, 2017, 9:53 p.m.)


Review request for sentry, Alexander Kolbasov, Sergio Pena, and Vamsee Yarlagadda.


Repository: sentry


Description
-------

reduce field size `SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME` in `SENTRY_DB_PRIVILEGE` from 4000 to 128, and remove `URI` from being part of the unique id


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 734ea7f 


Diff: https://reviews.apache.org/r/61793/diff/4/

Changes: https://reviews.apache.org/r/61793/diff/3-4/


Testing
-------

run datanucleus schema tool. Without this change, it complains "com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes. 
 After those change, the datanucleus schema tool finishes successfully with command "mvn datanucleus:schema-create -X -e"


Thanks,

Na Li


Re: Review Request 61793: SENTRY-1894: Update field size in package.jdo for dataNucleus to match size in sql

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

> On Sept. 7, 2017, 4:34 p.m., Sergio Pena wrote:
> > The SQL files for derby and DB2 do have 4000 characters long on the fields you just modified. Should we change those as well?
> 
> Na Li wrote:
>     I know that. Changing those will require changing sql files. Since they are not causing any issue,  I will update those sql files to make them consistent in sentry-1914.
> 
> Sergio Pena wrote:
>     I don't understand. Why we don't want to keep the consistency between package.jdo and derby/db2 sql files? 
>     Sentry-1914 is going to override these changes?
> 
> Na Li wrote:
>     Yes. sentry-1914 is going to fix it once for all. Its change was part of this code change for sentry-1894. but Sasha only wants me to change package.jdo for this issue, and does not want me to change sql script. That is why I created sentry-1914, and move those sql script changes to sentry-1914. 
>     
>     Now, you want me to move those sql to this issue. What should I do?

Right now, my goal is just to get this issue resolved, and get sentry-1914 resolve later.


- Na


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


On Sept. 1, 2017, 9:53 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61793/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2017, 9:53 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> reduce field size `SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME` in `SENTRY_DB_PRIVILEGE` from 4000 to 128, and remove `URI` from being part of the unique id
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 734ea7f 
> 
> 
> Diff: https://reviews.apache.org/r/61793/diff/4/
> 
> 
> Testing
> -------
> 
> run datanucleus schema tool. Without this change, it complains "com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes. 
>  After those change, the datanucleus schema tool finishes successfully with command "mvn datanucleus:schema-create -X -e"
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 61793: SENTRY-1894: Update field size in package.jdo for dataNucleus to match size in sql

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

> On Sept. 7, 2017, 4:34 p.m., Sergio Pena wrote:
> > The SQL files for derby and DB2 do have 4000 characters long on the fields you just modified. Should we change those as well?

I know that. Changing those will require changing sql files. Since they are not causing any issue,  I will update those sql files to make them consistent in sentry-1914.


- Na


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


On Sept. 1, 2017, 9:53 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61793/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2017, 9:53 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> reduce field size `SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME` in `SENTRY_DB_PRIVILEGE` from 4000 to 128, and remove `URI` from being part of the unique id
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 734ea7f 
> 
> 
> Diff: https://reviews.apache.org/r/61793/diff/4/
> 
> 
> Testing
> -------
> 
> run datanucleus schema tool. Without this change, it complains "com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes. 
>  After those change, the datanucleus schema tool finishes successfully with command "mvn datanucleus:schema-create -X -e"
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 61793: SENTRY-1894: Update field size in package.jdo for dataNucleus to match size in sql

Posted by Sergio Pena <se...@cloudera.com>.

> On Sept. 7, 2017, 4:34 p.m., Sergio Pena wrote:
> > The SQL files for derby and DB2 do have 4000 characters long on the fields you just modified. Should we change those as well?
> 
> Na Li wrote:
>     I know that. Changing those will require changing sql files. Since they are not causing any issue,  I will update those sql files to make them consistent in sentry-1914.

I don't understand. Why we don't want to keep the consistency between package.jdo and derby/db2 sql files? 
Sentry-1914 is going to override these changes?


- Sergio


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


On Sept. 1, 2017, 9:53 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61793/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2017, 9:53 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> reduce field size `SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME` in `SENTRY_DB_PRIVILEGE` from 4000 to 128, and remove `URI` from being part of the unique id
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 734ea7f 
> 
> 
> Diff: https://reviews.apache.org/r/61793/diff/4/
> 
> 
> Testing
> -------
> 
> run datanucleus schema tool. Without this change, it complains "com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes. 
>  After those change, the datanucleus schema tool finishes successfully with command "mvn datanucleus:schema-create -X -e"
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 61793: SENTRY-1894: Update field size in package.jdo for dataNucleus to match size in sql

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

> On Sept. 7, 2017, 4:34 p.m., Sergio Pena wrote:
> > The SQL files for derby and DB2 do have 4000 characters long on the fields you just modified. Should we change those as well?
> 
> Na Li wrote:
>     I know that. Changing those will require changing sql files. Since they are not causing any issue,  I will update those sql files to make them consistent in sentry-1914.
> 
> Sergio Pena wrote:
>     I don't understand. Why we don't want to keep the consistency between package.jdo and derby/db2 sql files? 
>     Sentry-1914 is going to override these changes?

Yes. sentry-1914 is going to fix it once for all. Its change was part of this code change for sentry-1894. but Sasha only wants me to change package.jdo for this issue, and does not want me to change sql script. That is why I created sentry-1914, and move those sql script changes to sentry-1914. 

Now, you want me to move those sql to this issue. What should I do?


- Na


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


On Sept. 1, 2017, 9:53 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61793/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2017, 9:53 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> reduce field size `SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME` in `SENTRY_DB_PRIVILEGE` from 4000 to 128, and remove `URI` from being part of the unique id
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 734ea7f 
> 
> 
> Diff: https://reviews.apache.org/r/61793/diff/4/
> 
> 
> Testing
> -------
> 
> run datanucleus schema tool. Without this change, it complains "com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes. 
>  After those change, the datanucleus schema tool finishes successfully with command "mvn datanucleus:schema-create -X -e"
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 61793: SENTRY-1894: Update field size in package.jdo for dataNucleus to match size in sql

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61793/#review184838
-----------------------------------------------------------



The SQL files for derby and DB2 do have 4000 characters long on the fields you just modified. Should we change those as well?

- Sergio Pena


On Sept. 1, 2017, 9:53 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61793/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2017, 9:53 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> reduce field size `SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME` in `SENTRY_DB_PRIVILEGE` from 4000 to 128, and remove `URI` from being part of the unique id
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 734ea7f 
> 
> 
> Diff: https://reviews.apache.org/r/61793/diff/4/
> 
> 
> Testing
> -------
> 
> run datanucleus schema tool. Without this change, it complains "com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes. 
>  After those change, the datanucleus schema tool finishes successfully with command "mvn datanucleus:schema-create -X -e"
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 61793: SENTRY-1894: Update field size in package.jdo for dataNucleus to match size in sql

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

> On Sept. 8, 2017, 6:05 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
> > Line 122 (original)
> > <https://reviews.apache.org/r/61793/diff/4/?file=1813351#file1813351line122>
> >
> >     This change makes package.jdo inconsistent with sql scripts which do use these fields as keys and the goal was to make these consistent.

The data type and size in package.jdo should be one-to-one mapping to what's in sql.
The index defined in package.jdo is inserted into DB, and the index defined in SQL is also inserted into DB. So the index in package.jdo is not one-to-one mapping to index in sql. In my opinion, we should only keep the index in sql.

when I issue mysql command: "show crete table SENTRY_DB_PRIVILEGE;", the table SENTRY_DB_PRIVILEGE contains two unique index: `PRIVILEGE_INDEX` from datanucleus package.jdo, and `SENTRY_DB_PRIV_PRIV_NAME_UNIQ` from sql script (like sentry-mysql-2.0.0.sql). Should we just keep one unique index for this table?
SENTRY_DB_PRIVILEGE	CREATE TABLE `SENTRY_DB_PRIVILEGE` (
`DB_PRIVILEGE_ID` bigint(20) NOT NULL,
`PRIVILEGE_SCOPE` varchar(32) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL,
`SERVER_NAME` varchar(128) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL,
`DB_NAME` varchar(128) CHARACTER SET utf8 COLLATE utf8_bin DEFAULT '_NULL_',
`TABLE_NAME` varchar(128) CHARACTER SET utf8 COLLATE utf8_bin DEFAULT '_NULL_',
`COLUMN_NAME` varchar(128) CHARACTER SET utf8 COLLATE utf8_bin DEFAULT '_NULL_',
`URI` varchar(4000) CHARACTER SET utf8 COLLATE utf8_bin DEFAULT '_NULL_',
`ACTION` varchar(128) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL,
`CREATE_TIME` bigint(20) NOT NULL,
`WITH_GRANT_OPTION` char(1) NOT NULL,
PRIMARY KEY (`DB_PRIVILEGE_ID`),
UNIQUE KEY `PRIVILEGE_INDEX` (`SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME`,`ACTION`,`WITH_GRANT_OPTION`),
UNIQUE KEY `SENTRY_DB_PRIV_PRIV_NAME_UNIQ` (`SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME`,`URI`(250),`ACTION`,`WITH_GRANT_OPTION`),
KEY `SENTRY_PRIV_SERV_IDX` (`SERVER_NAME`),
KEY `SENTRY_PRIV_DB_IDX` (`DB_NAME`),
KEY `SENTRY_PRIV_TBL_IDX` (`TABLE_NAME`),
KEY `SENTRY_PRIV_COL_IDX` (`COLUMN_NAME`),
KEY `SENTRY_PRIV_URI_IDX` (`URI`(255))
) ENGINE=InnoDB DEFAULT CHARSET=utf8


- Na


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


On Sept. 1, 2017, 9:53 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61793/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2017, 9:53 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> reduce field size `SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME` in `SENTRY_DB_PRIVILEGE` from 4000 to 128, and remove `URI` from being part of the unique id
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 734ea7f 
> 
> 
> Diff: https://reviews.apache.org/r/61793/diff/4/
> 
> 
> Testing
> -------
> 
> run datanucleus schema tool. Without this change, it complains "com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes. 
>  After those change, the datanucleus schema tool finishes successfully with command "mvn datanucleus:schema-create -X -e"
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 61793: SENTRY-1894: Update field size in package.jdo for dataNucleus to match size in sql

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61793/#review184946
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
Line 122 (original)
<https://reviews.apache.org/r/61793/#comment261165>

    This change makes package.jdo inconsistent with sql scripts which do use these fields as keys and the goal was to make these consistent.


- Alexander Kolbasov


On Sept. 1, 2017, 9:53 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61793/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2017, 9:53 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> reduce field size `SERVER_NAME`,`DB_NAME`,`TABLE_NAME`,`COLUMN_NAME` in `SENTRY_DB_PRIVILEGE` from 4000 to 128, and remove `URI` from being part of the unique id
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 734ea7f 
> 
> 
> Diff: https://reviews.apache.org/r/61793/diff/4/
> 
> 
> Testing
> -------
> 
> run datanucleus schema tool. Without this change, it complains "com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes. 
>  After those change, the datanucleus schema tool finishes successfully with command "mvn datanucleus:schema-create -X -e"
> 
> 
> Thanks,
> 
> Na Li
> 
>