Skip to content

Commit

Permalink
feat(redshift): columns require an id attribute (under feature flag) (#…
Browse files Browse the repository at this point in the history
…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*
  • Loading branch information
Rizxcviii committed Mar 2, 2023
1 parent 856defb commit 9a07ab0
Show file tree
Hide file tree
Showing 48 changed files with 3,985 additions and 155 deletions.
15 changes: 15 additions & 0 deletions packages/@aws-cdk/aws-redshift/README.md
Expand Up @@ -214,6 +214,21 @@ new Table(this, 'Table', {
});
```

Table columns can also contain an `id` attribute, which can allow table columns to be renamed.

**NOTE** To use the `id` attribute, you must also enable the `@aws-cdk/aws-redshift:columnId` feature flag.

```ts fixture=cluster
new Table(this, 'Table', {
tableColumns: [
{ id: 'col1', name: 'col1', dataType: 'varchar(4)' },
{ id: 'col2', name: 'col2', dataType: 'float' }
],
cluster: cluster,
databaseName: 'databaseName',
});
```

### Granting Privileges

You can give a user privileges to perform certain actions on a table by using the
Expand Down
Expand Up @@ -10,6 +10,7 @@ export async function handler(props: TableAndClusterProps, event: AWSLambda.Clou
const tableNameSuffix = props.tableName.generateSuffix === 'true' ? `${event.RequestId.substring(0, 8)}` : '';
const tableColumns = props.tableColumns;
const tableAndClusterProps = props;
const useColumnIds = props.useColumnIds;

if (event.RequestType === 'Create') {
const tableName = await createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps);
Expand All @@ -23,6 +24,7 @@ export async function handler(props: TableAndClusterProps, event: AWSLambda.Clou
tableNamePrefix,
tableNameSuffix,
tableColumns,
useColumnIds,
tableAndClusterProps,
event.OldResourceProperties as TableAndClusterProps,
);
Expand Down Expand Up @@ -77,6 +79,7 @@ async function updateTable(
tableNamePrefix: string,
tableNameSuffix: string,
tableColumns: Column[],
useColumnIds: boolean,
tableAndClusterProps: TableAndClusterProps,
oldResourceProperties: TableAndClusterProps,
): Promise<string> {
Expand All @@ -94,19 +97,44 @@ async function updateTable(

const oldTableColumns = oldResourceProperties.tableColumns;
const columnDeletions = oldTableColumns.filter(oldColumn => (
tableColumns.every(column => oldColumn.name !== column.name)
tableColumns.every(column => {
if (useColumnIds) {
return oldColumn.id ? oldColumn.id !== column.id : oldColumn.name !== column.name;
}
return oldColumn.name !== column.name;
})
));
if (columnDeletions.length > 0) {
alterationStatements.push(...columnDeletions.map(column => `ALTER TABLE ${tableName} DROP COLUMN ${column.name}`));
}

const columnAdditions = tableColumns.filter(column => {
return !oldTableColumns.some(oldColumn => column.name === oldColumn.name && column.dataType === oldColumn.dataType);
return !oldTableColumns.some(oldColumn => {
if (useColumnIds) {
return oldColumn.id ? oldColumn.id === column.id : oldColumn.name === column.name;
}
return oldColumn.name === column.name;
});
}).map(column => `ADD ${column.name} ${column.dataType}`);
if (columnAdditions.length > 0) {
alterationStatements.push(...columnAdditions.map(addition => `ALTER TABLE ${tableName} ${addition}`));
}

if (useColumnIds) {
const columnNameUpdates = tableColumns.reduce((updates, column) => {
const oldColumn = oldTableColumns.find(oldCol => oldCol.id && oldCol.id === column.id);
if (oldColumn && oldColumn.name !== column.name) {
updates[oldColumn.name] = column.name;
}
return updates;
}, {} as Record<string, string>);
if (Object.keys(columnNameUpdates).length > 0) {
alterationStatements.push(...Object.entries(columnNameUpdates).map(([oldName, newName]) => (
`ALTER TABLE ${tableName} RENAME COLUMN ${oldName} TO ${newName}`
)));
}
}

const oldDistStyle = oldResourceProperties.distStyle;
if ((!oldDistStyle && tableAndClusterProps.distStyle) ||
(oldDistStyle && !tableAndClusterProps.distStyle)) {
Expand Down
Expand Up @@ -21,6 +21,7 @@ export interface TableHandlerProps {
readonly distStyle?: TableDistStyle;
readonly sortStyle: TableSortStyle;
readonly tableComment?: string;
readonly useColumnIds: boolean;
}

export interface TablePrivilege {
Expand Down
31 changes: 29 additions & 2 deletions packages/@aws-cdk/aws-redshift/lib/table.ts
@@ -1,4 +1,6 @@
/* eslint-disable import/no-extraneous-dependencies */
import * as cdk from '@aws-cdk/core';
import { REDSHIFT_COLUMN_ID } from '@aws-cdk/cx-api';
import { Construct, IConstruct } from 'constructs';
import { ICluster } from './cluster';
import { DatabaseOptions } from './database-options';
Expand Down Expand Up @@ -55,9 +57,18 @@ export enum TableAction {
*/
export interface Column {
/**
* The unique name/identifier of the column.
* The unique identifier of the column.
*
* **NOTE**. After deploying this column, you cannot change its name. Doing so will cause the column to be dropped and recreated.
* 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.
*
* **NOTE** - This field will be set, however, only by setting the `@aws-cdk/aws-redshift:columnId` feature flag will this field be used.
*
* @default - the column name is used as the identifier
*/
readonly id?: string;

/**
* The name of the column. This will appear on Amazon Redshift.
*/
readonly name: string;

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

this.addColumnIds(props.tableColumns);
this.validateDistKeyColumns(props.tableColumns);
if (props.distStyle) {
this.validateDistStyle(props.distStyle, props.tableColumns);
Expand All @@ -229,6 +241,8 @@ export class Table extends TableBase {
this.cluster = props.cluster;
this.databaseName = props.databaseName;

const useColumnIds = !!cdk.FeatureFlags.of(this).isEnabled(REDSHIFT_COLUMN_ID);

this.resource = new DatabaseQuery<TableHandlerProps>(this, 'Resource', {
removalPolicy: cdk.RemovalPolicy.RETAIN,
...props,
Expand All @@ -242,6 +256,7 @@ export class Table extends TableBase {
distStyle: props.distStyle,
sortStyle: props.sortStyle ?? this.getDefaultSortStyle(props.tableColumns),
tableComment: props.tableComment,
useColumnIds,
},
});

Expand Down Expand Up @@ -297,6 +312,18 @@ export class Table extends TableBase {
const sortKeyColumns = getSortKeyColumns(columns);
return (sortKeyColumns.length === 0) ? TableSortStyle.AUTO : TableSortStyle.COMPOUND;
}

private addColumnIds(columns: Column[]): void {
const columnIds = new Set<string>();
for (const column of columns) {
if (column.id) {
if (columnIds.has(column.id)) {
throw new Error(`Column id '${column.id}' is not unique.`);
}
columnIds.add(column.id);
}
}
}
}

/**
Expand Down
Expand Up @@ -19,6 +19,7 @@ const adminUserArn = 'adminUserArn';
const databaseName = 'databaseName';
const physicalResourceId = 'PhysicalResourceId';
const resourceProperties: ResourcePropertiesType = {
useColumnIds: true,
tableName: {
prefix: tableNamePrefix,
generateSuffix: 'true',
Expand Down Expand Up @@ -270,6 +271,59 @@ describe('update', () => {
}));
});

describe('column name', () => {
test('does not replace if column name changed', async () => {
const newEvent = {
...event,
OldResourceProperties: {
...event.OldResourceProperties,
tableColumns: [
{ id: 'col1', name: 'col1', dataType: 'varchar(1)' },
],
},
};
const newTableColumnName = 'col2';
const newResourceProperties: ResourcePropertiesType = {
...resourceProperties,
tableColumns: [
{ id: 'col1', name: newTableColumnName, dataType: 'varchar(1)' },
],
};

await expect(manageTable(newResourceProperties, newEvent)).resolves.toMatchObject({
PhysicalResourceId: physicalResourceId,
});
expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
Sql: `ALTER TABLE ${physicalResourceId} RENAME COLUMN col1 TO ${newTableColumnName}`,
}));
});

test('does not replace if column id assigned, from undefined', async () => {
const newEvent = {
...event,
OldResourceProperties: {
...event.OldResourceProperties,
tableColumns: [
{ name: 'col1', dataType: 'varchar(1)' },
],
},
};
const newResourceProperties: ResourcePropertiesType = {
...resourceProperties,
tableColumns: [
{ id: 'col1', name: 'col1', dataType: 'varchar(1)' },
],
};

await expect(manageTable(newResourceProperties, newEvent)).resolves.toMatchObject({
PhysicalResourceId: physicalResourceId,
});
expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
Sql: `ALTER TABLE ${physicalResourceId} RENAME COLUMN col1 TO col1`,
}));
});
});

describe('distStyle and distKey', () => {
test('replaces if distStyle is added', async () => {
const newResourceProperties: ResourcePropertiesType = {
Expand Down

0 comments on commit 9a07ab0

Please sign in to comment.