Skip to content

Commit 9a07ab0

Browse files
authored
feat(redshift): columns require an id attribute (under feature flag) (#24272)
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*
1 parent 856defb commit 9a07ab0

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+3985
-155
lines changed

packages/@aws-cdk/aws-redshift/README.md

+15
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,21 @@ new Table(this, 'Table', {
214214
});
215215
```
216216

217+
Table columns can also contain an `id` attribute, which can allow table columns to be renamed.
218+
219+
**NOTE** To use the `id` attribute, you must also enable the `@aws-cdk/aws-redshift:columnId` feature flag.
220+
221+
```ts fixture=cluster
222+
new Table(this, 'Table', {
223+
tableColumns: [
224+
{ id: 'col1', name: 'col1', dataType: 'varchar(4)' },
225+
{ id: 'col2', name: 'col2', dataType: 'float' }
226+
],
227+
cluster: cluster,
228+
databaseName: 'databaseName',
229+
});
230+
```
231+
217232
### Granting Privileges
218233

219234
You can give a user privileges to perform certain actions on a table by using the

packages/@aws-cdk/aws-redshift/lib/private/database-query-provider/table.ts

+30-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export async function handler(props: TableAndClusterProps, event: AWSLambda.Clou
1010
const tableNameSuffix = props.tableName.generateSuffix === 'true' ? `${event.RequestId.substring(0, 8)}` : '';
1111
const tableColumns = props.tableColumns;
1212
const tableAndClusterProps = props;
13+
const useColumnIds = props.useColumnIds;
1314

1415
if (event.RequestType === 'Create') {
1516
const tableName = await createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps);
@@ -23,6 +24,7 @@ export async function handler(props: TableAndClusterProps, event: AWSLambda.Clou
2324
tableNamePrefix,
2425
tableNameSuffix,
2526
tableColumns,
27+
useColumnIds,
2628
tableAndClusterProps,
2729
event.OldResourceProperties as TableAndClusterProps,
2830
);
@@ -77,6 +79,7 @@ async function updateTable(
7779
tableNamePrefix: string,
7880
tableNameSuffix: string,
7981
tableColumns: Column[],
82+
useColumnIds: boolean,
8083
tableAndClusterProps: TableAndClusterProps,
8184
oldResourceProperties: TableAndClusterProps,
8285
): Promise<string> {
@@ -94,19 +97,44 @@ async function updateTable(
9497

9598
const oldTableColumns = oldResourceProperties.tableColumns;
9699
const columnDeletions = oldTableColumns.filter(oldColumn => (
97-
tableColumns.every(column => oldColumn.name !== column.name)
100+
tableColumns.every(column => {
101+
if (useColumnIds) {
102+
return oldColumn.id ? oldColumn.id !== column.id : oldColumn.name !== column.name;
103+
}
104+
return oldColumn.name !== column.name;
105+
})
98106
));
99107
if (columnDeletions.length > 0) {
100108
alterationStatements.push(...columnDeletions.map(column => `ALTER TABLE ${tableName} DROP COLUMN ${column.name}`));
101109
}
102110

103111
const columnAdditions = tableColumns.filter(column => {
104-
return !oldTableColumns.some(oldColumn => column.name === oldColumn.name && column.dataType === oldColumn.dataType);
112+
return !oldTableColumns.some(oldColumn => {
113+
if (useColumnIds) {
114+
return oldColumn.id ? oldColumn.id === column.id : oldColumn.name === column.name;
115+
}
116+
return oldColumn.name === column.name;
117+
});
105118
}).map(column => `ADD ${column.name} ${column.dataType}`);
106119
if (columnAdditions.length > 0) {
107120
alterationStatements.push(...columnAdditions.map(addition => `ALTER TABLE ${tableName} ${addition}`));
108121
}
109122

123+
if (useColumnIds) {
124+
const columnNameUpdates = tableColumns.reduce((updates, column) => {
125+
const oldColumn = oldTableColumns.find(oldCol => oldCol.id && oldCol.id === column.id);
126+
if (oldColumn && oldColumn.name !== column.name) {
127+
updates[oldColumn.name] = column.name;
128+
}
129+
return updates;
130+
}, {} as Record<string, string>);
131+
if (Object.keys(columnNameUpdates).length > 0) {
132+
alterationStatements.push(...Object.entries(columnNameUpdates).map(([oldName, newName]) => (
133+
`ALTER TABLE ${tableName} RENAME COLUMN ${oldName} TO ${newName}`
134+
)));
135+
}
136+
}
137+
110138
const oldDistStyle = oldResourceProperties.distStyle;
111139
if ((!oldDistStyle && tableAndClusterProps.distStyle) ||
112140
(oldDistStyle && !tableAndClusterProps.distStyle)) {

packages/@aws-cdk/aws-redshift/lib/private/handler-props.ts

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export interface TableHandlerProps {
2121
readonly distStyle?: TableDistStyle;
2222
readonly sortStyle: TableSortStyle;
2323
readonly tableComment?: string;
24+
readonly useColumnIds: boolean;
2425
}
2526

2627
export interface TablePrivilege {

packages/@aws-cdk/aws-redshift/lib/table.ts

+29-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
/* eslint-disable import/no-extraneous-dependencies */
12
import * as cdk from '@aws-cdk/core';
3+
import { REDSHIFT_COLUMN_ID } from '@aws-cdk/cx-api';
24
import { Construct, IConstruct } from 'constructs';
35
import { ICluster } from './cluster';
46
import { DatabaseOptions } from './database-options';
@@ -55,9 +57,18 @@ export enum TableAction {
5557
*/
5658
export interface Column {
5759
/**
58-
* The unique name/identifier of the column.
60+
* The unique identifier of the column.
5961
*
60-
* **NOTE**. After deploying this column, you cannot change its name. Doing so will cause the column to be dropped and recreated.
62+
* This is not the name of the column, and renaming this identifier will cause a new column to be created and the old column to be dropped.
63+
*
64+
* **NOTE** - This field will be set, however, only by setting the `@aws-cdk/aws-redshift:columnId` feature flag will this field be used.
65+
*
66+
* @default - the column name is used as the identifier
67+
*/
68+
readonly id?: string;
69+
70+
/**
71+
* The name of the column. This will appear on Amazon Redshift.
6172
*/
6273
readonly name: string;
6374

@@ -217,6 +228,7 @@ export class Table extends TableBase {
217228
constructor(scope: Construct, id: string, props: TableProps) {
218229
super(scope, id);
219230

231+
this.addColumnIds(props.tableColumns);
220232
this.validateDistKeyColumns(props.tableColumns);
221233
if (props.distStyle) {
222234
this.validateDistStyle(props.distStyle, props.tableColumns);
@@ -229,6 +241,8 @@ export class Table extends TableBase {
229241
this.cluster = props.cluster;
230242
this.databaseName = props.databaseName;
231243

244+
const useColumnIds = !!cdk.FeatureFlags.of(this).isEnabled(REDSHIFT_COLUMN_ID);
245+
232246
this.resource = new DatabaseQuery<TableHandlerProps>(this, 'Resource', {
233247
removalPolicy: cdk.RemovalPolicy.RETAIN,
234248
...props,
@@ -242,6 +256,7 @@ export class Table extends TableBase {
242256
distStyle: props.distStyle,
243257
sortStyle: props.sortStyle ?? this.getDefaultSortStyle(props.tableColumns),
244258
tableComment: props.tableComment,
259+
useColumnIds,
245260
},
246261
});
247262

@@ -297,6 +312,18 @@ export class Table extends TableBase {
297312
const sortKeyColumns = getSortKeyColumns(columns);
298313
return (sortKeyColumns.length === 0) ? TableSortStyle.AUTO : TableSortStyle.COMPOUND;
299314
}
315+
316+
private addColumnIds(columns: Column[]): void {
317+
const columnIds = new Set<string>();
318+
for (const column of columns) {
319+
if (column.id) {
320+
if (columnIds.has(column.id)) {
321+
throw new Error(`Column id '${column.id}' is not unique.`);
322+
}
323+
columnIds.add(column.id);
324+
}
325+
}
326+
}
300327
}
301328

302329
/**

packages/@aws-cdk/aws-redshift/test/database-query-provider/table.test.ts

+54
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const adminUserArn = 'adminUserArn';
1919
const databaseName = 'databaseName';
2020
const physicalResourceId = 'PhysicalResourceId';
2121
const resourceProperties: ResourcePropertiesType = {
22+
useColumnIds: true,
2223
tableName: {
2324
prefix: tableNamePrefix,
2425
generateSuffix: 'true',
@@ -270,6 +271,59 @@ describe('update', () => {
270271
}));
271272
});
272273

274+
describe('column name', () => {
275+
test('does not replace if column name changed', async () => {
276+
const newEvent = {
277+
...event,
278+
OldResourceProperties: {
279+
...event.OldResourceProperties,
280+
tableColumns: [
281+
{ id: 'col1', name: 'col1', dataType: 'varchar(1)' },
282+
],
283+
},
284+
};
285+
const newTableColumnName = 'col2';
286+
const newResourceProperties: ResourcePropertiesType = {
287+
...resourceProperties,
288+
tableColumns: [
289+
{ id: 'col1', name: newTableColumnName, dataType: 'varchar(1)' },
290+
],
291+
};
292+
293+
await expect(manageTable(newResourceProperties, newEvent)).resolves.toMatchObject({
294+
PhysicalResourceId: physicalResourceId,
295+
});
296+
expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
297+
Sql: `ALTER TABLE ${physicalResourceId} RENAME COLUMN col1 TO ${newTableColumnName}`,
298+
}));
299+
});
300+
301+
test('does not replace if column id assigned, from undefined', async () => {
302+
const newEvent = {
303+
...event,
304+
OldResourceProperties: {
305+
...event.OldResourceProperties,
306+
tableColumns: [
307+
{ name: 'col1', dataType: 'varchar(1)' },
308+
],
309+
},
310+
};
311+
const newResourceProperties: ResourcePropertiesType = {
312+
...resourceProperties,
313+
tableColumns: [
314+
{ id: 'col1', name: 'col1', dataType: 'varchar(1)' },
315+
],
316+
};
317+
318+
await expect(manageTable(newResourceProperties, newEvent)).resolves.toMatchObject({
319+
PhysicalResourceId: physicalResourceId,
320+
});
321+
expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
322+
Sql: `ALTER TABLE ${physicalResourceId} RENAME COLUMN col1 TO col1`,
323+
}));
324+
});
325+
});
326+
273327
describe('distStyle and distKey', () => {
274328
test('replaces if distStyle is added', async () => {
275329
const newResourceProperties: ResourcePropertiesType = {

0 commit comments

Comments
 (0)