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:
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.