mirror of
https://github.com/percona/percona-toolkit.git
synced 2025-09-14 15:33:49 +00:00
Fix for 937234: pt-query-advisor issues wrong RES.001
This commit is contained in:
@@ -3813,9 +3813,13 @@ sub get_rules {
|
|||||||
grep { $_->{column} }
|
grep { $_->{column} }
|
||||||
@$groupby;
|
@$groupby;
|
||||||
return unless scalar %groupby_col;
|
return unless scalar %groupby_col;
|
||||||
my $cols = $event->{query_struct}->{columns};
|
my $cols = [
|
||||||
|
grep { _looks_like_column($_->{col}) }
|
||||||
|
@{$event->{query_struct}->{columns}}
|
||||||
|
];
|
||||||
foreach my $col ( @$cols ) {
|
foreach my $col ( @$cols ) {
|
||||||
return 0 unless $groupby_col{ $col->{col} };
|
return 0 unless $groupby_col{ $col->{col} }
|
||||||
|
|| ($col->{alias} && $groupby_col{ $col->{alias} });
|
||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
},
|
},
|
||||||
@@ -4049,6 +4053,13 @@ sub determine_table_for_column {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
sub _looks_like_column {
|
||||||
|
my $col = shift;
|
||||||
|
return if $col eq '*' || $col eq 'NULL';
|
||||||
|
return if $col =~ /\A(?:\b[0-9]+\b|[^(]+\(|\@{1,2}.+)/;
|
||||||
|
return $col;
|
||||||
|
}
|
||||||
|
|
||||||
sub _d {
|
sub _d {
|
||||||
my ($package, undef, $line) = caller 0;
|
my ($package, undef, $line) = caller 0;
|
||||||
@_ = map { (my $temp = $_) =~ s/\n/\n# /g; $temp; }
|
@_ = map { (my $temp = $_) =~ s/\n/\n# /g; $temp; }
|
||||||
|
@@ -341,11 +341,16 @@ sub get_rules {
|
|||||||
grep { $_->{column} }
|
grep { $_->{column} }
|
||||||
@$groupby;
|
@$groupby;
|
||||||
return unless scalar %groupby_col;
|
return unless scalar %groupby_col;
|
||||||
my $cols = $event->{query_struct}->{columns};
|
# Skip non-columns -- NULL, digits, functions, variables
|
||||||
|
my $cols = [
|
||||||
|
grep { _looks_like_column($_->{col}) }
|
||||||
|
@{$event->{query_struct}->{columns}}
|
||||||
|
];
|
||||||
# All SELECT cols must be in GROUP BY cols clause.
|
# All SELECT cols must be in GROUP BY cols clause.
|
||||||
# E.g. select a, b, c from tbl group by a; -- non-deterministic
|
# E.g. select a, b, c from tbl group by a; -- non-deterministic
|
||||||
foreach my $col ( @$cols ) {
|
foreach my $col ( @$cols ) {
|
||||||
return 0 unless $groupby_col{ $col->{col} };
|
return 0 unless $groupby_col{ $col->{col} }
|
||||||
|
|| ($col->{alias} && $groupby_col{ $col->{alias} });
|
||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
},
|
},
|
||||||
@@ -658,6 +663,14 @@ sub determine_table_for_column {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
sub _looks_like_column {
|
||||||
|
my $col = shift;
|
||||||
|
# NULL, numbers, variables and functions are definitely not columns
|
||||||
|
return if $col eq '*' || $col eq 'NULL';
|
||||||
|
return if $col =~ /\A(?:\b[0-9]+\b|[^(]+\(|\@{1,2}.+)/;
|
||||||
|
return $col;
|
||||||
|
}
|
||||||
|
|
||||||
sub _d {
|
sub _d {
|
||||||
my ($package, undef, $line) = caller 0;
|
my ($package, undef, $line) = caller 0;
|
||||||
@_ = map { (my $temp = $_) =~ s/\n/\n# /g; $temp; }
|
@_ = map { (my $temp = $_) =~ s/\n/\n# /g; $temp; }
|
||||||
|
@@ -9,7 +9,7 @@ BEGIN {
|
|||||||
use strict;
|
use strict;
|
||||||
use warnings FATAL => 'all';
|
use warnings FATAL => 'all';
|
||||||
use English qw(-no_match_vars);
|
use English qw(-no_match_vars);
|
||||||
use Test::More tests => 87;
|
use Test::More;
|
||||||
|
|
||||||
use PerconaTest;
|
use PerconaTest;
|
||||||
use PodParser;
|
use PodParser;
|
||||||
@@ -466,6 +466,11 @@ my @cases = (
|
|||||||
query => "select col1, col2 from tbl where i=1 order by col1, col2 desc",
|
query => "select col1, col2 from tbl where i=1 order by col1, col2 desc",
|
||||||
advice => [qw(CLA.007)],
|
advice => [qw(CLA.007)],
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name => 'Bug 937234: wrong RES.001',
|
||||||
|
query => q{select NULL, 1, COUNT(*), @@time_zone, foo as field2 from t1 group by field2},
|
||||||
|
advice => [qw(CLA.001)],
|
||||||
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
# Run the test cases.
|
# Run the test cases.
|
||||||
@@ -524,4 +529,6 @@ like(
|
|||||||
qr/Complete test coverage/,
|
qr/Complete test coverage/,
|
||||||
'_d() works'
|
'_d() works'
|
||||||
);
|
);
|
||||||
|
|
||||||
|
done_testing;
|
||||||
exit;
|
exit;
|
||||||
|
Reference in New Issue
Block a user