fix(redshift): tables were dropped on table name change (#24308)

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*
aws · Dec 7, 2023 · 7ac237b · 7ac237b
1 parent 569593c
commit 7ac237b
Show file tree

Hide file tree

Showing 30 changed files with 34,120 additions and 519 deletions.
diff --git a/packages/@aws-cdk/aws-redshift-alpha/README.md b/packages/@aws-cdk/aws-redshift-alpha/README.md
@@ -166,6 +166,18 @@ new Table(this, 'Table', {
 });
 ```
 
+Tables greater than v2.114.1 can have their table name changed, for versions <= v2.114.1, this would not be possible.
+Therefore, changing of table names for <= v2.114.1 have been disabled.
+
+```ts fixture=cluster
+new Table(this, 'Table', {
+  tableName: 'oldTableName' // This value can be change for versions greater than v2.114.1
+  tableColumns: [{ name: 'col1', dataType: 'varchar(4)' }, { name: 'col2', dataType: 'float' }],
+  cluster: cluster,
+  databaseName: 'databaseName',
+});
+```
+
 The table can be configured to have distStyle attribute and a distKey column:
 
 ```ts fixture=cluster

diff --git a/packages/@aws-cdk/aws-redshift-alpha/lib/private/database-query-provider/table.ts b/packages/@aws-cdk/aws-redshift-alpha/lib/private/database-query-provider/table.ts
@@ -1,34 +1,41 @@
 /* eslint-disable-next-line import/no-unresolved */
 import * as AWSLambda from 'aws-lambda';
+import { Column } from '../../table';
 import { executeStatement } from './redshift-data';
 import { ClusterProps, TableAndClusterProps, TableSortStyle } from './types';
-import { areColumnsEqual, getDistKeyColumn, getSortKeyColumns } from './util';
-import { Column } from '../../table';
+import { areColumnsEqual, getDistKeyColumn, getSortKeyColumns, makePhysicalId } from './util';
 
 export async function handler(props: TableAndClusterProps, event: AWSLambda.CloudFormationCustomResourceEvent) {
   const tableNamePrefix = props.tableName.prefix;
-  const tableNameSuffix = props.tableName.generateSuffix === 'true' ? `${event.RequestId.substring(0, 8)}` : '';
+  const getTableNameSuffix = (generateSuffix: string) => generateSuffix === 'true' ? `${event.StackId.substring(event.StackId.length - 12)}` : '';
   const tableColumns = props.tableColumns;
   const tableAndClusterProps = props;
   const useColumnIds = props.useColumnIds;
+  let tableName = tableNamePrefix + getTableNameSuffix(props.tableName.generateSuffix);
 
   if (event.RequestType === 'Create') {
-    const tableName = await createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps);
-    return { PhysicalResourceId: tableName };
+    tableName = await createTable(tableNamePrefix, getTableNameSuffix(props.tableName.generateSuffix), tableColumns, tableAndClusterProps);
+    return { PhysicalResourceId: makePhysicalId(tableNamePrefix, tableAndClusterProps, event.StackId.substring(event.StackId.length - 12)) };
   } else if (event.RequestType === 'Delete') {
-    await dropTable(event.PhysicalResourceId, tableAndClusterProps);
+    await dropTable(
+      event.PhysicalResourceId.includes(event.StackId.substring(event.StackId.length - 12)) ? tableName : event.PhysicalResourceId,
+      tableAndClusterProps,
+    );
     return;
   } else if (event.RequestType === 'Update') {
-    const tableName = await updateTable(
-      event.PhysicalResourceId,
+    const isTableV2 = event.PhysicalResourceId.includes(event.StackId.substring(event.StackId.length - 12));
+    const oldTableName = event.OldResourceProperties.tableName.prefix + getTableNameSuffix(event.OldResourceProperties.tableName.generateSuffix);
+    tableName = await updateTable(
+      isTableV2 ? oldTableName : event.PhysicalResourceId,
       tableNamePrefix,
-      tableNameSuffix,
+      getTableNameSuffix(props.tableName.generateSuffix),
       tableColumns,
       useColumnIds,
       tableAndClusterProps,
       event.OldResourceProperties as TableAndClusterProps,
+      isTableV2,
     );
-    return { PhysicalResourceId: tableName };
+    return { PhysicalResourceId: event.PhysicalResourceId };
   } else {
     /* eslint-disable-next-line dot-notation */
     throw new Error(`Unrecognized event type: ${event['RequestType']}`);
@@ -87,19 +94,16 @@ async function updateTable(
   useColumnIds: boolean,
   tableAndClusterProps: TableAndClusterProps,
   oldResourceProperties: TableAndClusterProps,
+  isTableV2: boolean,
 ): Promise<string> {
   const alterationStatements: string[] = [];
+  const newTableName = tableNamePrefix + tableNameSuffix;
 
   const oldClusterProps = oldResourceProperties;
   if (tableAndClusterProps.clusterName !== oldClusterProps.clusterName || tableAndClusterProps.databaseName !== oldClusterProps.databaseName) {
     return createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps);
   }
 
-  const oldTableNamePrefix = oldResourceProperties.tableName.prefix;
-  if (tableNamePrefix !== oldTableNamePrefix) {
-    return createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps);
-  }
-
   const oldTableColumns = oldResourceProperties.tableColumns;
   const columnDeletions = oldTableColumns.filter(oldColumn => (
     tableColumns.every(column => {
@@ -208,6 +212,14 @@ async function updateTable(
 
   await Promise.all(alterationStatements.map(statement => executeStatement(statement, tableAndClusterProps)));
 
+  if (isTableV2) {
+    const oldTableNamePrefix = oldResourceProperties.tableName.prefix;
+    if (tableNamePrefix !== oldTableNamePrefix) {
+      await executeStatement(`ALTER TABLE ${tableName} RENAME TO ${newTableName}`, tableAndClusterProps);
+      return tableNamePrefix + tableNameSuffix;
+    }
+  }
+
   return tableName;
 }