[ http://issues.apache.org/jira/browse/DERBY-1489?page=comments#action_12438937 ] Bryan Pendleton commented on DERBY-1489: ----------------------------------------
Mamta, thanks again for the great review and the suggestions! They have been very interesting to pursue. I am intending to attach a revised patch later this weekend incorporating your suggestions, but I wanted to respond to a couple of your notes separately, as follows: 1) Regarding the handling of indexes when dropping a column, there was a discussion in early July on the derby-dev list in which we concluded that CASCADE/RESTRICT should not consider indexes, but instead should always simply remove the column from any affected indexes, and drop the entire index if this column was the last column. Here's a pointer to the email discussion: http://www.nabble.com/forum/ViewPost.jtp?post=5254954&framed=y 2) I'm glad you asked some questions about constraint handling because that caused me to write some more tests with PRIMARY and FOREIGN keys, and I found a problem in the case where the primary key, and the foreign key which references it, are both in the same table. The execution code was correctly cascading the delete, but during the table rebuild we were using a stale table descriptor and getting a "conglomerate not found" error. I changed the execution logic so that, after it has dropped all the affected PRIMARY and FOREIGN key constraints (and indexes) and before it rebuilds the base table, it re-reads the table descriptor from the system tables to ensure that it has the right table descriptor information. So I added a bunch more tests in this latest patch :) 3) You asked a very interesting question: why is following if condition not required anymore? if (numRefCols > 1 || cd.getConstraintType() == DataDictionary.PRIMARYKEY_CONSTRAINT) I removed this "if" test when I was changing DROP COLUMN to properly handle UNIQUE constraints. This line of code is in the code path which is performing RESTRICT analysis, to see if the constraint that has been found should cause the DROP COLUMN to fail. The effect of the "if" test was to say that the only constraints that caused DROP COLUMN RESTRICT to fail were those that were either: - PRIMARY KEY constraints that referred to this column, or - other types of constraints that referred to this column, as well as some other column in this table When I added processing for UNIQUE constraints, I thought about modifying this "if" test to say || cd.getConstraintType() == DataDictionary.UNIQUE_CONSTRAINT so that UNIQUE constraints that referred to *only* this column in this table would fail in RESTRICT mode, but the more I looked at this "if" test, the more I thought that the test was just unnecessary. We don't actually care whether the constraint affects 1 column or multiple columns, and we don't care whether it is a PRIMARYKEY_CONSTRAINT or some other sort of constraint. All we care about is that there is a constraint on this table which references this column, and the user has said RESTRICT. By removing the "if" test, the code interprets RESTRICT to apply to any such constraint, which I think is the right behavior. =================== I believe I've incorporated all your other suggestions into the revised patch. When my testing completes, I'll attach that patch, and look forward to additional feedback. > Provide ALTER TABLE DROP COLUMN functionality > --------------------------------------------- > > Key: DERBY-1489 > URL: http://issues.apache.org/jira/browse/DERBY-1489 > Project: Derby > Issue Type: New Feature > Components: Documentation, SQL > Affects Versions: 10.0.2.0, 10.0.2.1, 10.1.1.0, 10.2.1.5, 10.1.2.1, > 10.1.3.0, 10.1.3.1 > Reporter: Bryan Pendleton > Assigned To: Bryan Pendleton > Attachments: drop_column_v5_grant_tests.diff, > drop_column_v6_grant_not_implemented.diff, dropColumn_2.diff, > dropColumn_v3_view_drop.diff, dropColumn_v4_grant_tests.diff > > > Provide a way to drop a column from an existing table. Possible syntax would > be: > ALTER TABLE tablename DROP COLUMN columnname CASCADE / RESTRICT; > Feature should properly handle columns which are used in constraints, views, > triggers, indexes, etc. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
