[POC] Migrating data with foreign key constraints #288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Manan007224
changed the title
shuhaowu
reviewed
Jun 16, 2021
refer to an issue in ghostferry instead of an internal one
Manan007224
force-pushed
the
table-creation-priority-order
branch
from
a408db1
to
988e6b2
Compare
shuhaowu
reviewed
Jun 17, 2021
| logrus.Info("creating databases and tables on target") | ||
| for _, tableName := range this.Ferry.Tables.GetTableListWithPriority(this.config.TablesToBeCreatedFirst) { | ||
| logger.Info("creating databases and tables on target") | ||
| var prioritzedTableNames []string |
There was a problem hiding this comment.
Suggested change
| var prioritzedTableNames []string | |
| var prioritizedTableNames []string |
| @@ -342,6 +342,75 @@ func (c TableSchemaCache) GetTableListWithPriority(priorityList []string) (prior | |||
| return | |||
| } | |||
|
|
|||
| func (c TableSchemaCache) GetTableCreationOrder(db *sql.DB) ([]string, error) { | |||
There was a problem hiding this comment.
Maybe change the name so it doesn't seem like a generic order that people should follow?
Suggested change
| func (c TableSchemaCache) GetTableCreationOrder(db *sql.DB) ([]string, error) { | |
| func (c TableSchemaCache) GetTableCreationOrderForFKConstraints(db *sql.DB) ([]string, error) { |
|
|
||
| for table := range c { | ||
| if _, found := visitedTables[table]; !found { | ||
| visitedTables, tableOrder = getTableCreationOrderUtil(visitedTables, tableOrder, tableMap, table) |
There was a problem hiding this comment.
Can you document how this algorithm is supposed to work (in either comments attached to a function here, or via a separate document)? It looks feasible to trace through myself, but it's better for a non-trivial algorithm to have its intent documented, so reviewers can focus on if the implementation is right or wrong, not reverse engineering what you're trying to do.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an experimental feature in ghostferry to migrate tables with foreign key constraints.
Most of the features are explained in this doc - https://github.com/Shopify/ghostferry/blob/4ea968d2aa702593a4c1ee0f71260333ed88982d/docs/source/copydbforeignkey.rst