feat(redshift): columns require an id attribute (under feature flag)

[Rizxcviii]

Adds an id attribute to retain tables on changing of the column name. This will essentially fire an ALTER statement to rename the column, whilst persisting the id, so columns cannot be dropped.

Closes #24234.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

[TheRealAmazonKendra]

Even though this is module isn't stable, I think we need a feature flag on this. The change itself will cause a replacement and I don't want customers to unintentionally lose data due to a change that is meant to prevent that very thing.

[Rizxcviii]

The change itself will cause a replacement and I don't want customers to unintentionally lose data due to a change that is meant to prevent that very thing.

@TheRealAmazonKendra I don't think it will, since I perform a check to see if the oldColumn.id is defined. If the attribute is not defined by the developer, then we compare using the current Boolean expression.
For example,

column => oldColumn.name !== column.name

now becomes

column => oldColumn.id ? oldColumn.id !== column.id : oldColumn.name !== column.name

This should ensure backwards compatibility, unless I am mistaken.

[MrArnoldPalmer]

@Rizxcviii so that part in the custom resource logic seems to be non-breaking but the new id prop on column is not optional, so users would have to provide an ID where there was a name before. Will that still not cause replacement and if not then the ID just isn't being handled for users with existing columns at all?

[Rizxcviii]

@MrArnoldPalmer
Are you referring to the initial assignment of the id when users are forced to give an id to a column? If so, then I think the logic within the code will handle this correctly? Within the logic, the newly assigned id will belong to the column object, but oldColumn.id will still be undefined, since this was not specified in the previous deployment.

Or are you referring to the id "replacing" the name prop. The purpose of the id prop is to be persistent, so the id will never change throughout the initial assignment. My logic is to keep the id static, so thereafter whenever the name changes, the column name on redshift can be altered, whilst still persisting the existing column.
The id prop will take over the responsibility of the names responsibility of being persistent, to maintain column data; but will allow the name of a column to be changed, essentially renaming it on Redshift.
Again, if there is another side effect that I am overlooking, then please correct me.

[Rizxcviii]

Actually, now that I'm thinking about it, you are right. If within that initial deployment, the user assumes that the id is persistent, they may change the name attribute, which will effectively drop their column. I will add a feature flag to address the issue @TheRealAmazonKendra @MrArnoldPalmer

[TheRealAmazonKendra]

Actually, now that I'm thinking about it, you are right. If within that initial deployment, the user assumes that the id is persistent, they may change the name attribute, which will effectively drop their column. I will add a feature flag to address the issue @TheRealAmazonKendra @MrArnoldPalmer

Great, thank you!

Pull request has been modified.

[comcalvi]

Just one minor comment, but this looks very good!

Pull request has been modified.

[MrArnoldPalmer]

Just a test change.

Pull request has been modified.

[MrArnoldPalmer]

LGTM! @comcalvi added dnm for you to check out again if needed.

Pull request has been modified.

[comcalvi]

lgtm! Nice work!

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 89bcc7c
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).