ALTER TABLE DROP COLUMN needs to update GRANTed column privileges

• Key: DERBY-1909
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Bryan Pendleton
• Reporter: Bryan Pendleton
• Component/s: SQL
• Affects Version/s: 10.3.1.4
• Fix Version/s: 10.3.1.4

Description

When ALTER TABLE DROP COLUMN is used to drop a column from a table, it needs to update the GRANTed column privileges on that table.

The following behaviors need to be provided:
- DROP COLUMN needs to automatically revoke any privileges which have been granted on this column
- Privileges which have been granted on other columns in this table need to be adjusted due to changed column position numbers

When this problem is fixed, the following additional cleanup steps should occur:
- The check that disables DROP COLUMN in sqlAuthorization mode needs to be removed
- There is a comment in AlterTableConstantAction that needs to be revised
- The tests in lang/altertableDropColumn.sql should be moved into altertable.sql
- altertableDropColumn.sql and altertableDropColumn.out should be deleted

Search for this JIRA issue number in the code to find the relevant places to change.

Comments

[Bryan Pendleton]

This issue is a follow-on from DERBY-1489. DERBY-1489 implements most of the DROP COLUMN behaviors, but did not handle the interaction with GRANTed column privileges.

[Bryan Pendleton]

The techniques in DERBY-1847 may be useful as a building block for fixing this issue.

[Bryan Pendleton]

repro.sql is a first start at a script demonstrating the problem

[Bryan Pendleton]

I started working on a possible patch, but hit a bit of a confusion while trying
to re-factor some code in DataDictionaryImpl.java. I don't want to lose the
work I did, so I'm attaching d1909_partial_incomplete.diff to save that work.
Hopefully I can pick up this work and continue it once I understand the DERBY-1847
fix better.

[Bryan Pendleton]

Attached is rewrite_syscolperms_v1.diff, a proposed patch for this problem.

The core of this proposed patch involves refactoring and reusing the
DERBY-1847 method which knows how to rewrite SYSCOLPERMS rows
to update the COLUMNS column. The DERBY-1847 version of that code
only handled the case of adding a bit to the COLUMNS column; this patch
extends that method to support removing a bit from the COLUMNS
column as well, then calls the method from the AlterTable execution logic.

The diff file is rather large because because it includes the removal of
altertableDropColumn.sql and altertableDropColumn.out, and the
merging of those files into altertable.sql and altertable.out, so the
entire contents of those files are included in the diff twice each :)

The rest of the diff is pretty small and straightforward and hopefully is
not hard to review.

The diff includes the proposed 2-line change from DERBY-1847, because
that change is mandatory for this fix. Once the DERBY-1847 change is
resolved, I'll revisit this diff to remove those two lines from DataDictionaryImpl.java

The proposed patch has one particularly interesting behavior: in the case
in which a column is dropped from a table, and there exists a user who
was granted privileges on that column, and that column only, in that table,
the diff processes their SYSCOLPERMS row and leaves a "empty" row; that
is, a row in which the COLUMNS column of SYSCOLPERMS has all bits clear.
From what I can tell, this does not cause any problems; the behavior is
reasonable. However, it might be argued that it would be preferable to
entirely delete the row in this case. Feedback from reviewers about this
behavior and about how to address it (if necessary), would be much appreciated.

[Bryan Pendleton]

Attached is rewrite_syscolperms_v2.diff. This patch proposal differs from
the v1 version only in that the code patch to DataDictionaryImpl.java
for DERBY-1847 has been removed from this patch.

This patch now applies cleanly against the head of trunk and is, I
believe, ready for commit. derbyall and suites.All pass with this patch.
Please review the patch as time permits.

[Bryan Pendleton]

Attached is rewrite_syscolperms_v3.diff. This patch is up to date with the head of trunk, and includes the change to EmptyDictionary.java (thanks Andrew!).

I intend to commit this patch, unless I find any problems in my last re-run of derbyall & suites.All.

If there are any review comments, please let me know.

[Bryan Pendleton]

Committed rewrite_syscolperms_v3.diff to subversion as revision 503550.