From 39c020020cb21eaa850bcdd25dbbae173e0655c3 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Fri, 13 Dec 2013 19:31:30 -0800 Subject: [PATCH] Add and test --[no]check-child-tables. --- bin/pt-table-sync | 84 +++++++++++++++++++++++- t/pt-table-sync/basics.t | 2 +- t/pt-table-sync/safety_checks.t | 88 ++++++++++++++++++++++++++ t/pt-table-sync/samples/on_del_cas.sql | 19 ++++++ 4 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 t/pt-table-sync/safety_checks.t create mode 100644 t/pt-table-sync/samples/on_del_cas.sql diff --git a/bin/pt-table-sync b/bin/pt-table-sync index 2b79084c..eb9b27b6 100755 --- a/bin/pt-table-sync +++ b/bin/pt-table-sync @@ -10943,6 +10943,27 @@ sub ok_to_sync { } } + my $replace = $o->get('replace') + || $o->get('replicate') + || $o->get('sync-to-master'); + if ( $replace && $o->get('execute') && $o->get('check-child-tables') ) { + my $child_tables = find_child_tables( + tbl => $src, + dbh => $src->{dbh}, + Quoter => $q, + ); + if ( $child_tables ) { + foreach my $tbl ( @$child_tables ) { + my $ddl = $tp->get_create_table( + $src->{dbh}, $tbl->{db}, $tbl->{tbl}); + if ( $ddl && $ddl =~ m/(ON (?:DELETE|UPDATE) (?:SET|CASCADE))/ ) { + my $fk = $1; + die "REPLACE statements on $src->{db}.$src->{tbl} can adversely affect child table $tbl->{name} because it has an $fk foreign key constraint. See --[no]check-child-tables in the documentation for more information. --check-child-tables error\n" + } + } + } + } + return; } @@ -11363,6 +11384,46 @@ sub diff_where { } } +sub find_child_tables { + my ( %args ) = @_; + my @required_args = qw(tbl dbh Quoter); + foreach my $arg ( @required_args ) { + die "I need a $arg argument" unless $args{$arg}; + } + my ($tbl, $dbh, $q) = @args{@required_args}; + + if ( lc($tbl->{tbl_struct}->{engine} || '') eq 'myisam' ) { + PTDEBUG && _d(q{MyISAM table, not looking for child tables}); + return; + } + + PTDEBUG && _d('Finding child tables'); + + my $sql = "SELECT table_schema, table_name " + . "FROM information_schema.key_column_usage " + . "WHERE constraint_schema='$tbl->{db}' " + . "AND referenced_table_name='$tbl->{tbl}'"; + PTDEBUG && _d($sql); + my $rows = $dbh->selectall_arrayref($sql); + if ( !$rows || !@$rows ) { + PTDEBUG && _d('No child tables found'); + return; + } + + my @child_tables; + foreach my $row ( @$rows ) { + my $tbl = { + db => $row->[0], + tbl => $row->[1], + name => $q->quote(@$row), + }; + push @child_tables, $tbl; + } + + PTDEBUG && _d('Child tables:', Dumper(\@child_tables)); + return \@child_tables; +} + sub _d { my ($package, undef, $line) = caller 0; @_ = map { (my $temp = $_) =~ s/\n/\n# /g; $temp; } @@ -11589,7 +11650,7 @@ L<"SYNOPSIS"> for the overview of the correct usage. Also be careful with tables that have foreign key constraints with C or C definitions because these might cause unintended changes on the -child tables. +child tables. See L<"--[no]check-child-tables">. In general, this tool is best suited when your tables have a primary key or unique index. Although it can synchronize data in tables lacking a primary key @@ -11875,6 +11936,27 @@ runs SET NAMES UTF8 after connecting to MySQL. Any other value sets binmode on STDOUT without the utf8 layer, and runs SET NAMES after connecting to MySQL. +=item --[no]check-child-tables + +default: yes + +Check if L<"--execute"> will adversely affect child tables. When +L<"--replace">, L<"--replicate">, or L<"--sync-to-master"> is specified, +the tool may sync tables using C statements. If a table being +synced has child tables with C, C, +or C, the tool prints an error and skips the table because +C becomes C then C, so the C will cascade +to the child table and delete its rows. In the worst case, this can delete +all rows in child tables! + +Specify C<--no-check-child-tables> to disable this check. To completely +avoid affecting child tables, also specify C<--no-foreign-key-checks> +so MySQL will not cascade any operations from the parent to child tables. + +This check is only preformed if L<"--execute"> and one of L<"--replace">, +L<"--replicate">, or L<"--sync-to-master"> is specified. L<"--print"> +does not check child tables. + =item --[no]check-master default: yes diff --git a/t/pt-table-sync/basics.t b/t/pt-table-sync/basics.t index 58d46f2e..aaba73af 100644 --- a/t/pt-table-sync/basics.t +++ b/t/pt-table-sync/basics.t @@ -182,7 +182,7 @@ $output = output( sub { pt_table_sync::main('h=127.1,P=12345,u=msandbox,p=msandbox', qw(--print --execute --replicate percona.checksums), - qw(--no-foreign-key-checks)) + qw(--no-foreign-key-checks --no-check-child-tables)) } ); diff --git a/t/pt-table-sync/safety_checks.t b/t/pt-table-sync/safety_checks.t new file mode 100644 index 00000000..77554a3e --- /dev/null +++ b/t/pt-table-sync/safety_checks.t @@ -0,0 +1,88 @@ +#!/usr/bin/env perl + +BEGIN { + die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n" + unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH}; + unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib"; +}; + +use strict; +use warnings FATAL => 'all'; +use English qw(-no_match_vars); +use Test::More; +use Data::Dumper; + +use PerconaTest; +use Sandbox; +require "$trunk/bin/pt-table-sync"; + +my $output; +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $master_dbh = $sb->get_dbh_for('master'); +my $slave_dbh = $sb->get_dbh_for('slave1'); + +if ( !$master_dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} +elsif ( !$slave_dbh ) { + plan skip_all => 'Cannot connect to sandbox slave'; +} + +my $master_dsn = $sb->dsn_for('master'); +my $slave1_dsn = $sb->dsn_for('slave1'); + +# ############################################################################# +# --[no]check-child-tables +# pt-table-sync deletes child table rows Edit +# https://bugs.launchpad.net/percona-toolkit/+bug/1223458 +# ############################################################################# + +$sb->load_file('master', 't/pt-table-sync/samples/on_del_cas.sql'); + +$master_dbh->do("INSERT INTO on_del_cas.parent VALUES (1), (2)"); +$master_dbh->do("INSERT INTO on_del_cas.child1 VALUES (null, 1)"); +$master_dbh->do("INSERT INTO on_del_cas.child2 VALUES (null, 1)"); +$sb->wait_for_slaves(); + +$output = output( + sub { + pt_table_sync::main($slave1_dsn, qw(--sync-to-master), + qw(--execute -d on_del_cas)) + }, + stderr => 1, +); + +like( + $output, + qr/on on_del_cas.parent can adversely affect child table `on_del_cas`.`child2` because it has an ON DELETE CASCADE/, + "check-child-tables: error message" +); + +my $rows = $slave_dbh->selectall_arrayref("select * from on_del_cas.child2"); +is_deeply( + $rows, + [ [1,1] ], + "check-child-tables: child2 row not deleted" +) or diag(Dumper($rows)); + +$output = output( + sub { + pt_table_sync::main($slave1_dsn, qw(--sync-to-master), + qw(--print -d on_del_cas)) + }, + stderr => 1, +); + +unlike( + $output, + qr/on on_del_cas.parent can adversely affect child table `on_del_cas`.`child2` because it has an ON DELETE CASCADE/, + "check-child-tables: no error message with --print" +); + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($master_dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing; diff --git a/t/pt-table-sync/samples/on_del_cas.sql b/t/pt-table-sync/samples/on_del_cas.sql new file mode 100644 index 00000000..15a21ab4 --- /dev/null +++ b/t/pt-table-sync/samples/on_del_cas.sql @@ -0,0 +1,19 @@ +DROP DATABASE IF EXISTS on_del_cas; +CREATE DATABASE on_del_cas; +USE on_del_cas; + +CREATE TABLE parent ( + id INT NOT NULL AUTO_INCREMENT PRIMARY KEY +) ENGINE=InnoDB; + +CREATE TABLE child1 ( + id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, + parent_id INT NOT NULL, + FOREIGN KEY fk1 (parent_id) REFERENCES parent (id) ON DELETE RESTRICT +) ENGINE=InnoDB; + +CREATE TABLE child2 ( + id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, + parent_id INT NOT NULL, + FOREIGN KEY fk1 (parent_id) REFERENCES parent (id) ON DELETE CASCADE +) ENGINE=InnoDB;