Index: java/engine/org/apache/derby/impl/sql/compile/CompilerContextImpl.java =================================================================== --- java/engine/org/apache/derby/impl/sql/compile/CompilerContextImpl.java (revision 449144) +++ java/engine/org/apache/derby/impl/sql/compile/CompilerContextImpl.java (working copy) @@ -37,10 +37,12 @@ import org.apache.derby.iapi.sql.dictionary.ColumnDescriptor; import org.apache.derby.iapi.sql.dictionary.TableDescriptor; import org.apache.derby.iapi.sql.dictionary.AliasDescriptor; +import org.apache.derby.iapi.sql.dictionary.StatementPermission; import org.apache.derby.iapi.sql.dictionary.StatementTablePermission; import org.apache.derby.iapi.sql.dictionary.StatementSchemaPermission; import org.apache.derby.iapi.sql.dictionary.StatementColumnPermission; import org.apache.derby.iapi.sql.dictionary.StatementRoutinePermission; +import org.apache.derby.iapi.sql.dictionary.DataDictionary; import org.apache.derby.iapi.types.DataTypeDescriptor; @@ -728,7 +730,8 @@ * * @param column The column whose privileges we're interested in. */ - public void addRequiredColumnPriv( ColumnDescriptor column) + public void addRequiredColumnPriv( ColumnDescriptor column) + throws StandardException { if( requiredColumnPrivileges == null // Using old style authorization || currPrivType == Authorizer.NULL_PRIV @@ -765,6 +768,8 @@ currPrivType, new FormatableBitSet( td.getNumberOfColumns())); requiredColumnPrivileges.put(key, tableColumnPrivileges); + + addToPermissionsCache(lcc, tableColumnPrivileges); } tableColumnPrivileges.getColumns().set(column.getPosition() - 1); } // end of addRequiredColumnPriv @@ -775,12 +780,15 @@ * @see CompilerContext#addRequiredRoutinePriv */ public void addRequiredTablePriv( TableDescriptor table) + throws StandardException { if( requiredTablePrivileges == null || table == null) return; StatementTablePermission key = new StatementTablePermission( table.getUUID(), currPrivType); requiredTablePrivileges.put(key, key); + + addToPermissionsCache(lcc, key); } /** @@ -789,6 +797,7 @@ * @see CompilerContext#addRequiredRoutinePriv */ public void addRequiredRoutinePriv( AliasDescriptor routine) + throws StandardException { // routine == null for built in routines if( requiredRoutinePrivileges == null || routine == null) @@ -799,7 +808,14 @@ return; if (requiredRoutinePrivileges.get(routine.getUUID()) == null) - requiredRoutinePrivileges.put(routine.getUUID(), ReuseFactory.getInteger(1)); + { + StatementRoutinePermission routPerm = + new StatementRoutinePermission( routine.getUUID() ); + + requiredRoutinePrivileges.put(routine.getUUID(), routPerm); + + addToPermissionsCache(lcc, routPerm); + } } /** @@ -816,6 +832,10 @@ StatementSchemaPermission(schemaName, aid, privType); requiredSchemaPrivileges.put(key, key); + + // There is no corresponding system permissions table for schema. + // This is just an in-memory object, so there is no need to call + // addToPermissionsCache(). } /** @@ -836,11 +856,9 @@ ArrayList list = new ArrayList( size); if( requiredRoutinePrivileges != null) { - for( Iterator itr = requiredRoutinePrivileges.keySet().iterator(); itr.hasNext();) + for( Iterator itr = requiredRoutinePrivileges.values().iterator(); itr.hasNext();) { - UUID routineUUID = (UUID) itr.next(); - - list.add( new StatementRoutinePermission( routineUUID)); + list.add( itr.next()); } } if( requiredTablePrivileges != null) @@ -867,6 +885,48 @@ return list; } // end of getRequiredPermissionsList + /** + * load the PermissionsDescriptor if any to the permission cache for + * both authId and PUBLIC. + * + * PermissionsDescriptors should also be read from the permissions + * related system table(s) at compilation time. This is done so + * that at execution time, the system does not have to go through + * the permissions related system table(s) and hold those shared + * lock(s) till end of user transaction. Note that this mechanism + * is currently performed only on authId that is compiling the + * statement. (since there may be a tendency that this authId may + * execute the compiled statement later.) + * + * Permission checking is performed at execution time. + * + * This aux method simply re-use the StatementPermission object's + * logic to perform the work. The actual mechanism of loading a + * PermissionsDescriptor from its corresponding system table is + * implemented in the DataDictionary. + * + * @param lcc language connection context + * @param permission the statement permission + * + * @exception StandardException thrown on error + */ + private void addToPermissionsCache(LanguageConnectionContext lcc, + StatementPermission perm) + throws StandardException + { + DataDictionary dd = lcc.getDataDictionary(); + + // no need to load from system tables if authId is the database owner + if (lcc.getAuthorizationId().equals(dd.getAuthorizationDatabaseOwner())) + return; + + perm.getPermissionDescriptor( lcc.getAuthorizationId(), dd); + + // look for PUBLIC also since execution checks for this as well. + perm.getPermissionDescriptor( Authorizer.PUBLIC_AUTHORIZATION_ID, dd); + + } // end of addToPermissionsCache + /* ** Context state must be reset in restContext() */ Index: java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java =================================================================== --- java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java (revision 449144) +++ java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java (working copy) @@ -525,15 +525,19 @@ * Add a column privilege to the list of used column privileges. * * @param column + * + * @exception StandardException Thrown on error */ - public void addRequiredColumnPriv( ColumnDescriptor column); + public void addRequiredColumnPriv( ColumnDescriptor column) throws StandardException; /** * Add a table or view privilege to the list of used table privileges. * * @param table + * + * @exception StandardException Thrown on error */ - public void addRequiredTablePriv( TableDescriptor table); + public void addRequiredTablePriv( TableDescriptor table) throws StandardException; /** * Add a schema privilege to the list of used privileges. @@ -548,8 +552,10 @@ * Add a routine execute privilege to the list of used routine privileges. * * @param routine + * + * @exception StandardException Thrown on error */ - public void addRequiredRoutinePriv( AliasDescriptor routine); + public void addRequiredRoutinePriv( AliasDescriptor routine) throws StandardException; /** * @return The list of required privileges. Index: java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL_app.properties =================================================================== --- java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL_app.properties (revision 449144) +++ java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL_app.properties (working copy) @@ -2,8 +2,14 @@ ij.showNoConnectionsAtStart=true derby.database.sqlAuthorization=true + +# we want wait timeouts to be quick +derby.locks.deadlockTimeout=5 +derby.locks.waitTimeout=2 + useextdirs=true # DataSource properties, only used if ij.dataSource is set ij.dataSource.databaseName=wombat ij.dataSource.createDatabase=create + Index: java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL.sql =================================================================== --- java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL.sql (revision 449144) +++ java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL.sql (working copy) @@ -1948,3 +1948,31 @@ grant references(c112) on d1589t11ConstraintTest to PUBLIC; set connection mamta3; create table d1589t31ConstraintTest (c311 int, c312 int, foreign key(c311, c312) references mamta1.d1589t11ConstraintTest); + +-- DERBY-1716 +-- Revoking select privilege from a user times out when that user still have +-- a cursor open before the patch. +set connection user1; +drop table t1; +create table t1 (c varchar(1)); +insert into t1 values 'a', 'b', 'c'; +grant select on t1 to user2; +set connection user2; +autocommit off; +GET CURSOR crs1 AS 'select * from user1.t1'; +next crs1; +set connection user1; +-- should succeed without blocking +revoke select on t1 from user2; +set connection user2; +-- still ok to fetch. +next crs1; +next crs1; +close crs1; +commit; +-- should fail since select privilege got revoked +GET CURSOR crs1 AS 'select * from user1.t1'; +next crs1; +close crs1; +autocommit on; +set connection user1; Index: java/testing/org/apache/derbyTesting/functionTests/master/grantRevokeDDL.out =================================================================== --- java/testing/org/apache/derbyTesting/functionTests/master/grantRevokeDDL.out (revision 449144) +++ java/testing/org/apache/derbyTesting/functionTests/master/grantRevokeDDL.out (working copy) @@ -3077,4 +3077,48 @@ ij(MAMTA1)> set connection mamta3; ij(MAMTA3)> create table d1589t31ConstraintTest (c311 int, c312 int, foreign key(c311, c312) references mamta1.d1589t11ConstraintTest); 0 rows inserted/updated/deleted -ij(MAMTA3)> +ij(MAMTA3)> -- DERBY-1716 +-- Revoking select privilege from a user times out when that user still have +-- a cursor open before the patch. +set connection user1; +ij(USER1)> drop table t1; +ERROR: Failed with SQLSTATE 42Y55 +ij(USER1)> create table t1 (c varchar(1)); +0 rows inserted/updated/deleted +ij(USER1)> insert into t1 values 'a', 'b', 'c'; +3 rows inserted/updated/deleted +ij(USER1)> grant select on t1 to user2; +0 rows inserted/updated/deleted +ij(USER1)> set connection user2; +ij(USER2)> autocommit off; +ij(USER2)> GET CURSOR crs1 AS 'select * from user1.t1'; +ij(USER2)> next crs1; +C +---- +a +ij(USER2)> set connection user1; +ij(USER1)> -- should succeed without blocking +revoke select on t1 from user2; +0 rows inserted/updated/deleted +ij(USER1)> set connection user2; +ij(USER2)> -- still ok to fetch. +next crs1; +C +---- +b +ij(USER2)> next crs1; +C +---- +c +ij(USER2)> close crs1; +ij(USER2)> commit; +ij(USER2)> -- should fail since select privilege got revoked +GET CURSOR crs1 AS 'select * from user1.t1'; +ERROR: Failed with SQLSTATE 28508 +ij(USER2)> next crs1; +IJ ERROR: Unable to establish cursor +ij(USER2)> close crs1; +IJ ERROR: Unable to establish cursor +ij(USER2)> autocommit on; +ij(USER2)> set connection user1; +ij(USER1)>