-
Notifications
You must be signed in to change notification settings - Fork 30
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
Refactor Redshift Append #539
Conversation
@nathan-artie This unblocks the append mode for reader. We just need to rewrite the dedupe query and we can enable it for Redshift deploys. |
clients/redshift/writes.go
Outdated
fmt.Sprintf(`ALTER TABLE %s APPEND FROM %s;`, tableID.FullyQualifiedName(), temporaryTableID.FullyQualifiedName()), | ||
) | ||
return err | ||
return shared.Append(s, tableData, types.AppendOpts{TempTableID: tableID}) |
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.
Is this comment outdated now:
transfer/lib/destination/types/types.go
Line 48 in 4153191
// Redshift then has a separate step after `shared.Append(...)` to merge the two tables together. |
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 it is. I'm removing that whole option
Changes
Instead of creating a separate staging table and then running
ALTER TABLE APPEND
, we can just directly load this data into the Redshift table.By doing this, we don't have to worry about the edge cases of ALTER TABLE APPEND like having distribution mode clashes.