You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda <kk...@cloudera.com> on 2017/02/08 19:17:01 UTC
Review Request 56458: SENTRY-1614 Removed code having PATHS column as
primary key. Primary key of this long is not accepted by all databases
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56458/
-----------------------------------------------------------
Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
Repository: sentry
Description
-------
SENTRY-1614 Removed code having PATHS column as primary key. Primary key of this long is not accepted by all databases
Code changes include following
1. Removing PATHS column from primary key as Primary key of this long is not accepted by all databases.
2. Remove the index on MAUTHZPATHSMAPPING_PATHS. As there would be implicit index created with AUTHZ_OBJ_ID_OID.
3. Added test to run Schematool tests with mysql.
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java d75e24bb17221a7b676e735d9976e83d8f17c182
sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql 1883626262bf4f4936f156a7ac74365b9b5873df
sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.mysql.sql 1829e2fa1f02a4339e7af4bf45a169013e9ec65f
sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.oracle.sql 7de9751892a8ff84067f67d542ac58d33e9148d8
sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.postgres.sql adf5f1f39596309183f8c80d2c8ad1f1a7730236
sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.8.0.sql 0606116a1d31c400fb6dadcd80a2284007117ab2
sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.8.0.sql be9a33e6b0904b7c8ec522906d967081631108d6
sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.8.0.sql 1c8848c30d0432ebdbd18b73a1d27c61d2b7bcae
sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.8.0.sql fc7b53f41e97fad5e5baeb254b686a0e8cc5b003
sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.8.0.sql ce807a5aee9d04e119dde32784cbe958bf933feb
sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-1.7.0-to-1.8.0.sql 92d0e3314141e01cab3a8321bba0ca253378b27f
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaTool.java 68abf277e0eab48a16f9c1b10147a11b9d10394a
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java PRE-CREATION
Diff: https://reviews.apache.org/r/56458/diff/
Testing
-------
1. ran the sql scripts with all the databases we support(db2, postgress, oracle and mysql)
2. Ran TestSentrySchemaTool tests.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 56458: SENTRY-1614 Removed code having PATHS
column as
primary key. Primary key of this long is not accepted by all databases
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56458/#review164874
-----------------------------------------------------------
Please try to separate SQL changes from java changes into separate changesets.
SQL changes are fine.
For Java changes - the mysql test looks more like a home directory tool so I would question it inclusion here.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java (line 160)
<https://reviews.apache.org/r/56458/#comment236713>
Why do you need this method? Who is using it? Is it only for testing or actually used for something? If this is testing-only, please move it to the test.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java (line 165)
<https://reviews.apache.org/r/56458/#comment236711>
Is this a stray non-empty line?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaTool.java
<https://reviews.apache.org/r/56458/#comment236715>
Why is this removed? It is never used or there is another reason?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java (line 30)
<https://reviews.apache.org/r/56458/#comment236716>
Is this test included in automatic run or it should be run manually?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java (line 36)
<https://reviews.apache.org/r/56458/#comment236717>
Please format using javadoc conventions - use <p> for paragraph separation and {@code} for code snippets.
Where should these dependencies be added?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java (line 40)
<https://reviews.apache.org/r/56458/#comment236718>
lines 40 and 41 should probably be removed
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java (line 46)
<https://reviews.apache.org/r/56458/#comment236719>
Do we need to change it when we are running the test?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java (line 55)
<https://reviews.apache.org/r/56458/#comment236720>
Should we change this manually every time?
Please do not use your name or "cloudera" as a pass.
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java (line 58)
<https://reviews.apache.org/r/56458/#comment236721>
What is dot in this path? WIll this work if we run test from a different dir?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java (line 66)
<https://reviews.apache.org/r/56458/#comment236722>
What is this IP address?
- Alexander Kolbasov
On Feb. 8, 2017, 7:16 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56458/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2017, 7:16 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1614 Removed code having PATHS column as primary key. Primary key of this long is not accepted by all databases
>
> Code changes include following
> 1. Removing PATHS column from primary key as Primary key of this long is not accepted by all databases.
> 2. Remove the index on MAUTHZPATHSMAPPING_PATHS. As there would be implicit index created with AUTHZ_OBJ_ID_OID.
> 3. Added test to run Schematool tests with mysql.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java d75e24bb17221a7b676e735d9976e83d8f17c182
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql 1883626262bf4f4936f156a7ac74365b9b5873df
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.mysql.sql 1829e2fa1f02a4339e7af4bf45a169013e9ec65f
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.oracle.sql 7de9751892a8ff84067f67d542ac58d33e9148d8
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.postgres.sql adf5f1f39596309183f8c80d2c8ad1f1a7730236
> sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.8.0.sql 0606116a1d31c400fb6dadcd80a2284007117ab2
> sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.8.0.sql be9a33e6b0904b7c8ec522906d967081631108d6
> sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.8.0.sql 1c8848c30d0432ebdbd18b73a1d27c61d2b7bcae
> sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.8.0.sql fc7b53f41e97fad5e5baeb254b686a0e8cc5b003
> sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.8.0.sql ce807a5aee9d04e119dde32784cbe958bf933feb
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-1.7.0-to-1.8.0.sql 92d0e3314141e01cab3a8321bba0ca253378b27f
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaTool.java 68abf277e0eab48a16f9c1b10147a11b9d10394a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/56458/diff/
>
>
> Testing
> -------
>
> 1. ran the sql scripts with all the databases we support(db2, postgress, oracle and mysql)
> 2. Ran TestSentrySchemaTool tests.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>