diff --git a/bin/pt-query-digest b/bin/pt-query-digest index 5f91b150c..ef3880879 100755 --- a/bin/pt-query-digest +++ b/bin/pt-query-digest @@ -9405,6 +9405,11 @@ has history_metrics => ( isa => 'ArrayRef', ); +has main_columns => ( + is => 'rw', + isa => 'ArrayRef', +); + has column_pattern => ( is => 'ro', isa => 'Regexp', @@ -9426,29 +9431,33 @@ sub set_history_options { my $col_pat = $self->column_pattern(); my @cols; + my @main_cols; my @metrics; foreach my $col ( @{$args{tbl_struct}->{cols}} ) { my ( $attr, $metric ) = $col =~ m/$col_pat/; - next unless $attr && $metric; - + if ( $attr && $metric ) { - $attr = ucfirst $attr if $attr =~ m/_/; - $attr = 'Filesort' if $attr eq 'filesort'; + $attr = ucfirst $attr if $attr =~ m/_/; + $attr = 'Filesort' if $attr eq 'filesort'; - $attr =~ s/^Qc_hit/QC_Hit/; # Qc_hit is really QC_Hit - $attr =~ s/^Innodb/InnoDB/g; # Innodb is really InnoDB - $attr =~ s/_io_/_IO_/g; # io is really IO + $attr =~ s/^Qc_hit/QC_Hit/; # Qc_hit is really QC_Hit + $attr =~ s/^Innodb/InnoDB/g; # Innodb is really InnoDB + $attr =~ s/_io_/_IO_/g; # io is really IO - push @cols, $col; - push @metrics, [$attr, $metric]; + push @cols, $col; + push @metrics, [$attr, $metric]; + } else { + push @main_cols, $col; + } } my $ts_default = $self->ts_default; my $sql = "REPLACE INTO $args{table}(" . join(', ', - map { Quoter->quote($_) } ('checksum', 'sample', @cols)) - . ') VALUES (?, ?' + map { Quoter->quote($_) } (@main_cols, @cols)) + . ') VALUES (' + . join(', ', map { '?' } @main_cols) . (@cols ? ', ' : '') # issue 1265 . join(', ', map { $_ eq 'ts_min' || $_ eq 'ts_max' @@ -9459,20 +9468,25 @@ sub set_history_options { $self->history_sth($self->history_dbh->prepare($sql)); $self->history_metrics(\@metrics); + $self->main_columns(\@main_cols); return; } sub set_review_history { - my ( $self, $id, $sample, %data ) = @_; + my ( $self, $id, $sample, $data, $users) = @_; foreach my $thing ( qw(min max) ) { - next unless defined $data{ts} && defined $data{ts}->{$thing}; - $data{ts}->{$thing} = parse_timestamp($data{ts}->{$thing}); + next unless defined $data->{ts} && defined $data->{ts}->{$thing}; + $data->{ts}->{$thing} = parse_timestamp($data->{ts}->{$thing}); + } + my @execute_params; + for my $col (@{$self->main_columns}) { + push @execute_params, make_checksum($id) if $col eq 'checksum'; + push @execute_params, $sample if $col eq 'sample'; + push @execute_params, "[".join(",", map { "\"".$_."\""} sort keys %{$users})."]" if $col eq 'usernames'; } - $self->history_sth->execute( - make_checksum($id), - $sample, - map { $data{$_->[0]}->{$_->[1]} } @{$self->history_metrics}); + push @execute_params, map { $data->{$_->[0]}->{$_->[1]} } @{$self->history_metrics}; + $self->history_sth->execute(@execute_params); } sub _d { @@ -15082,6 +15096,10 @@ sub update_query_history_table { foreach my $worst_info ( @$worst ) { my $item = $worst_info->[0]; + my %users = (); + if (defined $ea->results->{'classes'}{$item}{'user'}{'unq'}) { + %users = %{$ea->results->{'classes'}{$item}{'user'}{'unq'}}; + } my $sample = $ea->results->{samples}->{$item}; my %history; @@ -15092,7 +15110,7 @@ sub update_query_history_table { ); } $qh->set_review_history( - $item, $sample->{arg} || '', %history); + $item, $sample->{arg} || '', \%history, \%users); } return; @@ -15991,11 +16009,13 @@ is specified (see L<"--[no]create-history-table">). pt-query-digest inspects the columns in the table. The table must have at least the following columns: - CREATE TABLE query_review_history ( + CREATE TABLE query_history ( checksum CHAR(32) NOT NULL, sample TEXT NOT NULL ); +There could be also C column. + Any columns not mentioned above are inspected to see if they follow a certain naming convention. The column is special if the name ends with an underscore followed by any of these values: @@ -16024,6 +16044,7 @@ MAGIC_create_history_table CREATE TABLE IF NOT EXISTS query_history ( checksum CHAR(32) NOT NULL, sample TEXT NOT NULL, + usernames JSON, ts_min DATETIME, ts_max DATETIME, ts_cnt FLOAT, diff --git a/lib/QueryHistory.pm b/lib/QueryHistory.pm index fb1fb6b90..cd4d0ae45 100644 --- a/lib/QueryHistory.pm +++ b/lib/QueryHistory.pm @@ -43,6 +43,11 @@ has history_metrics => ( isa => 'ArrayRef', ); +has main_columns => ( + is => 'rw', + isa => 'ArrayRef', +); + has column_pattern => ( is => 'ro', isa => 'Regexp', @@ -68,37 +73,41 @@ sub set_history_options { # Pick out columns, attributes and metrics that need to be stored in the # table. my @cols; + my @main_cols; my @metrics; foreach my $col ( @{$args{tbl_struct}->{cols}} ) { my ( $attr, $metric ) = $col =~ m/$col_pat/; - next unless $attr && $metric; - - # TableParser lowercases the column names so, e.g., Query_time - # becomes query_time. We have to fix this so attribs in the event - # match keys in $self->{history_metrics}... - - # If the attrib name has at least one _ then it's a multi-word - # attrib like Query_time or Lock_time, so the first letter should - # be uppercase. Else, it's a one-word attrib like ts, checksum - # or sample, so we leave it alone. Except Filesort which is yet - # another exception. - $attr = ucfirst $attr if $attr =~ m/_/; - $attr = 'Filesort' if $attr eq 'filesort'; - - $attr =~ s/^Qc_hit/QC_Hit/; # Qc_hit is really QC_Hit - $attr =~ s/^Innodb/InnoDB/g; # Innodb is really InnoDB - $attr =~ s/_io_/_IO_/g; # io is really IO - - push @cols, $col; - push @metrics, [$attr, $metric]; + if ( $attr && $metric ) { + # TableParser lowercases the column names so, e.g., Query_time + # becomes query_time. We have to fix this so attribs in the event + # match keys in $self->{history_metrics}... + + # If the attrib name has at least one _ then it's a multi-word + # attrib like Query_time or Lock_time, so the first letter should + # be uppercase. Else, it's a one-word attrib like ts, checksum + # or sample, so we leave it alone. Except Filesort which is yet + # another exception. + $attr = ucfirst $attr if $attr =~ m/_/; + $attr = 'Filesort' if $attr eq 'filesort'; + + $attr =~ s/^Qc_hit/QC_Hit/; # Qc_hit is really QC_Hit + $attr =~ s/^Innodb/InnoDB/g; # Innodb is really InnoDB + $attr =~ s/_io_/_IO_/g; # io is really IO + + push @cols, $col; + push @metrics, [$attr, $metric]; + } else { + push @main_cols, $col; + } } my $ts_default = $self->ts_default; my $sql = "REPLACE INTO $args{table}(" . join(', ', - map { Quoter->quote($_) } ('checksum', 'sample', @cols)) - . ') VALUES (?, ?' + map { Quoter->quote($_) } (@main_cols, @cols)) + . ') VALUES (' + . join(', ', map { '?' } @main_cols) . (@cols ? ', ' : '') # issue 1265 . join(', ', map { # ts_min and ts_max might be part of the PK, in which case they must @@ -111,6 +120,7 @@ sub set_history_options { $self->history_sth($self->history_dbh->prepare($sql)); $self->history_metrics(\@metrics); + $self->main_columns(\@main_cols); return; } @@ -119,16 +129,20 @@ sub set_history_options { # of hashes. Each top-level key is an attribute name, and each second-level key # is a metric name. Look at the test for more examples. sub set_review_history { - my ( $self, $id, $sample, %data ) = @_; + my ( $self, $id, $sample, $data, $users) = @_; # Need to transform ts->min/max into timestamps foreach my $thing ( qw(min max) ) { - next unless defined $data{ts} && defined $data{ts}->{$thing}; - $data{ts}->{$thing} = parse_timestamp($data{ts}->{$thing}); + next unless defined $data->{ts} && defined $data->{ts}->{$thing}; + $data->{ts}->{$thing} = parse_timestamp($data->{ts}->{$thing}); + } + my @execute_params; + for my $col (@{$self->main_columns}) { + push @execute_params, make_checksum($id) if $col eq 'checksum'; + push @execute_params, $sample if $col eq 'sample'; + push @execute_params, "[".join(",", map { "\"".$_."\""} sort keys %{$users})."]" if $col eq 'usernames'; } - $self->history_sth->execute( - make_checksum($id), - $sample, - map { $data{$_->[0]}->{$_->[1]} } @{$self->history_metrics}); + push @execute_params, map { $data->{$_->[0]}->{$_->[1]} } @{$self->history_metrics}; + $self->history_sth->execute(@execute_params); } sub _d { diff --git a/t/lib/samples/slowlogs/slow062.txt b/t/lib/samples/slowlogs/slow062.txt new file mode 100644 index 000000000..04363c298 --- /dev/null +++ b/t/lib/samples/slowlogs/slow062.txt @@ -0,0 +1,83 @@ +# Time: 071218 11:48:27 # User@Host: [SQL_SLAVE] @ [] +# Thread_id: 10 +# Query_time: 0.000012 Lock_time: 0.000000 Rows_sent: 0 Rows_examined: 0 +# QC_Hit: No Full_scan: No Full_join: No Tmp_table: No Tmp_table_on_disk: No +# Filesort: No Filesort_on_disk: No Merge_passes: 0 +# No InnoDB statistics available for this query +BEGIN; +# User@Host: [SQL_SLAVE] @ [] +# Thread_id: 10 +# Query_time: 0.726052 Lock_time: 0.000091 Rows_sent: 0 Rows_examined: 62951 +# QC_Hit: No Full_scan: Yes Full_join: No Tmp_table: No Tmp_table_on_disk: No +# Filesort: No Filesort_on_disk: No Merge_passes: 0 +# No InnoDB statistics available for this query +use db1; +SET timestamp=1197996507; +update db2.tuningdetail_21_265507 n + inner join db1.gonzo a using(gonzo) + set n.column1 = a.column1, n.word3 = a.word3; +# User@Host: [SQL_SLAVE] @ [] +# Thread_id: 10 +# Query_time: 0.000512 Lock_time: 0.000077 Rows_sent: 0 Rows_examined: 0 +# QC_Hit: No Full_scan: No Full_join: No Tmp_table: No Tmp_table_on_disk: No +# Filesort: No Filesort_on_disk: No Merge_passes: 0 +# InnoDB_IO_r_ops: 0 InnoDB_IO_r_bytes: 0 InnoDB_IO_r_wait: 0.000000 +# InnoDB_rec_lock_wait: 0.000000 InnoDB_queue_wait: 0.000000 +# InnoDB_pages_distinct: 24 +SET timestamp=1197996507; +INSERT INTO db3.vendor11gonzo (makef, bizzle) +VALUES ('', 'Exact'); +# User@Host: [SQL_SLAVE] @ [] +# Thread_id: 10 +# Query_time: 0.033384 Lock_time: 0.000028 Rows_sent: 0 Rows_examined: 0 +# QC_Hit: No Full_scan: No Full_join: No Tmp_table: No Tmp_table_on_disk: No +# Filesort: No Filesort_on_disk: No Merge_passes: 0 +# InnoDB_IO_r_ops: 0 InnoDB_IO_r_bytes: 0 InnoDB_IO_r_wait: 0.000000 +# InnoDB_rec_lock_wait: 0.000000 InnoDB_queue_wait: 0.000000 +# InnoDB_pages_distinct: 11 +UPDATE db4.vab3concept1upload +SET vab3concept1id = '91848182522' +WHERE vab3concept1upload='6994465'; +# User@Host: [SQL_SLAVE] @ [] +# Thread_id: 10 +# Query_time: 0.000530 Lock_time: 0.000027 Rows_sent: 0 Rows_examined: 0 +# QC_Hit: No Full_scan: No Full_join: No Tmp_table: No Tmp_table_on_disk: No +# Filesort: No Filesort_on_disk: No Merge_passes: 0 +# InnoDB_IO_r_ops: 0 InnoDB_IO_r_bytes: 0 InnoDB_IO_r_wait: 0.000000 +# InnoDB_rec_lock_wait: 0.000000 InnoDB_queue_wait: 0.000000 +# InnoDB_pages_distinct: 18 +SET insert_id=34484549,timestamp=1197996507; +INSERT INTO db1.conch (word3, vid83) +VALUES ('211', '18'); +# User@Host: [SQL_SLAVE1] @ [] +# Thread_id: 10 +# Query_time: 0.000530 Lock_time: 0.000027 Rows_sent: 0 Rows_examined: 0 +# QC_Hit: No Full_scan: No Full_join: No Tmp_table: No Tmp_table_on_disk: No +# Filesort: No Filesort_on_disk: No Merge_passes: 0 +# InnoDB_IO_r_ops: 0 InnoDB_IO_r_bytes: 0 InnoDB_IO_r_wait: 0.000000 +# InnoDB_rec_lock_wait: 0.000000 InnoDB_queue_wait: 0.000000 +# InnoDB_pages_distinct: 18 +UPDATE foo.bar +SET biz = '91848182522'; +# User@Host: [SQL_SLAVE] @ [] +# Thread_id: 10 +# Query_time: 0.000530 Lock_time: 0.000027 Rows_sent: 0 Rows_examined: 0 +# QC_Hit: No Full_scan: No Full_join: No Tmp_table: No Tmp_table_on_disk: No +# Filesort: No Filesort_on_disk: No Merge_passes: 0 +# InnoDB_IO_r_ops: 0 InnoDB_IO_r_bytes: 0 InnoDB_IO_r_wait: 0.000000 +# InnoDB_rec_lock_wait: 0.000000 InnoDB_queue_wait: 0.000000 +# InnoDB_pages_distinct: 18 +SET timestamp=1197996508; +UPDATE bizzle.bat +SET boop='bop: 899' +WHERE fillze='899'; +# User@Host: [SQL_SLAVE] @ [] +# Thread_id: 10 +# Query_time: 0.000530 Lock_time: 0.000027 Rows_sent: 0 Rows_examined: 0 +# QC_Hit: No Full_scan: No Full_join: No Tmp_table: No Tmp_table_on_disk: No +# Filesort: No Filesort_on_disk: No Merge_passes: 0 +# InnoDB_IO_r_ops: 0 InnoDB_IO_r_bytes: 0 InnoDB_IO_r_wait: 0.000000 +# InnoDB_rec_lock_wait: 0.000000 InnoDB_queue_wait: 0.000000 +# InnoDB_pages_distinct: 18 +UPDATE foo.bar +SET biz = '91848182522'; diff --git a/t/pt-query-digest/history.t b/t/pt-query-digest/history.t index 1f9f1dff7..b0e1f7557 100644 --- a/t/pt-query-digest/history.t +++ b/t/pt-query-digest/history.t @@ -161,7 +161,7 @@ is($table, 'query_history', '--create-history-table creates both percona_schema # ############################################################################# $dbh->do('truncate table test.query_review_history'); -run_with("slow002.txt", +run_with("slow062.txt", '--history', "$dsn,D=test,t=query_review_history", '--no-report', '--filter', '$event->{arg} =~ m/foo\.bar/'); @@ -171,6 +171,7 @@ $res = $dbh->selectall_arrayref( 'SELECT * FROM test.query_review_history', $expected = [ { checksum => '32F63B9B2CE5A9B1B211BA2B8D6D065C', + usernames => '["[SQL_SLAVE1]", "[SQL_SLAVE]"]', filesort_cnt => '2', filesort_on_disk_cnt => '2', filesort_on_disk_sum => '0', @@ -304,6 +305,46 @@ unlike( "No error using minimum 2-column query review history table (issue 1265)", ); +# ############################################################################# +# Review history with 3 main columns working +# ############################################################################# +$dbh->do('use test'); +$dbh->do('drop table test.query_review_history'); + +# mqd says "The table must have at least the following columns:" +my $usernames_tbl = "CREATE TABLE test.query_review_history ( + checksum CHAR(32) NOT NULL PRIMARY KEY, + sample TEXT NOT NULL, + usernames JSON +)"; +$dbh->do($usernames_tbl); + +run_with("slow062.txt", + '--history', "$dsn,D=test,t=query_review_history", + '--no-report', '--filter', '$event->{arg} =~ m/foo\.bar/'); + +$res = $dbh->selectall_arrayref( 'SELECT * FROM test.query_review_history', + { Slice => {} } ); + +$expected = [ + { + checksum => '32F63B9B2CE5A9B1B211BA2B8D6D065C', + usernames => '["[SQL_SLAVE1]", "[SQL_SLAVE]"]', + sample => 'UPDATE foo.bar +SET biz = \'91848182522\'', + } +]; + +normalize_numbers($res); +normalize_numbers($expected); + +# 4 +is_deeply( + $res, + $expected, + "Review history with 3 main columns working", +); + # ############################################################################# # Done. # #############################################################################