[
https://issues.apache.org/jira/browse/DERBY-4988?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12991639#comment-12991639
]
Mamta A. Satoor commented on DERBY-4988:
----------------------------------------
Dag, thanks for taking the time to review the patch. You comments/questions
were as follows
Comments on AlterTableConstantAction
1)You are right about following. This was a pre-existing comment. I will get
rid of it because what we are doing follows the SQL standards.
* // otherwsie there would be unexpected behaviors
Apart from the typo, is this not SQL requirement?
2)
The "change" variable can probably be eliminated, since this test is repeated
over again at end of loop:
The code about the "change" variable is pre-existing too. Following is why I
think it may have been coded this way.
Inside the "if (j == refColLen && changed)" true code, we drop the trigger and
then adjust the column positions of the affected trigger columns and then
recreate the trigger with correct column positions. We drop the trigger because
we know from changed being true, that the trigger column position(s) has been
affected by the drop column.
If we took out the information kept by"changed", then we won't be sure if we
should drop the trigger because we are not certain if any trigger column's
position changed because of the drop column. We could change the
code inside the if to alway drop the trigger inside the if (j == refColLen) ie
even if none of the column positions got impacted, then rearrange the column
positions if needed and then recreate the trigger. ALTER TABLE DROP COLUMN
should be a rare operation in production environment and hence it choosing to
drop the unaffected trigger and recreating them should not be a big drawback.
Or we can be more intelligent about dropping the trigger by doing so only if
the trigger column positions have changed.
So, we can either add some comments to the existing code to improve the
readability of the if codition or we can get rid of the "changed" variable and
simply drop the trigger, rearrange it's column positions if needed and recrete
the trigger. Existing code avoids unnecessary drop and recreate of the trigger
if the trigger columns are not affected. But your suggestion makes the code
more readable. I am fine with either of the 2 choices.
3)
changedColPositionInTriggerAction is set but not used anywhere?
Shouldn't the positions in referencedColsInTriggerAction be adjusted
above droppedColumnPosition, too?
For trigger reference columns, we can't simply do what is being done for
trigger columns because trigger action sql gets changed to look at the columns
available through the REFERENCING clause to be accessed by their column
positions. Hence, just changing the column positions of affected referenced
columns in systrigges is not enough. We need to regenerate the trigger action
too.I am looking at tackling the regeneration of the trigger action for such a
case through the jira DERBY-4984 which I am working on currently.
4)You are correct about my comment being incorrect in AlterTableTest. I will
fix that why I do my checkin for DERBY-4984
> ALTER TABLE DROP COLUMN should make use of information in SYSTRIGGERS to
> detect column used through REFERENCING clause to find trigger dependencies
> ---------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-4988
> URL: https://issues.apache.org/jira/browse/DERBY-4988
> Project: Derby
> Issue Type: Bug
> Components: SQL
> Affects Versions: 10.7.1.1
> Reporter: Mamta A. Satoor
> Assignee: Mamta A. Satoor
> Fix For: 10.8.0.0
>
>
> At the time of ALTER TABLE DROP COLUMN, Derby checks if the column being
> dropped in a trigger column and if so, then it will not drop the column if it
> is being done in RESTRICT mode or it will drop the trigger while dropping the
> column in CASCADE mode. This does not implement SQL standard to it's
> entirety.
> **************************************
> SQL standard says following about ALTER TABLE DROP COLUMN RESTRICT and
> trigger dependency in CREATE TRIGGER section
> If RESTRICT is specified, then C shall not be referenced in any of the
> following
> d) Either an explicit trigger column list or a triggered action column set of
> any trigger descriptor.
> (The triggered action column set included in the trigger descriptor is the
> set of all distinct, fully qualified names of columns contained in the
> <triggered action>.)
> **************************************
> What is missing from Derby implementation from SQL standard point of view is
> detected triggered action column set.
> Starting 10.7(with DERBY-1482), Derby started keeping track of trigger action
> columns which are referenced through REFERENCING clause. This information can
> be used to improve the behavior of ALTER TABLE DROP COLUMN in 10.7 and
> higher. This will not cover all the trigger action columns since columns
> referenced without the REFERENCING clause are not tracked anywhere at this
> point. More work will need to be done to implement SQL standard completely.
> But we can take a step forward by using the information available in 10.7 and
> higher to detect trigger action columns which are referenced through
> REFERENCING clause
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
