-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
aws-redshift: table is dropped on table name change #24246
Comments
I think you are right. When the provided tableName is changed and stack re-deployed, the following logic will be executed by creating a new table, which should be avoid and execute an ALTER statement instead. aws-cdk/packages/@aws-cdk/aws-redshift/lib/private/database-query-provider/table.ts Lines 91 to 93 in fdb0cf1
I am making this as a p2 and any PR submission will be highly appreciated. |
Currently blocked by #24874 |
…26955) To support solving #24246, there needs to first be a refactor of the UserTablePriviliges to instead record the table id. The reason being that new privileges would be granted/revoked on old tables that now have new names, since a change in table name caused an UPDATE action to be triggered. However the issue remains where, since the table has a new name, the grant/revoke action will be called on an invalid table name (i.e. the old table name). We now use the table id to track tables, therefore preventing UPDATE events to be triggered. This blocks #24308 This was originally PR #24874, however that had closed. @kaizencc requested changes, that I had added in here. This was originally PR #26558, however that had closed. @kaizencc requested changes, that I have already implemented previously. However, he did not review them. BREAKING CHANGE: the behavior of redshift tables has changed. UPDATE action will not be triggered on new table names and instead be triggered on table id changes. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Upon a refactor of the `UserTablePrivilieges` now using `tableId` to record changes, this fix will essentially allow table names to be changed. Closes #24246. BREAKING CHANGE: Further updates of the Redshift table will fail for existing tables, if the table name is changed. Therefore, changing the table name for existing Redshift tables have been disabled. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Describe the bug
When a table name changes, the entire table is dropped (if the removal policy is
DESTROY
). Amazon Redshift allows the table name to be altered, but the table is being dropped, even though the original table construct is still intact (the resource ID does not change).Expected Behavior
Table names should change
Current Behavior
A new redshift table is created, with the old table being dropped.
Provided a
DESTROY
removal policy, the table, and it's respective data will be deleted.Reproduction Steps
We then change the redshift table name, this will drop the redshift table, and create a new table.
Possible Solution
Persist the redshift table when table names change, given the use of the redshift data API, we could use
ALTER
statements to change the name of the table. Redundant tables that are no longer used in the Stack can still be "removed" by specifying a new id/removing the existing table construct in the code.Additional Information/Context
Obviously not all developers would add a
DESTROY
removal policy, but this was not the point of this issue. The point of the issue is the table not persisting through a table name change, but a new table being created. I am simply using aDESTROY
removal policy for the worst case scenario, as an example.Not quite sure if this is intended behaviour. However from my personal development experience, I would say I was surprised that tables were being dropped/being replaced in the stack.
The
cdk diff
command shows the following changesThe above shows to myself that we're just changing the table name, not creating a new construct
CDK CLI Version
2.65.0
Framework Version
2.65.0
Node.js Version
v16.16.0
OS
Amazon Linux 2
Language
Python
Language Version
3.7.16
Other information
This might be a feature request... If the original behaviour is intended behaviour.
The text was updated successfully, but these errors were encountered: