Update per Daniel's comments & some minor fixes

This commit is contained in:
Brian Fraser
2012-10-30 15:15:38 -03:00
parent fcf5fbcc6f
commit d5a3a6e59c
2 changed files with 68 additions and 6 deletions

View File

@@ -7800,10 +7800,17 @@ sub main {
PTDEBUG && _d("Renamed columns (old => new): ", Dumper(\%renamed_cols));
if ( %renamed_cols && $o->get('check-alter') ) {
die "Your --alter appears to be renaming a column. While "
. "the tool should handle this, we recommend testing it "
. "first with --dry-run before you run the tool with "
. "--no-check-alter to disable this error."
my $msg = "Your --alter appears to be renaming these columns: "
. join ", ", map "$_ to $renamed_cols{$_}", keys %renamed_cols;
if ( $o->get('dry-run') ) {
print $msg . "\n"
}
else {
die $msg
. ". While the tool should handle this, we recommend testing it "
. "first with --dry-run before you run the tool with "
. "--no-check-alter to disable this error."
}
}
}
@@ -8383,7 +8390,7 @@ sub _find_renamed_cols {
/x;
my $table_ident = qr/$unquoted_ident|`$quoted_ident`|"$ansi_quotes_ident"/;
my $alter_change_col_re = qr/CHANGE \s+ (?:COLUMN \s+)?
my $alter_change_col_re = qr/\bCHANGE \s+ (?:COLUMN \s+)?
($table_ident) \s+ ($table_ident)/ix;
my %renames;
@@ -8392,6 +8399,8 @@ sub _find_renamed_cols {
next unless $orig && $new;
my (undef, $orig_tbl) = Quoter->split_unquote($orig);
my (undef, $new_tbl) = Quoter->split_unquote($new);
# Silly but plausible: CHANGE COLUMN same_name same_name ...
next if lc($orig_tbl) eq lc($new_tbl);
$renames{$orig_tbl} = $new_tbl;
}
return %renames;
@@ -9517,7 +9526,22 @@ Sleep time between checks for L<"--max-lag">.
default: yes
Parses the --alter specified and tries to warn for some dubious behavior.
Parses the L<"--alter"> specified and tries to warn of possible unintended
behavior. Currently, it checks for:
=over
=item Column renames
In previous versions of the tool, renaming a column with
C<CHANGE COLUMN name new_name> would lead to that column's data being lost.
The tool now parses the alter statement and tries to catch these cases, so
the renamed columns should have the same data as the originals. However, the
code that does this is not a full-blown SQL parser, so we recommend that users
run the tool with L<"--dry-run"> and check if it's detecting the renames
correctly.
=back
=item --[no]check-plan

View File

@@ -206,6 +206,11 @@ $sb->load_file('master', "$sample/data-loss-bug-1068562.sql");
qw(--execute)) },
);
ok(
$exit_status,
"Bug 1068562: --execute dies if renaming a column without --no-check-alter"
);
like(
$output,
qr/--no-check-alter to disable this error/,
@@ -297,6 +302,39 @@ is_deeply(
"Bug 1068562: No columns went missing when renaming the columns back"
);
($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args,
"$master_dsn,D=sakila,t=staff",
"--alter", "change column first_name first_name_mod varchar(45) NOT NULL, change column last_name last_name_mod varchar(45) NOT NULL",
qw(--dry-run --alter-foreign-keys-method auto)) },
);
ok(
!$exit_status,
"Bug 1068562: No error with --dry-run"
);
like(
$output,
qr/first_name to first_name_mod, last_name to last_name_mod/ms,
"Bug 1068562: --dry-run warns about renaming columns"
);
# CHANGE COLUMN same_name same_name
($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args,
"$master_dsn,D=sakila,t=staff",
"--alter", "change column first_name first_name varchar(45) NOT NULL",
qw(--execute --alter-foreign-keys-method auto)) },
);
unlike(
$output,
qr/fist_name to fist_name/,
"Bug 1068562: change column same_name same_name doesn't warn about renames"
);
# #############################################################################
# Done.
# #############################################################################