[
http://issues.apache.org/jira/browse/DERBY-1489?page=comments#action_12438545 ]
Mamta A. Satoor commented on DERBY-1489:
----------------------------------------
Bryan, thanks for the patch. I have some comments on it.
***************************************************************************************************************************************************************
Comments for the engine changes
***************************************************************************************************************************************************************
1)In ViewDescriptor.prepareToInvalide, there are some existing comments saying
that REVOKE_PRIVILEGE_RESTRICT is going to throw an exception. Should we add
some comments for DROP_COLUMN_RESTRICT too? I can see why you didn't add
anything because by default, any invalidation that is not caught will endup in
exception and hence probably no need to add comments for them.
********
2)In AlterTableConstantAction, why is following if condition not required
anymore?
if (numRefCols > 1 || cd.getConstraintType() ==
DataDictionary.PRIMARYKEY_CONSTRAINT)
********
3)Is keyword COLUMN optional in the following syntax?
ALTER TABLE tablename DROP COLUMN columnname [CASCADE | RESTRICT]
The reason I ask is there is a test in altertableDropColumn as follows
+ij> alter table atdc_2 drop b;
+ERROR 42X14: 'B' is not a column in table or VTI 'ATDC_2'.
I thought the error message will be something different for missing keyword
COLUMN/CONSTRAINT.
********
4)I am wondering if we should change the error text for the drop column in sql
authorization mode ie rather than saying (sqlAuthorization=true), say that we
are in SQL Authorization mode or something along that line. But since this is
temporary, probably not worth it.
+ERROR 0A000: Feature not implemented: ALTER TABLE DROP COLUMN
(sqlAuthorization=true).
***************************************************************************************************************************************************************
***************************************************************************************************************************************************************
Comments for the test cases
***************************************************************************************************************************************************************
1)If there is a check constraint defined on a column and drop column CASCADE is
executed, should it drop the check constraint? In the new test file, I see an
example for drop column restrict failing when there is a check constraint
defined on it but I am not clear on what would happen for drop column cascade
in this case.
********
2)Also, I didn't quite understand the behavior when a column is dropped that
participates in a multi-column index? It seems from the test that the column
gets dropped but index is still around.
Also, from the test, it looks like if the colum being dropped is the only
column in the index, then the column and index both get dropped.
********
3)Is there some work required for following comment in the new test?
+-- FIXME: cascade/restrict processing doesn't currently consider indexes?
It seems like your concern is correct because if there is a primary key/check
constraint, the drop column fails. So, seems like it should fail for index too.
I haven't checked what the SQL spec says.
If we find that the behavior of drop column is different for different kinds of
constraints, then some documentation on that will be very useful.
********
4)With one level dependency, ie a view defined on a column. The column drop
drops the view but there is no warning raised. In case of triggers, we generate
a warning (copied following from the new test master). Should there be warning
for drop of views/other constraints that get dropped automatically because of
drop column cascade?
+ij> alter table atdc_6 drop column b cascade;
+0 rows inserted/updated/deleted
+WARNING 01502: The trigger ATDC_6_TRIGGER_1 on table ATDC_6 has been dropped.
********
5)Consider a case where a column is referenced by a view and that view is
referenced by another view. What would happen if someone tries to
drop that column with cascade behavior. Will it succeed, will both views get
dropped too or will the drop fail?
column->view1->view2
drop column cascade will drop both views?
***************************************************************************************************************************************************************
General comment
1)Bryan, I think you already have this in your radar screen but I will bring it
up, just in case. I think you are planning to add a new jira entry for DROP
COLUMN and sql authorization if we decide to go ahead with the functionality
proposed by this patch. Once that is done and before this patch gets committed,
we should put the correct jira number in comments in altertableDropColumn.sql
and in altertable.sql. Since we donot have a jira entry at this point, I see
that we refer to it as DERBY-XXXX in the test comments. Also, in the new jira
entry for DROP COLUMN in sql authorization mode, we should mention that
altertableDropColumn.sql and in altertable.sql should be consolidated into
altertable.sql.
> 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
