-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(redshift): Add support for distStyle, distKey, sortStyle and sortKey to Table #17135
Changes from 5 commits
8d4bb41
c1d5970
be5da70
8a16a4e
8a7fa87
e153971
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,29 @@ | ||
/* eslint-disable-next-line import/no-unresolved */ | ||
import * as AWSLambda from 'aws-lambda'; | ||
import { Column } from '../../table'; | ||
import { TableHandlerProps } from '../handler-props'; | ||
import { ClusterProps, executeStatement } from './util'; | ||
import { ClusterProps, TableAndClusterProps, TableSortStyle } from './types'; | ||
import { areColumnsEqual, executeStatement, getDistKeyColumn, getSortKeyColumns } from './util'; | ||
|
||
export async function handler(props: TableHandlerProps & ClusterProps, event: AWSLambda.CloudFormationCustomResourceEvent) { | ||
export async function handler(props: TableAndClusterProps, event: AWSLambda.CloudFormationCustomResourceEvent) { | ||
const tableNamePrefix = props.tableName.prefix; | ||
const tableNameSuffix = props.tableName.generateSuffix ? `${event.RequestId.substring(0, 8)}` : ''; | ||
const tableColumns = props.tableColumns; | ||
const clusterProps = props; | ||
const tableAndClusterProps = props; | ||
|
||
if (event.RequestType === 'Create') { | ||
const tableName = await createTable(tableNamePrefix, tableNameSuffix, tableColumns, clusterProps); | ||
const tableName = await createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps); | ||
return { PhysicalResourceId: tableName }; | ||
} else if (event.RequestType === 'Delete') { | ||
await dropTable(event.PhysicalResourceId, clusterProps); | ||
await dropTable(event.PhysicalResourceId, tableAndClusterProps); | ||
return; | ||
} else if (event.RequestType === 'Update') { | ||
const tableName = await updateTable( | ||
event.PhysicalResourceId, | ||
tableNamePrefix, | ||
tableNameSuffix, | ||
tableColumns, | ||
clusterProps, | ||
event.OldResourceProperties as TableHandlerProps & ClusterProps, | ||
tableAndClusterProps, | ||
event.OldResourceProperties as TableAndClusterProps, | ||
); | ||
return { PhysicalResourceId: tableName }; | ||
} else { | ||
|
@@ -32,10 +32,33 @@ export async function handler(props: TableHandlerProps & ClusterProps, event: AW | |
} | ||
} | ||
|
||
async function createTable(tableNamePrefix: string, tableNameSuffix: string, tableColumns: Column[], clusterProps: ClusterProps): Promise<string> { | ||
async function createTable( | ||
tableNamePrefix: string, | ||
tableNameSuffix: string, | ||
tableColumns: Column[], | ||
tableAndClusterProps: TableAndClusterProps, | ||
): Promise<string> { | ||
const tableName = tableNamePrefix + tableNameSuffix; | ||
const tableColumnsString = tableColumns.map(column => `${column.name} ${column.dataType}`).join(); | ||
await executeStatement(`CREATE TABLE ${tableName} (${tableColumnsString})`, clusterProps); | ||
|
||
let statement = `CREATE TABLE ${tableName} (${tableColumnsString})`; | ||
|
||
if (tableAndClusterProps.distStyle) { | ||
statement += ` DISTSTYLE ${tableAndClusterProps.distStyle}`; | ||
} | ||
|
||
const distKeyColumn = getDistKeyColumn(tableColumns); | ||
if (distKeyColumn) { | ||
statement += ` DISTKEY(${distKeyColumn.name})`; | ||
} | ||
|
||
const sortKeyColumns = getSortKeyColumns(tableColumns); | ||
if (sortKeyColumns.length > 0) { | ||
const sortKeyColumnsString = getSortKeyColumnsString(sortKeyColumns); | ||
statement += ` ${tableAndClusterProps.sortStyle} SORTKEY(${sortKeyColumnsString})`; | ||
} | ||
|
||
await executeStatement(statement, tableAndClusterProps); | ||
return tableName; | ||
} | ||
|
||
|
@@ -48,28 +71,79 @@ async function updateTable( | |
tableNamePrefix: string, | ||
tableNameSuffix: string, | ||
tableColumns: Column[], | ||
clusterProps: ClusterProps, | ||
oldResourceProperties: TableHandlerProps & ClusterProps, | ||
tableAndClusterProps: TableAndClusterProps, | ||
oldResourceProperties: TableAndClusterProps, | ||
): Promise<string> { | ||
const alterationStatements: string[] = []; | ||
|
||
const oldClusterProps = oldResourceProperties; | ||
if (clusterProps.clusterName !== oldClusterProps.clusterName || clusterProps.databaseName !== oldClusterProps.databaseName) { | ||
return createTable(tableNamePrefix, tableNameSuffix, tableColumns, clusterProps); | ||
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, clusterProps); | ||
return createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps); | ||
} | ||
|
||
const oldTableColumns = oldResourceProperties.tableColumns; | ||
if (!oldTableColumns.every(oldColumn => tableColumns.some(column => column.name === oldColumn.name && column.dataType === oldColumn.dataType))) { | ||
return createTable(tableNamePrefix, tableNameSuffix, tableColumns, clusterProps); | ||
return createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps); | ||
} | ||
|
||
const additions = tableColumns.filter(column => { | ||
const columnAdditions = tableColumns.filter(column => { | ||
return !oldTableColumns.some(oldColumn => column.name === oldColumn.name && column.dataType === oldColumn.dataType); | ||
}).map(column => `ADD ${column.name} ${column.dataType}`); | ||
await Promise.all(additions.map(addition => executeStatement(`ALTER TABLE ${tableName} ${addition}`, clusterProps))); | ||
if (columnAdditions.length > 0) { | ||
alterationStatements.push(...columnAdditions.map(addition => `ALTER TABLE ${tableName} ${addition}`)); | ||
} | ||
|
||
const oldDistStyle = oldResourceProperties.distStyle; | ||
if ((!oldDistStyle && tableAndClusterProps.distStyle) || | ||
(oldDistStyle && !tableAndClusterProps.distStyle)) { | ||
return createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps); | ||
} else if (oldDistStyle !== tableAndClusterProps.distStyle) { | ||
alterationStatements.push(`ALTER TABLE ${tableName} ALTER DISTSTYLE ${tableAndClusterProps.distStyle}`); | ||
} | ||
|
||
const oldDistKey = getDistKeyColumn(oldTableColumns)?.name; | ||
const newDistKey = getDistKeyColumn(tableColumns)?.name; | ||
if ((!oldDistKey && newDistKey ) || (oldDistKey && !newDistKey)) { | ||
return createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps); | ||
} else if (oldDistKey !== newDistKey) { | ||
alterationStatements.push(`ALTER TABLE ${tableName} ALTER DISTKEY ${newDistKey}`); | ||
} | ||
|
||
const oldSortKeyColumns = getSortKeyColumns(oldTableColumns); | ||
const newSortKeyColumns = getSortKeyColumns(tableColumns); | ||
const oldSortStyle = oldResourceProperties.sortStyle; | ||
const newSortStyle = tableAndClusterProps.sortStyle; | ||
if ((oldSortStyle === newSortStyle && !areColumnsEqual(oldSortKeyColumns, newSortKeyColumns)) | ||
|| (oldSortStyle !== newSortStyle)) { | ||
switch (newSortStyle) { | ||
case TableSortStyle.INTERLEAVED: | ||
Comment on lines
+123
to
+124
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you link to the documentation here describing why one type of switch requires a new table, and others can use an ALTER command? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this doc: https://docs.aws.amazon.com/redshift/latest/dg/r_ALTER_TABLE.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome; can you add that as a comment here, just so its clear in-source? |
||
// INTERLEAVED sort key addition requires replacement. | ||
// https://docs.aws.amazon.com/redshift/latest/dg/r_ALTER_TABLE.html | ||
return createTable(tableNamePrefix, tableNameSuffix, tableColumns, tableAndClusterProps); | ||
|
||
case TableSortStyle.COMPOUND: { | ||
const sortKeyColumnsString = getSortKeyColumnsString(newSortKeyColumns); | ||
alterationStatements.push(`ALTER TABLE ${tableName} ALTER ${newSortStyle} SORTKEY(${sortKeyColumnsString})`); | ||
break; | ||
} | ||
|
||
case TableSortStyle.AUTO: { | ||
alterationStatements.push(`ALTER TABLE ${tableName} ALTER SORTKEY ${newSortStyle}`); | ||
break; | ||
} | ||
} | ||
} | ||
|
||
await Promise.all(alterationStatements.map(statement => executeStatement(statement, tableAndClusterProps))); | ||
njlynch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return tableName; | ||
} | ||
|
||
function getSortKeyColumnsString(sortKeyColumns: Column[]) { | ||
return sortKeyColumns.map(column => column.name).join(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import { DatabaseQueryHandlerProps, TableHandlerProps } from '../handler-props'; | ||
ayush987goyal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export type ClusterProps = Omit<DatabaseQueryHandlerProps, 'handler'>; | ||
export type TableAndClusterProps = TableHandlerProps & ClusterProps; | ||
|
||
/** | ||
* The sort style of a table. | ||
* This has been duplicated here to exporting private types. | ||
*/ | ||
export enum TableSortStyle { | ||
/** | ||
* Amazon Redshift assigns an optimal sort key based on the table data. | ||
*/ | ||
AUTO = 'AUTO', | ||
|
||
/** | ||
* Specifies that the data is sorted using a compound key made up of all of the listed columns, | ||
* in the order they are listed. | ||
*/ | ||
COMPOUND = 'COMPOUND', | ||
|
||
/** | ||
* Specifies that the data is sorted using an interleaved sort key. | ||
*/ | ||
INTERLEAVED = 'INTERLEAVED', | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you educate me? What happens in this case? We have a table already with a name (e.g., MyTable) and we add a column. Then we execute a
CREATE TABLE MyTable...
statement with a slightly different set of columns. Wouldn't that fail due to the table name already existing? I'm wondering the same about all of the otherreturn createTable
calls further down this method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the create table call would fail here in case the table needs replacing. The customer would then have a choice of manually dropping the table and/or updating the columns to avoid replacement.