Skip to content
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

fix: Allow proper filtering with --where in parent-child relationships #885

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions bin/pt-online-schema-change
Original file line number Diff line number Diff line change
Expand Up @@ -9392,7 +9392,6 @@ sub main {
Cxn => $cxn,
Quoter => $q,
only_same_schema_fks => $o->get('only-same-schema-fks'),
where => $o->get('where'),
);
if ( !$child_tables ) {
if ( $alter_fk_method ) {
Expand Down Expand Up @@ -10484,7 +10483,6 @@ sub main {
max_rows => $max_rows,
Cxn => $cxn,
OptionParser => $o,
where => $o->get('where'),
);

if ( $alter_fk_method eq 'drop_swap' ) {
Expand Down
222 changes: 222 additions & 0 deletions t/pt-online-schema-change/pt-1751.t
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,228 @@ is(
'Exit code 1 with --where and child tables'
);


# #############################################################################
# Option --where and foreign keys different ids
# #############################################################################

$sb->load_file('master', "$sample/basic_with_fks_diff_ids.sql");

$output = output(
sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=city",
'--alter', 'drop column last_update', '--where', 'city_id >= 3', '--execute',
'--alter-foreign-keys-method', 'rebuild_constraints', '--force') }
);

like(
$output,
qr/Successfully altered/i,
'Option --where runs with --force and --alter-foreign-keys-method=rebuild_constraints'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.city"`;
is(
$output + 0,
3,
'Only 3 rows copied'
);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.city where city_id < 3"`;
is(
$output + 0,
0,
'Rows, satisfying --where condition are not copied'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.country"`;
is(
$output + 0,
5,
'Table country not corrupted'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.address"`;
is(
$output + 0,
5,
'Table address not modified'
) or diag($output);

$sb->load_file('master', "$sample/basic_with_fks.sql");

$output = output(
sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=city",
'--alter', 'drop column last_update', '--where', 'city_id >= 3', '--execute',
'--alter-foreign-keys-method', 'auto', '--force') }
);

like(
$output,
qr/Successfully altered/i,
'Option --where runs with --force and --alter-foreign-keys-method=auto'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.city"`;
is(
$output + 0,
3,
'Only 3 rows copied'
);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.city where city_id < 3"`;
is(
$output + 0,
0,
'Rows, satisfying --where condition are not copied'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.country"`;
is(
$output + 0,
5,
'Table country not corrupted'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.address"`;
is(
$output + 0,
5,
'Table address not modified'
) or diag($output);

$sb->load_file('master', "$sample/basic_with_fks_diff_ids.sql");

$output = output(
sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=city",
'--alter', 'drop column last_update', '--where', 'city_id >= 3', '--execute',
'--alter-foreign-keys-method', 'drop_swap', '--force') }
);

like(
$output,
qr/Successfully altered/i,
'Option --where runs with --force and --alter-foreign-keys-method=drop_swap'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.city"`;
is(
$output + 0,
3,
'Only 3 rows copied'
);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.city where city_id < 3"`;
is(
$output + 0,
0,
'Rows, satisfying --where condition are not copied'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.country"`;
is(
$output + 0,
5,
'Table country not corrupted'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.address"`;
is(
$output + 0,
5,
'Table address not modified'
) or diag($output);

$sb->load_file('master', "$sample/basic_with_fks.sql");

$output = output(
sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=city",
'--alter', 'drop column last_update', '--where', 'city_id >= 3', '--execute',
'--alter-foreign-keys-method', 'none', '--force') }
);

like(
$output,
qr/Successfully altered/i,
'Option --where runs with --force and --alter-foreign-keys-method=none'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.city"`;
is(
$output + 0,
3,
'Only 3 rows copied'
);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.city where city_id < 3"`;
is(
$output + 0,
0,
'Rows, satisfying --where condition are not copied'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.country"`;
is(
$output + 0,
5,
'Table country not corrupted'
) or diag($output);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.address"`;
is(
$output + 0,
5,
'Table address not modified'
) or diag($output);

$sb->load_file('master', "$sample/basic_with_fks_diff_ids.sql");

($output, $exit) = full_output(
sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=city",
'--alter', 'drop column last_update', '--where', 'city_id >= 3', '--execute',
'--alter-foreign-keys-method', 'rebuild_constraints',
'--no-drop-new-table', '--no-swap-tables') }
);

like(
$output,
qr/Child tables found and option --where specified. Rebuilding foreign key constraints may lead to errors./i,
'Option --where does not run without --force and --alter-foreign-keys-method=rebuild_constraints when child tables are found'
) or diag($output);

is(
$exit,
1,
'Exit code 1 with --where and child tables'
);

$output = `/tmp/12345/use -N -e "select count(*) from pt_osc.address"`;
is(
$output + 0,
5,
'Table address not modified'
) or diag($output);

$sb->load_file('master', "$sample/basic_with_fks_diff_ids.sql");

($output, $exit) = full_output(
sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=city",
'--alter', 'drop column last_update', '--where', 'city_id >= 3', '--execute',
'--alter-foreign-keys-method', 'auto',
'--no-drop-new-table', '--no-swap-tables') }
);

like(
$output,
qr/Child tables found and option --where specified. Rebuilding foreign key constraints may lead to errors./i,
'Option --where does not run without --force and --alter-foreign-keys-method=auto when child tables are found'
) or diag($output);

is(
$exit,
1,
'Exit code 1 with --where and child tables'
);

# #############################################################################
# Done.
# #############################################################################
Expand Down
70 changes: 70 additions & 0 deletions t/pt-online-schema-change/samples/basic_with_fks_diff_ids.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
DROP DATABASE IF EXISTS pt_osc;

CREATE DATABASE pt_osc;

USE pt_osc;

SET foreign_key_checks = 0;

CREATE TABLE `country` (
`country_id` smallint(5) unsigned NOT NULL AUTO_INCREMENT,
`country` varchar(50) NOT NULL,
`last_update` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
PRIMARY KEY (`country_id`)
) ENGINE = InnoDB;

CREATE TABLE `city` (
`id` smallint(5) unsigned NOT NULL AUTO_INCREMENT,
`city` varchar(50) NOT NULL,
`country_id` smallint(5) unsigned NOT NULL,
`last_update` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
PRIMARY KEY (`id`), -- corrected from `city_id` to `id`
KEY `idx_fk_country_id` (`country_id`),
CONSTRAINT `fk_city_country` FOREIGN KEY (`country_id`) REFERENCES `country` (`country_id`) ON UPDATE CASCADE
) ENGINE = InnoDB;

CREATE TABLE `address` (
`id` smallint(5) unsigned NOT NULL AUTO_INCREMENT,
`address` varchar(50) NOT NULL,
`city_id` smallint(5) unsigned NOT NULL,
`postal_code` varchar(10) DEFAULT NULL,
`last_update` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
PRIMARY KEY (`id`), -- corrected from `address_id` to `id`
KEY `idx_fk_city_id` (`city_id`),
CONSTRAINT `fk_address_city` FOREIGN KEY (`city_id`) REFERENCES `city` (`id`) ON UPDATE CASCADE -- corrected to reference `id` in `city`
) ENGINE = InnoDB;

INSERT INTO
pt_osc.country (`country_id`, `country`)
VALUES (1, 'Canada'),
(2, 'USA'),
(3, 'Mexico'),
(4, 'France'),
(5, 'Spain');

INSERT INTO
pt_osc.city (
`city_id`,
`city`,
`country_id`
)
VALUES (1, 'Montréal', 1),
(2, 'New York', 2),
(3, 'Durango', 3),
(4, 'Paris', 4),
(5, 'Madrid', 5);

INSERT INTO
pt_osc.address (
`address_id`,
`address`,
`city_id`,
`postal_code`
)
VALUES (1, 'addy 1', 1, '10000'),
(2, 'addy 2', 2, '20000'),
(3, 'addy 3', 3, '30000'),
(4, 'addy 4', 4, '40000'),
(5, 'addy 5', 5, '50000');

SET foreign_key_checks = 1;