|
patch is available, making it a 10.2 candidate.
Yip, I did a review of the attached package and following couple questions/comments.
1)I had question about changes to DropSchemaNode. I realize that my question here is related to pre-existing logic that you needed to extend for drop schema. But I wondered why for table privilege, we push the privilege requirement(select/trigger/update etc) by calling CompilerContext.pushCurrentPrivType and use that to determine what kind of privilege is required for the table in CompilerContextImpl.addRequiredTablePriv method. But for privilege requirement for schemas, we pass the privilege type to CompilerContextImpl.addRequiredSchemaPriv rather than pushing it onto CompilerContext and then popping it back from there at the right time. I should have asked this question when the code was originally checked in for CreateSchemaNode. This is not a biggie, just a curiosity question. 2)In Authorizer.java, there is a constant called PRIV_TYPE_COUNT which has a value of 7. With the patch attached to this Jira entry, we have added 1 more privilege type. Do we need to change PRIV_TYPE_COUNT? May be the purpose of PRIV_TYPE_COUNT is to track only table and routine level privilege types and that is why we didn't need to modify it for this patch. 3)Can we have some positive test cases for drop schema in grantRevokeDDL2.sql showing that a database owner or schema owner can successfully drop the schema? Other than that, the changes look pretty good to me. Thanks, Yip. Thanks for reviewing the patch, Mamta. My response to your comments:
1) Yes, I noticed that also. If you look at the CREATE_SCHEMA_PRIV and MODIFY_SCHEMA_PRIV, they are of boolean types and not like the other 7 privilege types which are declared as integers. So it looks like privilege collection for schema are done abit differently and they were not considered as an integral part of the push/pop privilege logic in compiler context. (They could be, but its not necessary) 2) The PRIV_TYPE_COUNT should still be 7 since those privilege types(select, insert, update, ..., etc.) are part of grant/revoke statement. The other schema privileges are used sorely to differentiate what action to take during permission checking time. 3) Yes, there is already a testcase to test if database owner can drop the schema. I believe I have uncommented that part of test in grantRevokeDDL2.sql to re-enable it for this jira. I looked at applying this patch, but it failed with the following warning, not sure if it is my environment or that the patch needs to be regenerated:
m1_ibm142:52>pwd C:/p4/m1/opensource m1_ibm142:53>patch --dry-run -p0 -i c:/tmp/derby1858-trunk-diff01.txt patching file `java/engine/org/apache/derby/impl/sql/compile/CompilerContextImpl .java' patching file `java/engine/org/apache/derby/impl/sql/compile/DropSchemaNode.java ' patching file `java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.jav a' patching file `java/engine/org/apache/derby/iapi/sql/conn/Authorizer.java' patching file `java/engine/org/apache/derby/iapi/sql/dictionary/StatementSchemaP ermission.java' Hunk #3 FAILED at 66. 1 out of 3 hunks FAILED -- saving rejects to java/engine/org/apache/derby/iapi/s ql/dictionary/StatementSchemaPermission.java.rej patching file `java/testing/org/apache/derbyTesting/functionTests/tests/lang/gra ntRevokeDDL2.sql' patching file `java/testing/org/apache/derbyTesting/functionTests/master/jdk16/g rantRevokeDDL2.out' patching file `java/testing/org/apache/derbyTesting/functionTests/master/grantRe vokeDDL2.out' thanks, the 2nd patch applied cleanly for me.
committed change to the trunk.
m1_ibm142:161>svn commit Sending java\engine\org\apache\derby\iapi\sql\compile\CompilerContext.jav a Sending java\engine\org\apache\derby\iapi\sql\conn\Authorizer.java Sending java\engine\org\apache\derby\iapi\sql\dictionary\StatementSchemaP ermission.java Sending java\engine\org\apache\derby\impl\sql\compile\CompilerContextImpl .java Sending java\engine\org\apache\derby\impl\sql\compile\DropSchemaNode.java Sending java\testing\org\apache\derbyTesting\functionTests\master\grantRe vokeDDL2.out Sending java\testing\org\apache\derbyTesting\functionTests\master\jdk16\g rantRevokeDDL2.out Sending java\testing\org\apache\derbyTesting\functionTests\tests\lang\gra ntRevokeDDL2.sql Transmitting file data ........ Committed revision 449869. Patch applied to trunk and 10.2. Marking resolved.
This issue has been resolved for over a year with no further movement. Closing.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Major
A schema can be dropped by a non-schema owner in SQL authorization mode
DERBY-1858. The problem is that DropSchemaNode's bind phase did not add the required schema privilege for it to check at runtime. derbyall ran successfully.