Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

keep the table order of the table whitelist #73

Merged
merged 2 commits into from
Jul 19, 2019

Conversation

Kosta-Github
Copy link
Contributor

When providing a table whitelist the order of that whitelist should be kept when dumping the tables.

This helps if dumping the database without table locks and having tables with foreign key references into other tables. By carefully defining the order of tables you can still achieve a consistent dump without violating the referential integrity by first dumping the table containing the foreign key, followed by the table which is the target of the reference.

Copy link
Owner

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this give you a false sense of security unless your table structure is an acyclic graph?
As soon as there are potential cycles in your structure, this method is no safer.

I would probably go as far as to say that using this tool to dump a live production database with no locks should be classified as an unsupported use case because there are so many potential foot-guns.

src/getTables.ts Outdated Show resolved Hide resolved
src/getTables.ts Outdated
);
tables = restrictedTables
.map(tableName => actualTables.find(t => t.name === tableName)) // keeping the order of the passed-in whitelist
.filter(t => Boolean(t)) as Array<Table>; // filter out non-existing tables
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should avoid using a type cast unless it's absolutely necessary.
Also you should avoid using the Boolean constructor because it does too much magic in terms of truthy/falsey coercion.

For filter, you can annotate with a type predicate

Suggested change
.filter(t => Boolean(t)) as Array<Table>; // filter out non-existing tables
// filter out non-existing tables
.filter((t): t is Table => t != null);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will give you the lint error: Expected '!==' and instead saw '!=' eqeqeq ...

@Kosta-Github
Copy link
Contributor Author

yeah, my use case is for acyclic dependencies for which I can create a correct partial order of the tables, but this would break for true cycles...

my last PR about supporting table locks #68 doesn't play that nicely with AWS Aurora and read replica: SET GLOBAL read_only = ON is not allowed on them, so I am trying to find other solutions...

maybe you can provide some pre & post command hooks where someone can feed in some commands required to lock and unlock the DB accordingly, which might be different for MySQL and AWS Aurora - read replica, and maybe some other variants?

@bradzacher
Copy link
Owner

it seems to me like you want to set your master DB to read_only before you start your dump?
That or just dump normally, and then clean up the data afterward?

When providing a table whitelist the order of that whitelist should be kept when dumping the tables.

This helps if dumping the database without table locks and having tables with foreign key references into other tables. By carefully defining the order of tables you can still achieve a consistent dump without violating the referential integrity by first dumping the table containing the foreign key, followed by the table which is the target of the reference.
@Kosta-Github
Copy link
Contributor Author

What would be the drawback of merging this PR?
It just changes the order of the scanned tables from a not-controllable (random) manner to a controllable-by-the-calling-user order.

@bradzacher bradzacher merged commit 98f0c9d into bradzacher:develop Jul 19, 2019
@Kosta-Github
Copy link
Contributor Author

may I ask you for pushing out a new release containing these changes?

@Kosta-Github Kosta-Github deleted the whitelist-stable-order branch March 26, 2020 14:16
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants