From 0ab2e4bfd6429efa18c46ad76db69ecfa470e750 Mon Sep 17 00:00:00 2001 From: "mkanat%kerio.com" <> Date: Tue, 15 Mar 2005 07:17:46 +0000 Subject: Bug 251960: Search.pm uses DB dependent comma operator Patch By Tomas Kopal r=joel, a=justdave --- Bugzilla/Search.pm | 199 +++++++++++++----------- template/en/default/global/code-error.html.tmpl | 5 + 2 files changed, 115 insertions(+), 89 deletions(-) diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index c38d07054..e38aed7bf 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -115,32 +115,34 @@ sub init { # First, deal with all the old hard-coded non-chart-based poop. if (grep(/map_assigned_to/, @$fieldsref)) { - push @supptables, "profiles AS map_assigned_to"; - push @wherepart, "bugs.assigned_to = map_assigned_to.userid"; + push @supptables, "INNER JOIN profiles AS map_assigned_to " . + "ON bugs.assigned_to = map_assigned_to.userid"; } if (grep(/map_reporter/, @$fieldsref)) { - push @supptables, "profiles AS map_reporter"; - push @wherepart, "bugs.reporter = map_reporter.userid"; + push @supptables, "INNER JOIN profiles AS map_reporter " . + "ON bugs.reporter = map_reporter.userid"; } if (grep(/map_qa_contact/, @$fieldsref)) { - push @supptables, "LEFT JOIN profiles map_qa_contact ON bugs.qa_contact = map_qa_contact.userid"; + push @supptables, "LEFT JOIN profiles AS map_qa_contact " . + "ON bugs.qa_contact = map_qa_contact.userid"; } if (lsearch($fieldsref, 'map_products.name') >= 0) { - push @supptables, "products AS map_products"; - push @wherepart, "bugs.product_id = map_products.id"; + push @supptables, "INNER JOIN products AS map_products " . + "ON bugs.product_id = map_products.id"; } if (lsearch($fieldsref, 'map_classifications.name') >= 0) { - push @supptables, "classifications AS map_classifications"; - push @wherepart, "map_products.classification_id = map_classifications.id"; + push @supptables, + "INNER JOIN classifications AS map_classifications " . + "ON map_products.classification_id = map_classifications.id"; } if (lsearch($fieldsref, 'map_components.name') >= 0) { - push @supptables, "components AS map_components"; - push @wherepart, "bugs.component_id = map_components.id"; + push @supptables, "INNER JOIN components AS map_components " . + "ON bugs.component_id = map_components.id"; } my $minvotes; @@ -213,8 +215,8 @@ sub init { } if (lsearch($fieldsref, "(SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id)) AS actual_time") != -1) { - push(@supptables, "longdescs AS ldtime"); - push(@wherepart, "ldtime.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS ldtime " . + "ON ldtime.bug_id = bugs.bug_id"); } foreach my $id ("1", "2") { @@ -312,17 +314,18 @@ sub init { # @list won't have any elements if the only field being searched # is [Bug creation] (in which case we don't need bugs_activity). if(@list) { - push(@supptables, "bugs_activity actcheck"); - push(@wherepart, "actcheck.bug_id = bugs.bug_id"); + my $extra = ""; if($sql_chfrom) { - push(@wherepart, "actcheck.bug_when >= $sql_chfrom"); + $extra .= " AND actcheck.bug_when >= $sql_chfrom"; } if($sql_chto) { - push(@wherepart, "actcheck.bug_when <= $sql_chto"); + $extra .= " AND actcheck.bug_when <= $sql_chto"; } if($sql_chvalue) { - push(@wherepart, "actcheck.added = $sql_chvalue"); + $extra .= " AND actcheck.added = $sql_chvalue"; } + push(@supptables, "INNER JOIN bugs_activity AS actcheck " . + "ON actcheck.bug_id = bugs.bug_id $extra"); } # Now that we're done using @list to determine if there are any @@ -400,7 +403,7 @@ sub init { $groupid || ThrowUserError('invalid_group_name',{name => $group}); my @childgroups = @{$user->flatten_group_membership($groupid)}; my $table = "user_group_map_$chartid"; - push (@supptables, "LEFT JOIN user_group_map $table " . + push (@supptables, "LEFT JOIN user_group_map AS $table " . "ON $table.user_id = bugs.$f " . "AND $table.group_id IN(" . join(',', @childgroups) . ") " . @@ -421,13 +424,13 @@ sub init { $term = "bugs.$f <> " . pronoun($1, $user); }, "^(assigned_to|reporter),(?!changed)" => sub { - push(@supptables, "profiles AS map_$f"); - push(@wherepart, "bugs.$f = map_$f.userid"); + push(@supptables, "INNER JOIN profiles AS map_$f " . + "ON bugs.$f = map_$f.userid"); $f = "map_$f.login_name"; }, "^qa_contact,(?!changed)" => sub { - push(@supptables, - "LEFT JOIN profiles map_qa_contact ON bugs.qa_contact = map_qa_contact.userid"); + push(@supptables, "LEFT JOIN profiles AS map_qa_contact " . + "ON bugs.qa_contact = map_qa_contact.userid"); $f = "COALESCE(map_$f.login_name,'')"; }, @@ -442,9 +445,9 @@ sub init { $sequence++; } my $table = "user_group_map_$chartseq"; - push(@supptables, "LEFT JOIN cc cc_$chartseq " . + push(@supptables, "LEFT JOIN cc AS cc_$chartseq " . "ON bugs.bug_id = cc_$chartseq.bug_id"); - push(@supptables, "LEFT JOIN user_group_map $table " . + push(@supptables, "LEFT JOIN user_group_map AS $table " . "ON $table.user_id = cc_$chartseq.who " . "AND $table.group_id IN(" . join(',', @childgroups) . ") " . @@ -466,9 +469,9 @@ sub init { $chartseq = "CC$sequence"; $sequence++; } - push(@supptables, "LEFT JOIN cc cc_$chartseq " - . "ON bugs.bug_id = cc_$chartseq.bug_id " - . "AND cc_$chartseq.who = $match"); + push(@supptables, "LEFT JOIN cc AS cc_$chartseq " . + "ON bugs.bug_id = cc_$chartseq.bug_id " . + "AND cc_$chartseq.who = $match"); $term = "cc_$chartseq.who IS NOT NULL"; }, "^cc,(?:notequals),(%\\w+%)" => sub { @@ -478,9 +481,9 @@ sub init { $chartseq = "CC$sequence"; $sequence++; } - push(@supptables, "LEFT JOIN cc cc_$chartseq " - . "ON bugs.bug_id = cc_$chartseq.bug_id " - . "AND cc_$chartseq.who = $match"); + push(@supptables, "LEFT JOIN cc AS cc_$chartseq " . + "ON bugs.bug_id = cc_$chartseq.bug_id " . + "AND cc_$chartseq.who = $match"); $term = "cc_$chartseq.who IS NULL"; }, "^cc,(anyexact|substring)" => sub { @@ -492,12 +495,17 @@ sub init { $sequence++; } if ($list) { - push(@supptables, "LEFT JOIN cc cc_$chartseq ON bugs.bug_id = cc_$chartseq.bug_id AND cc_$chartseq.who IN($list)"); + push(@supptables, "LEFT JOIN cc AS cc_$chartseq " . + "ON bugs.bug_id = cc_$chartseq.bug_id " . + "AND cc_$chartseq.who IN($list)"); $term = "cc_$chartseq.who IS NOT NULL"; } else { - push(@supptables, "LEFT JOIN cc cc_$chartseq ON bugs.bug_id = cc_$chartseq.bug_id"); + push(@supptables, "LEFT JOIN cc AS cc_$chartseq " . + "ON bugs.bug_id = cc_$chartseq.bug_id"); + push(@supptables, + "LEFT JOIN profiles AS map_cc_$chartseq " . + "ON cc_$chartseq.who = map_cc_$chartseq.userid"); - push(@supptables, "LEFT JOIN profiles map_cc_$chartseq ON cc_$chartseq.who = map_cc_$chartseq.userid"); $ff = $f = "map_cc_$chartseq.login_name"; my $ref = $funcsbykey{",$t"}; &$ref; @@ -509,36 +517,37 @@ sub init { $chartseq = "CC$sequence"; $sequence++; } - push(@supptables, "LEFT JOIN cc cc_$chartseq ON bugs.bug_id = cc_$chartseq.bug_id"); + push(@supptables, "LEFT JOIN cc AS cc_$chartseq " . + "ON bugs.bug_id = cc_$chartseq.bug_id"); $ff = $f = "map_cc_$chartseq.login_name"; my $ref = $funcsbykey{",$t"}; &$ref; push(@supptables, - "LEFT JOIN profiles map_cc_$chartseq " . - "ON (cc_$chartseq.who = map_cc_$chartseq.userid " . - "AND ($term))" + "LEFT JOIN profiles AS map_cc_$chartseq " . + "ON (cc_$chartseq.who = map_cc_$chartseq.userid " . + "AND ($term))" ); $term = "$f IS NOT NULL"; }, "^long_?desc,changedby" => sub { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); my $id = &::DBNameToIdAndCheck($v); $term = "$table.who = $id"; }, "^long_?desc,changedbefore" => sub { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); $term = "$table.bug_when < " . &::SqlQuote(SqlifyDate($v)); }, "^long_?desc,changedafter" => sub { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); $term = "$table.bug_when > " . &::SqlQuote(SqlifyDate($v)); }, "^content,matches" => sub { @@ -551,13 +560,14 @@ sub init { # Add the longdescs table to the query so we can search comments. my $table = "longdescs_$chartid"; - push(@supptables, "INNER JOIN longdescs AS $table " . - "ON bugs.bug_id = $table.bug_id"); + my $extra = ""; if (Param("insidergroup") && !&::UserInGroup(Param("insidergroup"))) { - push(@wherepart, "$table.isprivate < 1"); + $extra = "AND $table.isprivate < 1"; } + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON bugs.bug_id = $table.bug_id $extra"); # Create search terms to add to the SELECT and WHERE clauses. # $term1 searches comments. @@ -618,9 +628,9 @@ sub init { if (Param("insidergroup") && !&::UserInGroup(Param("insidergroup"))) { $extra = "AND $table.isprivate < 1"; } - push(@supptables, "LEFT JOIN longdescs $table " - . "ON $table.bug_id = bugs.bug_id $extra " - . "AND $table.who IN ($match)"); + push(@supptables, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id $extra " . + "AND $table.who IN ($match)"); $term = "$table.who IS NOT NULL"; }, "^commenter," => sub { @@ -637,11 +647,15 @@ sub init { $extra = "AND $table.isprivate < 1"; } if ($list) { - push(@supptables, "LEFT JOIN longdescs $table ON $table.bug_id = bugs.bug_id $extra AND $table.who IN ($list)"); + push(@supptables, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id $extra " . + "AND $table.who IN ($list)"); $term = "$table.who IS NOT NULL"; } else { - push(@supptables, "LEFT JOIN longdescs $table ON $table.bug_id = bugs.bug_id $extra"); - push(@supptables, "LEFT JOIN profiles map_$table ON $table.who = map_$table.userid"); + push(@supptables, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id $extra"); + push(@supptables, "LEFT JOIN profiles AS map_$table " . + "ON $table.who = map_$table.userid"); $ff = $f = "map_$table.login_name"; my $ref = $funcsbykey{",$t"}; &$ref; @@ -649,39 +663,40 @@ sub init { }, "^long_?desc," => sub { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); + my $extra = ""; if (Param("insidergroup") && !&::UserInGroup(Param("insidergroup"))) { - push(@wherepart, "$table.isprivate < 1") ; + $extra = "AND $table.isprivate < 1"; } - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id $extra"); $f = "$table.thetext"; }, "^work_time,changedby" => sub { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); my $id = &::DBNameToIdAndCheck($v); $term = "(($table.who = $id"; $term .= ") AND ($table.work_time <> 0))"; }, "^work_time,changedbefore" => sub { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); $term = "(($table.bug_when < " . &::SqlQuote(SqlifyDate($v)); $term .= ") AND ($table.work_time <> 0))"; }, "^work_time,changedafter" => sub { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); $term = "(($table.bug_when > " . &::SqlQuote(SqlifyDate($v)); $term .= ") AND ($table.work_time <> 0))"; }, "^work_time," => sub { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); $f = "$table.work_time"; }, "^percentage_complete," => sub { @@ -703,8 +718,8 @@ sub init { } if ($oper ne "noop") { my $table = "longdescs_$chartid"; - push(@supptables, "longdescs $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "INNER JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); my $field = "(100*((SUM($table.work_time)*COUNT(DISTINCT $table.bug_when)/COUNT(bugs.bug_id))/((SUM($table.work_time)*COUNT(DISTINCT $table.bug_when)/COUNT(bugs.bug_id))+bugs.remaining_time))) AS percentage_complete_$table"; push(@fields, $field); push(@having, @@ -713,18 +728,23 @@ sub init { $term = "0=0"; }, "^bug_group,(?!changed)" => sub { - push(@supptables, "LEFT JOIN bug_group_map bug_group_map_$chartid ON bugs.bug_id = bug_group_map_$chartid.bug_id"); + push(@supptables, + "LEFT JOIN bug_group_map AS bug_group_map_$chartid " . + "ON bugs.bug_id = bug_group_map_$chartid.bug_id"); - push(@supptables, "LEFT JOIN groups groups_$chartid ON groups_$chartid.id = bug_group_map_$chartid.group_id"); + push(@supptables, + "LEFT JOIN groups AS groups_$chartid " . + "ON groups_$chartid.id = bug_group_map_$chartid.group_id"); $f = "groups_$chartid.name"; }, "^attachments\..*," => sub { my $table = "attachments_$chartid"; - push(@supptables, "attachments $table"); + my $extra = ""; if (Param("insidergroup") && !&::UserInGroup(Param("insidergroup"))) { - push(@wherepart, "$table.isprivate = 0") ; + $extra = "AND $table.isprivate = 0"; } - push(@wherepart, "bugs.bug_id = $table.bug_id"); + push(@supptables, "INNER JOIN attachments AS $table " . + "ON bugs.bug_id = $table.bug_id $extra"); $f =~ m/^attachments\.(.*)$/; my $field = $1; if ($t eq "changedby") { @@ -765,11 +785,11 @@ sub init { # a left join here so bugs without any flags still match # negative conditions (f.e. "flag isn't review+"). my $flags = "flags_$chartid"; - push(@supptables, "LEFT JOIN flags $flags " . + push(@supptables, "LEFT JOIN flags AS $flags " . "ON bugs.bug_id = $flags.bug_id " . "AND $flags.is_active = 1"); my $flagtypes = "flagtypes_$chartid"; - push(@supptables, "LEFT JOIN flagtypes $flagtypes " . + push(@supptables, "LEFT JOIN flagtypes AS $flagtypes " . "ON $flags.type_id = $flagtypes.id"); # Generate the condition by running the operator-specific @@ -796,19 +816,19 @@ sub init { }, "^requestees.login_name," => sub { my $flags = "flags_$chartid"; - push(@supptables, "LEFT JOIN flags $flags " . + push(@supptables, "LEFT JOIN flags AS $flags " . "ON bugs.bug_id = $flags.bug_id " . "AND $flags.is_active = 1"); - push(@supptables, "LEFT JOIN profiles requestees_$chartid " . + push(@supptables, "LEFT JOIN profiles AS requestees_$chartid " . "ON $flags.requestee_id = requestees_$chartid.userid"); $f = "requestees_$chartid.login_name"; }, "^setters.login_name," => sub { my $flags = "flags_$chartid"; - push(@supptables, "LEFT JOIN flags $flags " . + push(@supptables, "LEFT JOIN flags AS $flags " . "ON bugs.bug_id = $flags.bug_id " . "AND $flags.is_active = 1"); - push(@supptables, "LEFT JOIN profiles setters_$chartid " . + push(@supptables, "LEFT JOIN profiles AS setters_$chartid " . "ON $flags.setter_id = setters_$chartid.userid"); $f = "setters_$chartid.login_name"; }, @@ -878,8 +898,8 @@ sub init { } } if ($term) { - push(@supptables, "keywords $table"); - push(@wherepart, "$table.bug_id = bugs.bug_id"); + push(@supptables, "LEFT JOIN keywords AS $table " . + "ON $table.bug_id = bugs.bug_id"); } }, @@ -888,7 +908,7 @@ sub init { $ff = "$table.$f"; my $ref = $funcsbykey{",$t"}; &$ref; - push(@supptables, "LEFT JOIN dependencies $table " . + push(@supptables, "LEFT JOIN dependencies AS $table " . "ON $table.blocked = bugs.bug_id " . "AND ($term)"); $term = "$ff IS NOT NULL"; @@ -899,7 +919,7 @@ sub init { $ff = "$table.$f"; my $ref = $funcsbykey{",$t"}; &$ref; - push(@supptables, "LEFT JOIN dependencies $table " . + push(@supptables, "LEFT JOIN dependencies AS $table " . "ON $table.dependson = bugs.bug_id " . "AND ($term)"); $term = "$ff IS NOT NULL"; @@ -929,11 +949,11 @@ sub init { my $cutoff = "NOW() - " . $dbh->sql_interval("$quantity $unitinterval"); my $assigned_fieldid = &::GetFieldID('assigned_to'); - push(@supptables, "LEFT JOIN longdescs comment_$table " . + push(@supptables, "LEFT JOIN longdescs AS comment_$table " . "ON comment_$table.who = bugs.assigned_to " . "AND comment_$table.bug_id = bugs.bug_id " . "AND comment_$table.bug_when > $cutoff"); - push(@supptables, "LEFT JOIN bugs_activity activity_$table " . + push(@supptables, "LEFT JOIN bugs_activity AS activity_$table " . "ON (activity_$table.who = bugs.assigned_to " . "OR activity_$table.fieldid = $assigned_fieldid) " . "AND activity_$table.bug_id = bugs.bug_id " . @@ -1024,7 +1044,7 @@ sub init { if (!$fieldid) { ThrowCodeError("invalid_field_name", {field => $f}); } - push(@supptables, "LEFT JOIN bugs_activity $table " . + push(@supptables, "LEFT JOIN bugs_activity AS $table " . "ON $table.bug_id = bugs.bug_id " . "AND $table.fieldid = $fieldid " . "AND $table.bug_when $operator " . @@ -1038,7 +1058,7 @@ sub init { if (!$fieldid) { ThrowCodeError("invalid_field_name", {field => $f}); } - push(@supptables, "LEFT JOIN bugs_activity $table " . + push(@supptables, "LEFT JOIN bugs_activity AS $table " . "ON $table.bug_id = bugs.bug_id " . "AND $table.fieldid = $fieldid " . "AND $table.$operator = $q"); @@ -1051,7 +1071,7 @@ sub init { ThrowCodeError("invalid_field_name", {field => $f}); } my $id = &::DBNameToIdAndCheck($v); - push(@supptables, "LEFT JOIN bugs_activity $table " . + push(@supptables, "LEFT JOIN bugs_activity AS $table " . "ON $table.bug_id = bugs.bug_id " . "AND $table.fieldid = $fieldid " . "AND $table.who = $id"); @@ -1291,7 +1311,7 @@ sub init { my @supplist = (" "); foreach my $str (@supptables) { if (!$suppseen{$str}) { - if ($str =~ /^(LEFT|INNER) JOIN/i) { + if ($str =~ /^(LEFT|INNER|RIGHT)\s+JOIN/i) { $str =~ /^(.*?)\s+ON\s+(.*)$/i; my ($leftside, $rightside) = ($1, $2); if ($suppseen{$leftside}) { @@ -1301,8 +1321,9 @@ sub init { push @supplist, " $leftside ON ($rightside)"; } } else { - $suppstring .= ", $str"; - $suppseen{$str} = 1; + # Do not accept implicit joins using comma operator + # as they are not DB agnostic + ThrowCodeError("comma_operator_deprecated"); } } } diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index 5e75e1b30..b4b4dcbd0 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -260,6 +260,11 @@ [% ELSIF error == "no_matching_lock" %] Attempted to unlock tables without locking them first. + [% ELSIF error == "comma_operator_deprecated" %] + [% title = "SQL query generator internal error" %] + There is an internal error in the SQL query generation code, + creating queries with implicit JOIN. + [% ELSE %] [% title = "Internal error" %] An internal error has occured, but [% terms.Bugzilla %] doesn't know -- cgit v1.2.3-24-g4f1b