From 8d2259e5b37eab97fce5e78da439ca9220bf99a3 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Tue, 18 Oct 2011 09:32:48 -0600 Subject: [PATCH] Fix --ignore-columns in NibbleIterator. Increase test coverage to 93%. --- bin/pt-table-checksum | 34 ++-- lib/NibbleIterator.pm | 34 ++-- t/lib/NibbleIterator.t | 380 ++++++++++++++++++++++++++--------------- 3 files changed, 283 insertions(+), 165 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 1d2ec1d9..b85bcdbc 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -3477,14 +3477,13 @@ sub new { die "There is no good index and the table is oversized."; } - my $where = $o->get('where'); + my $tbl_struct = $tbl->{tbl_struct}; + my $ignore_col = $o->get('ignore-columns') || {}; + my $all_cols = $o->get('columns') || $tbl_struct->{cols}; + my @cols = grep { !$ignore_col->{$_} } @$all_cols; + my $where = $o->get('where'); my $self; if ( $one_nibble ) { - my $tbl_struct = $tbl->{tbl_struct}; - my $ignore_col = $o->get('ignore-columns') || {}; - my $all_cols = $o->get('columns') || $tbl_struct->{cols}; - my @cols = grep { !$ignore_col->{$_} } @$all_cols; - my $nibble_sql = ($args{dms} ? "$args{dms} " : "SELECT ") . ($args{select} ? $args{select} @@ -3518,6 +3517,7 @@ sub new { %args, tbl_struct => $tbl->{tbl_struct}, index => $index, + cols => \@cols, asc_only => 1, ); MKDEBUG && _d('Ascend params:', Dumper($asc)); @@ -3604,10 +3604,11 @@ sub new { }; } - $self->{row_est} = $row_est; - $self->{nibbleno} = 0; - $self->{have_rows} = 0; - $self->{rowno} = 0; + $self->{row_est} = $row_est; + $self->{nibbleno} = 0; + $self->{have_rows} = 0; + $self->{rowno} = 0; + $self->{oktonibble} = 1; return bless $self, $class; } @@ -3615,6 +3616,11 @@ sub new { sub next { my ($self) = @_; + if ( !$self->{oktonibble} ) { + MKDEBUG && _d('Not ok to nibble'); + return; + } + my %callback_args = ( Cxn => $self->{Cxn}, tbl => $self->{tbl}, @@ -3625,9 +3631,9 @@ sub next { $self->_prepare_sths(); $self->_get_bounds(); if ( my $callback = $self->{callbacks}->{init} ) { - my $oktonibble = $callback->(%callback_args); - MKDEBUG && _d('init callback returned', $oktonibble); - if ( !$oktonibble ) { + $self->{oktonibble} = $callback->(%callback_args); + MKDEBUG && _d('init callback returned', $self->{oktonibble}); + if ( !$self->{oktonibble} ) { $self->{no_more_boundaries} = 1; return; } @@ -3733,7 +3739,7 @@ sub one_nibble { sub chunk_size { my ($self) = @_; - return $self->{limit}; + return $self->{limit} + 1; } sub set_chunk_size { diff --git a/lib/NibbleIterator.pm b/lib/NibbleIterator.pm index 973e69a5..c663ffe0 100644 --- a/lib/NibbleIterator.pm +++ b/lib/NibbleIterator.pm @@ -70,14 +70,13 @@ sub new { die "There is no good index and the table is oversized."; } - my $where = $o->get('where'); + my $tbl_struct = $tbl->{tbl_struct}; + my $ignore_col = $o->get('ignore-columns') || {}; + my $all_cols = $o->get('columns') || $tbl_struct->{cols}; + my @cols = grep { !$ignore_col->{$_} } @$all_cols; + my $where = $o->get('where'); my $self; if ( $one_nibble ) { - my $tbl_struct = $tbl->{tbl_struct}; - my $ignore_col = $o->get('ignore-columns') || {}; - my $all_cols = $o->get('columns') || $tbl_struct->{cols}; - my @cols = grep { !$ignore_col->{$_} } @$all_cols; - # If the chunk size is >= number of rows in table, then we don't # need to chunk; we can just select all rows, in order, at once. my $nibble_sql @@ -114,6 +113,7 @@ sub new { %args, tbl_struct => $tbl->{tbl_struct}, index => $index, + cols => \@cols, asc_only => 1, ); MKDEBUG && _d('Ascend params:', Dumper($asc)); @@ -214,10 +214,11 @@ sub new { }; } - $self->{row_est} = $row_est; - $self->{nibbleno} = 0; - $self->{have_rows} = 0; - $self->{rowno} = 0; + $self->{row_est} = $row_est; + $self->{nibbleno} = 0; + $self->{have_rows} = 0; + $self->{rowno} = 0; + $self->{oktonibble} = 1; return bless $self, $class; } @@ -225,6 +226,11 @@ sub new { sub next { my ($self) = @_; + if ( !$self->{oktonibble} ) { + MKDEBUG && _d('Not ok to nibble'); + return; + } + my %callback_args = ( Cxn => $self->{Cxn}, tbl => $self->{tbl}, @@ -237,9 +243,9 @@ sub next { $self->_prepare_sths(); $self->_get_bounds(); if ( my $callback = $self->{callbacks}->{init} ) { - my $oktonibble = $callback->(%callback_args); - MKDEBUG && _d('init callback returned', $oktonibble); - if ( !$oktonibble ) { + $self->{oktonibble} = $callback->(%callback_args); + MKDEBUG && _d('init callback returned', $self->{oktonibble}); + if ( !$self->{oktonibble} ) { $self->{no_more_boundaries} = 1; return; } @@ -352,7 +358,7 @@ sub one_nibble { sub chunk_size { my ($self) = @_; - return $self->{limit}; + return $self->{limit} + 1; } sub set_chunk_size { diff --git a/t/lib/NibbleIterator.t b/t/lib/NibbleIterator.t index cf7a0b17..c27f6c9e 100644 --- a/t/lib/NibbleIterator.t +++ b/t/lib/NibbleIterator.t @@ -39,7 +39,7 @@ if ( !$dbh ) { plan skip_all => 'Cannot connect to sandbox master'; } else { - plan tests => 30; + plan tests => 42; } my $q = new Quoter(); @@ -108,6 +108,17 @@ my $ni = make_nibble_iter( argv => [qw(--databases test --chunk-size 5)], ); +ok( + !$ni->one_nibble(), + "one_nibble() false" +); + +is( + $ni->nibble_number(), + 0, + "nibble_number() 0" +); + my @rows = (); for (1..5) { push @rows, $ni->next(); @@ -118,6 +129,12 @@ is_deeply( 'a-z nibble 1' ) or print Dumper(\@rows); +is( + $ni->nibble_number(), + 1, + "nibble_number() 1" +); + @rows = (); for (1..5) { push @rows, $ni->next(); @@ -128,6 +145,12 @@ is_deeply( 'a-z nibble 2' ) or print Dumper(\@rows); +is( + $ni->nibble_number(), + 2, + "nibble_number() 2" +); + @rows = (); for (1..5) { push @rows, $ni->next(); @@ -158,6 +181,11 @@ is_deeply( 'a-z nibble 5' ) or print Dumper(\@rows); +ok( + $ni->more_boundaries(), + "more_boundaries() true" +); + # There's only 1 row left but extra calls shouldn't return anything or crash. @rows = (); for (1..5) { @@ -169,6 +197,17 @@ is_deeply( 'a-z nibble 6' ) or print Dumper(\@rows); +ok( + !$ni->more_boundaries(), + "more_boundaries() false" +); + +is( + $ni->chunk_size(), + 5, + "chunk_size()" +); + # ############################################################################ # a-y w/ chunk-size 5, even nibbles # ############################################################################ @@ -219,6 +258,11 @@ $ni = make_nibble_iter( argv => [qw(--databases test --chunk-size 100)], ); +ok( + $ni->one_nibble(), + "one_nibble() true" +); + @rows = (); for (1..3) { push @rows, $ni->next(); @@ -284,153 +328,206 @@ done "callbacks" ); +# Test that init callback can stop nibbling. +$ni = make_nibble_iter( + db => 'test', + tbl => 't', + argv => [qw(--databases test --chunk-size 2)], + callbacks => { + init => sub { print "init\n"; return 0; }, + after_nibble => sub { print "after nibble\n"; }, + done => sub { print "done\n"; }, + } +); + +$dbh->do('delete from test.t limit 20'); # 6 rows left + +$output = output( + sub { + for (1..8) { $ni->next() } + }, +); + +is( + $output, +"init +", + "init callbacks can stop nibbling" +); + # ############################################################################ # Nibble a larger table by numeric pk id # ############################################################################ -SKIP: { - skip "Sakila database is not loaded", 8 - unless @{ $dbh->selectall_arrayref('show databases like "sakila"') }; +$ni = make_nibble_iter( + db => 'sakila', + tbl => 'payment', + argv => [qw(--databases sakila --tables payment --chunk-size 100)], +); - $ni = make_nibble_iter( - db => 'sakila', - tbl => 'payment', - argv => [qw(--databases sakila --tables payment --chunk-size 100)], - ); +my $n_nibbles = 0; +$n_nibbles++ while $ni->next(); +is( + $n_nibbles, + 16049, + "Nibble sakila.payment (16049 rows)" +); - my $n_nibbles = 0; - $n_nibbles++ while $ni->next(); - is( - $n_nibbles, - 16049, - "Nibble sakila.payment (16049 rows)" - ); +my $tbl = { + db => 'sakila', + tbl => 'country', + tbl_struct => $tp->parse( + $tp->get_create_table($dbh, 'sakila', 'country')), +}; +my $chunk_checksum = $rc->make_chunk_checksum( + dbh => $dbh, + tbl => $tbl, +); +$ni = make_nibble_iter( + db => 'sakila', + tbl => 'country', + argv => [qw(--databases sakila --tables country --chunk-size 25)], + select => $chunk_checksum, +); - my $tbl = { - db => 'sakila', - tbl => 'country', - tbl_struct => $tp->parse( - $tp->get_create_table($dbh, 'sakila', 'country')), - }; - my $chunk_checksum = $rc->make_chunk_checksum( - dbh => $dbh, - tbl => $tbl, - ); - $ni = make_nibble_iter( - db => 'sakila', - tbl => 'country', - argv => [qw(--databases sakila --tables country --chunk-size 25)], - select => $chunk_checksum, - ); +my $row = $ni->next(); +is_deeply( + $row, + [25, 'da79784d'], + "SELECT chunk checksum 1 FROM sakila.country" +) or print STDERR Dumper($row); - my $row = $ni->next(); - is_deeply( - $row, - [25, 'da79784d'], - "SELECT chunk checksum 1 FROM sakila.country" - ) or print STDERR Dumper($row); - - $row = $ni->next(); - is_deeply( - $row, - [25, 'e860c4f9'], - "SELECT chunk checksum 2 FROM sakila.country" - ) or print STDERR Dumper($row); - - $row = $ni->next(); - is_deeply( - $row, - [25, 'eb651f58'], - "SELECT chunk checksum 3 FROM sakila.country" - ) or print STDERR Dumper($row); - - $row = $ni->next(); - is_deeply( - $row, - [25, '2d87d588'], - "SELECT chunk checksum 4 FROM sakila.country" - ) or print STDERR Dumper($row); - - $row = $ni->next(); - is_deeply( - $row, - [9, 'beb4a180'], - "SELECT chunk checksum 5 FROM sakila.country" - ) or print STDERR Dumper($row); +$row = $ni->next(); +is_deeply( + $row, + [25, 'e860c4f9'], + "SELECT chunk checksum 2 FROM sakila.country" +) or print STDERR Dumper($row); + +$row = $ni->next(); +is_deeply( + $row, + [25, 'eb651f58'], + "SELECT chunk checksum 3 FROM sakila.country" +) or print STDERR Dumper($row); + +$row = $ni->next(); +is_deeply( + $row, + [25, '2d87d588'], + "SELECT chunk checksum 4 FROM sakila.country" +) or print STDERR Dumper($row); + +$row = $ni->next(); +is_deeply( + $row, + [9, 'beb4a180'], + "SELECT chunk checksum 5 FROM sakila.country" +) or print STDERR Dumper($row); - # ######################################################################### - # exec_nibble callback and explain_sth - # ######################################################################### - my @expl; - $ni = make_nibble_iter( - db => 'sakila', - tbl => 'country', - argv => [qw(--databases sakila --tables country --chunk-size 60)], - select => $chunk_checksum, - callbacks => { - exec_nibble => sub { - my (%args) = @_; - my $nibble_iter = $args{NibbleIterator}; - my $sth = $nibble_iter->statements(); - my $boundary = $nibble_iter->boundaries(); - $sth->{explain_nibble}->execute( - @{$boundary->{lower}}, @{$boundary->{upper}}); - push @expl, $sth->{explain_nibble}->fetchrow_hashref(); - return 0; - }, +# ######################################################################### +# exec_nibble callback and explain_sth +# ######################################################################### +my @expl; +$ni = make_nibble_iter( + db => 'sakila', + tbl => 'country', + argv => [qw(--databases sakila --tables country --chunk-size 60)], + select => $chunk_checksum, + callbacks => { + exec_nibble => sub { + my (%args) = @_; + my $nibble_iter = $args{NibbleIterator}; + my $sth = $nibble_iter->statements(); + my $boundary = $nibble_iter->boundaries(); + $sth->{explain_nibble}->execute( + @{$boundary->{lower}}, @{$boundary->{upper}}); + push @expl, $sth->{explain_nibble}->fetchrow_hashref(); + return 0; }, - one_nibble => 0, - ); - $ni->next(); - $ni->next(); - is_deeply( - \@expl, - [ - { - id => '1', - key => 'PRIMARY', - key_len => '2', - possible_keys => 'PRIMARY', - ref => undef, - rows => '54', - select_type => 'SIMPLE', - table => 'country', - type => 'range', - extra => 'Using where', - }, - { - id => '1', - key => 'PRIMARY', - key_len => '2', - possible_keys => 'PRIMARY', - ref => undef, - rows => '49', - select_type => 'SIMPLE', - table => 'country', - type => 'range', - extra => 'Using where', - }, - ], - 'exec_nibble callbackup and explain_sth' - ) or print STDERR Dumper(\@expl); + }, + one_nibble => 0, +); +$ni->next(); +$ni->next(); +is_deeply( + \@expl, + [ + { + id => '1', + key => 'PRIMARY', + key_len => '2', + possible_keys => 'PRIMARY', + ref => undef, + rows => '54', + select_type => 'SIMPLE', + table => 'country', + type => 'range', + extra => 'Using where', + }, + { + id => '1', + key => 'PRIMARY', + key_len => '2', + possible_keys => 'PRIMARY', + ref => undef, + rows => '49', + select_type => 'SIMPLE', + table => 'country', + type => 'range', + extra => 'Using where', + }, + ], +'exec_nibble callbackup and explain_sth' +) or print STDERR Dumper(\@expl); - # ######################################################################### - # film_actor, multi-column pk - # ######################################################################### - $ni = make_nibble_iter( - db => 'sakila', - tbl => 'film_actor', - argv => [qw(--tables sakila.film_actor --chunk-size 1000)], - ); +# ######################################################################### +# film_actor, multi-column pk +# ######################################################################### +$ni = make_nibble_iter( + db => 'sakila', + tbl => 'film_actor', + argv => [qw(--tables sakila.film_actor --chunk-size 1000)], +); - $n_nibbles = 0; - $n_nibbles++ while $ni->next(); - is( - $n_nibbles, - 5462, - "Nibble sakila.film_actor (multi-column pk)" - ); -} +$n_nibbles = 0; +$n_nibbles++ while $ni->next(); +is( + $n_nibbles, + 5462, + "Nibble sakila.film_actor (multi-column pk)" +); + +is_deeply( + $ni->sql(), + { + boundaries => { + '<' => '((`actor_id` < ?) OR (`actor_id` = ? AND `film_id` < ?))', + '<=' => '((`actor_id` < ?) OR (`actor_id` = ? AND `film_id` <= ?))', + '>' => '((`actor_id` > ?) OR (`actor_id` = ? AND `film_id` > ?))', + '>=' => '((`actor_id` > ?) OR (`actor_id` = ? AND `film_id` >= ?))' + }, + columns => [qw(actor_id actor_id film_id)], + from => '`sakila`.`film_actor` FORCE INDEX(`PRIMARY`)', + where => undef, + order_by => '`actor_id`, `film_id`', + }, + "sql()" +); + +$ni = make_nibble_iter( + db => 'sakila', + tbl => 'address', + argv => [qw(--tables sakila.address --chunk-size 10), + '--ignore-columns', 'phone,last_update'], +); + +$ni->next(); +is( + $ni->statements()->{nibble}->{Statement}, + "SELECT `address_id`, `address`, `address2`, `district`, `city_id`, `postal_code` FROM `sakila`.`address` FORCE INDEX(`PRIMARY`) WHERE ((`address_id` >= ?)) AND ((`address_id` <= ?)) ORDER BY `address_id` /*checksum chunk*/", + "--ignore-columns" +); # ############################################################################ # Reset chunk size on-the-fly. @@ -623,9 +720,18 @@ is_deeply( "--chunk-size-limit 0 on empty table" ); - # ############################################################################# # Done. # ############################################################################# +{ + local *STDERR; + open STDERR, '>', \$output; + $ni->_d('Complete test coverage'); +} +like( + $output, + qr/Complete test coverage/, + '_d() works' +); $sb->wipe_clean($dbh); exit;