SQL standard requires that any column referenced in the HAVING clause be in the GROUP BY clause or be the argument of some aggregate function. This is done to make sure it is a scalar expression. This touches on many cgi's throught out Bugzilla. Patch coming up.

Comment on attachment 102763 [details] [diff] [review] Patch to make GROUP BY more standard compliant (v1) The vast majority of these sql statements don't have HAVING clauses This is real ugly - can we do better, w/o subselects?

Having a HAVING clause is not required for this to be in effect. With PostgreSQL and Oracle, if you have once or more columns in the SELECT that are part of an aggregate function, then all other columns must be listed in a GROUP BY clause. I am sure this could be nicer with subselects once Mysql supports them in the next release. Otherwise this is a good interim solution that works with multiple db's.

Comment on attachment 102763 [details] [diff] [review] Patch to make GROUP BY more standard compliant (v1) This turns out to be necessary for Sybase as well. :) This all looks really good except for Search.pm. In the two months since this patch was posted, there's been too many changes to Search.pm, and that one hunk of the patch is completely bitrotted, and I can't figure out how to unrot it. I did do a quick hack on that one file that got it working on Sybase, but I don't think it's production quality so I'd rather have you try your magic on it again. :-)

Just for reference, here's what I ended up doing: Index: Bugzilla/Search.pm =================================================================== RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v retrieving revision 1.37 diff -u -r1.37 Search.pm --- Bugzilla/Search.pm 22 Nov 2002 02:38:18 -0000 1.37 +++ Bugzilla/Search.pm 10 Dec 2002 10:30:05 -0000 @@ -949,7 +949,9 @@ } } - $query .= ") GROUP BY bugs.bug_id"; + my $group_by = join(', ',@fields); + $group_by =~ s/\w+\s+AS\s+//gsi; + $query .= ") GROUP BY $group_by"; if (@having) { $query .= " HAVING " . join(" AND ", @having);

*** Bug 247998 has been marked as a duplicate of this bug. ***

dkl's group-by patch unmodified from bug 247998

Comment on attachment 151416 [details] [diff] [review] Group by conversion patch >Index: Bugzilla/Search.pm + if ($fieldsref) { >+ foreach my $field (@$fieldsref) { >+ next if ($field =~ /(SUM|COUNT)/i); >+ push(@groupby, $field); >+ } >+ } I don't think this is a good way to exclude fields... we have columns with "count" as a substring of the column name, for example. Perhaps check for the open parenthesis after it as well? next if ($field =~ /(SUM|COUNT)\s*\(/i);

Changed code in Search.pm that looked for aggregate functions to match justdave's suggestion. my @groupby; if ($fieldsref) { foreach my $field (@$fieldsref) { next if ($field =~ /(trim|sum|count|length|lower|upper|to_char|date_format)\s*\(/i); push(@groupby, $field); } } Also added other possibilities such as upper, lower, to_char, etc.

Comment on attachment 151455 [details] [diff] [review] Group by conversion patch (v2) This looks OK, but 2 questions before I r= this.... 1) Has this been tested on mysql and/or oracle with timetracking enabled and displayed in buglist? 2) Is there any performance hit on mysql due to the extra columns in the group_by clause?

I have a test database setup with 5-6 test bugs running on Mysql setup. I have enabled timetracking and added some time values to several of the bugs. When adding the appropriate time columns to the buglist.cgi list, it displays the values properly with this patch attached. As for performance, someone else will have to try it out with Mysql and I will test it with out PgSQL database here to see if any slowdowns are noticed.

(In reply to comment #9) > next if ($field =~ > /(trim|sum|count|length|lower|upper|to_char|date_format)\s*\(/i); > Also added other possibilities such as upper, lower, to_char, etc. hmmm.... how safe would it be to do something along the lines of next if ($field =~ /^\s*\w+\s*\(/); ? Is it safe to assume that any one single word with an open paren after it is going to be a function?

Comment on attachment 151455 [details] [diff] [review] Group by conversion patch (v2) r=joel if you try this on postgres with timetracking enabled and buglisting with %complete as a field and if you change join(',', @groupby) to join(', ', @groupby) [so the debug=1 output can fit on the screen]

New patch with joels suggestion for proper separation in the join() part. Also changed to check for values in @groupby before adding to query string just in case we only select bugs.bug_id and no columns. As for testing, can someone apply the patch that has a database with quite a bit of test bugs in it and make sure that searches work properly with all of the timetracking columns enabled? I have done so myself but I only have a few reports in my test MySQL database.

I'm not sure if you want to make this change, but.... You can simplify things by putting bug_id in @groupby before you start adding anything else. Then, you just have to unconditionally append the joined list.

That does seem cleaner and like everyone says about Perl in general, there is always more than one way to do it ;) Changed patch to remove groupby condition and explicitly added bugs.bug_id to the groupby list.

Comment on attachment 151536 [details] [diff] [review] Group by converstion patch (v4) r=joel Tested on mysql including timetracking columns

Comment on attachment 151536 [details] [diff] [review] Group by converstion patch (v4) whoops! not so fast.... Unknown column '42217354' in 'group statement' when we run a graphical report. Looks like we need to kill numeric fields from the @groups list as well.

Skip numeric column names when forming @groupby. Fixes the problem with charts for me in my testing.

Comment on attachment 151722 [details] [diff] [review] Group by converstion patch (v5) r=joel Same assumption... you've tested in on PG with timetracking fields in the buglist.

Comment on attachment 151722 [details] [diff] [review] Group by converstion patch (v5) Seeking second review

Comment on attachment 151722 [details] [diff] [review] Group by converstion patch (v5) >Index: Bugzilla/FlagType.pm >=================================================================== >- $query .= " GROUP BY flagtypes.id " if ($include_count || $having ne ""); >+ $query .= " GROUP BY flagtypes.id, " . join(",", @base_columns) if ($include_count || $having ne ""); This produces "GROUP BY flagtypes.id, 1,flagtypes.id,flagtypes.name, ..." The 1 didn't give an error for me, but a similar one did for Joel (comment 18). Also, you have flagtypes.id twice (which isn't a real error though). Nit: The line exceeds 80 chars. >Index: Bugzilla/Search.pm >+ next if ($field =~ /(trim|sum|count|length|lower|upper|to_char|date_format)\s*\(/i || $field =~ /^\d+$/); Nit: Another line too long. Tested on MySQL 4.0.18.

I had problems with Search.pm with this patch, as it was not correctly removing "relevance" aggregate and Postgres was complaining. I had to move the loop to the end (just before query construction) and modified it to use $fields, instead of $fieldsref. I ended up with this (using older version, with just SUM and COUNT): Index: Search.pm =================================================================== RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v retrieving revision 1.60 diff -u -r1.60 Search.pm --- Search.pm 17 Jul 2004 01:33:51 -0000 1.60 +++ Search.pm 18 Jul 2004 06:17:29 -0000 @@ -1166,7 +1166,16 @@ } } - $query .= ") GROUP BY bugs.bug_id"; + # SQL standard requires that any column referenced in the HAVING + # clause be in the GROUP BY clause or be the argument of some aggregate + # function. This is done to make sure it is a scalar expression. + my @groupby; + foreach my $field (@fields) { + next if ($field =~ /(SUM|COUNT)/i); + push(@groupby, $field ); + } + + $query .= ") GROUP BY " . join(',', @groupby); if (@having) { $query .= " HAVING " . join(" AND ", @having);

bug 264249 also makes this conversion for Bugzilla/Series.pm

Changes for editclassifications.cgi, Series.pm and Chart.pm added in. Also moved @groupby down as per Tom's suggestion. Shortening the regex is still needed in Search.pm if someone can suggest the best way to do that without just using the multiline regex switch (m).

Comment on attachment 162020 [details] [diff] [review] Group by converstion patch (v6) >Index: Bugzilla/FlagType.pm >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v >retrieving revision 1.9 >diff -u -r1.9 FlagType.pm >--- Bugzilla/FlagType.pm 4 Aug 2004 16:17:10 -0000 1.9 >+++ Bugzilla/FlagType.pm 13 Oct 2004 22:49:28 -0000 >@@ -135,7 +135,7 @@ > my $where_clause = "WHERE " . join(" AND ", @criteria); > > my $query = "$select_clause $from_clause $where_clause"; >- $query .= " GROUP BY flagtypes.id " if ($include_count || $having ne ""); >+ $query .= " GROUP BY flagtypes.id, " . join(",", @base_columns) if ($include_count || $having ne ""); *nit* The precedence of the if and the "." are a little unclear, there. You might want to use parens to make sure that it's clear what we're doing there. >- $query .= ") GROUP BY bugs.bug_id"; >+ # SQL standard requires that any column referenced in the HAVING >+ # clause be in the GROUP BY clause or be the argument of some aggregate >+ # function. This is done to make sure it is a scalar expression. >+ my @groupby; >+ foreach my $field (@fields) { >+ next if ($field =~ /(trim|sum|count|length|lower|upper|to_char|date_format)\s*\(/i || $field =~ /^\d+$/); *nit* Can't we just search for parens? Is there any valid real field that will actually have a paren? All the rest of this looks "obviously correct" to me. If you assert that it works without bugs, I'm willing to give it a review+.

Comment on attachment 162020 [details] [diff] [review] Group by converstion patch (v6) Tomas, could you also give this patch a review?

(In reply to comment #27) > (From update of attachment 162020 [details] [diff] [review] [edit]) > Tomas, could you also give this patch a review? > Sorry, I don't have the rights to modify the attachment to set the flag, so here just goes my comments: I am still not completely sure this is the best way to tackle this problem. The main reason for my concern is this comment (https://bugzilla.mozilla.org/show_bug.cgi?id=173130#c3): "Also, ANSI SQL says if we use GROUP BY, that we have to include every column mentioned in the SELECT part in the GROUP BY clause. Sybase won't let you put more than 31 columns in a GROUP BY, and also has temporary table size limitations with GROUP BY that are pretty darn restrictive. So the "GROUP BY bugs.bug_id" has been replaced with "DISTINCT bugs.bug_id" at the beginning of the SELECT part." DISTINCT will not help for Postgres, we need all the columns in GROUP BY. But if it breaks Sybase, it may break others. Though this patch is a good short term solution for MySQL and Postgres, I would like to be as generic as possible. So I was thinking about alternatives, and so far the best solution appears to be a new function in DBcompat layer (once we have it in :-) ), taking two parameters, first string (or array) with columns which are mandatory, present for all DBs, and second with optional columns, present for Postgres and similar. The function would return the first param for MySQL and Sybase, and both concatenated for Postgres. Note this probably affects only Search.pm monster, all the other are quite simple and can go in as they are (although Bug.pm with 29 columns is close). Also with Search.pm, I sort of don't like the way we are first putting all neccessary columns to a list of fields, and then analyzing this list and trying to separate columns which needs to go to the GROUP BY statement. Are we sure we are not missing any column? I am not :-). It looks we are just complicating the thing more than it needs to be. Wouldn't it be possible e.g. to create a new list, where we would build the list of group_by columns in the same as we are currently doing with e.g. order_by? Functionality wise, I am running my code with this patch for a while now and it seems to work fine. The only thing I have added is one more change in Bugzilla/User.pm, function get_userlist. My GROUP BY "have userid, login_name, realname" instead of just "userid", otherwise when you turn "usemenuforusers" option on, Postgres will fail on bug creation. If the Bugzilla/User.pm gets fixed, I am giving the patch r+ except for the Search.pm part, which gets r-. I would even recommend taking the Search.pm changes to a new bug, so we can check in the rest here...

Also see discussion at bug 173130. There are some ideas how to cope with the GROUP BY problem in Search.pm.

Comment on attachment 162020 [details] [diff] [review] Group by converstion patch (v6) OK, that's an r- from Tomas, although it's an r+ on certain parts.

Well, we should probably give this another shot, this needs to get fixed. Ideas, anyone?

Ok, I finally managed to run some tests on MySQL: I created two tables, bugs and flags, both with 300.000 rows. I run two queries similar to what are generating in search module, left-joining these two tables, and selecting all columns with a GROUP BY and HAVING statements. First query was GROUPing only by bugs.bug_id (thats what the code is doing at the moment), the second by all columns of bugs table (thats what Pg needs). I run both queries six times, discarding first run. The first query took around 84 seconds, the second around 378 seconds. That corresponds what MySQL documentation is suggesting - less columns is less sorting and thus better performance. So, although specifying all columns is ANSI standard, we should not penalize MySQL performance for that. My suggestion is: Lets create a new method in the DB compatibility layer, sql_group_by(), taking two parameters. First parameter will be a list of all columns we need to group by (that's what we are using now), the second will be list of all the remaining columns. DB base class will join these lists together and do GROUP BY containing all columns, as ANSI SQL requires. MySQL specialization will override this method and will return just the first list, discarding the second, so we'll end up with the same statement as we have now. This also addresses the problem with Sybase not supporting more than certain number of columns in GROUP BY statement. Also, for Search.pm, I would suggest to construct the two lists during query construction, and not as a post-procesing, by parsing the resulting query. It should be more obvious what we are doing. Although I do not insist on this :-). Comments? DKL: Will you have some time to look at this in a very near future? We don't have much time before freeze, but I don't want to "steal" this from you without even asking :-).

(In reply to comment #32) > Also, for Search.pm, I would suggest to construct the two lists during query > construction, and not as a post-procesing, by parsing the resulting query. It > should be more obvious what we are doing. Although I do not insist on this :-). I just looked at the implementation and I realized that the columns are coming in as a parameter, they are not constructed in Search.pm. So the current loop parsing the columns is probably inevitable. So ignore my suggestion above :-). We want to keep that loop, we just want to remove bugs.bug_id from the list also (as it will be explicit first parameter to sql_group_by method and we don't want duplicates).

We really need to get this in, so I am taking it. I have implemented a new DB method sql_group_by, as I described it before, and modified the code to use it everywhere. Everything works ok, except for Search.pm. Current solution with a loop parsing all fields coming to the query seems to be the only viable at the moment, but it currently does not work correctly, as we can't simply ignore all fields with functions. - First, the exception for GROUP BY holds only for aggregate functions, such as SUM or COUNT. Functions as LOWER does not qualify, we still need to group by the column used as a parameter for the function. - Moreover, some of the fields contain expresstion with aggregate functions and normal columns. In this case, we can leave out the columns used in the aggregate functions, but we still need to inglude the other columns. These problems seems to affect time tracking (expression "(100*((SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id))/((SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id))+bugs.remaining_time))) AS percentage_complete" where bugs.remaining_time is not used in aggregate function), and full text search (expression "(SUM($term1)/COUNT($term1) + $term2) AS relevance" where $term2 is not used in aggregate function, and it corresponds to full text search in bugs.short_desc colum). Timetracking seems to add the bugs.remaining_time column to fields, when used (buglist.cgi: # remaining and actual_time are required for precentage_complete calculation: if (lsearch(\@displaycolumns, "percentage_complete") >= 0) { push (@selectcolumns, "remaining_time"); push (@selectcolumns, "actual_time"); } ), so the only problem remains with full text search, which is, fortunatelly simple to fix. I will post the patch later today. It's a solution which is prone to problems in the future, such as if we add more expressions like the timetracking one for display in the buglist page. However, I can't see any better solution without redesigning the whole search class (it probably should be done, as passing DB columns as parameters is not very object oriented approach, better would be to set some flags what we want to search for and Search class would create the query based on the flags). But that's definitelly outside of scope of this bug.

OK, here is my version. Tested on MySQL, very limited testing on Postgres.

Comment on attachment 176708 [details] [diff] [review] V7 OK, this *looks* good to me, but I need Joel to comment on the Search.pm stuff. :-)

+ next if ($field =~ /(AVG|SUM|COUNT|MAX|MIN|VARIANCE)\s*\(/i || Is this missing an anchor??

Comment on attachment 176708 [details] [diff] [review] V7 Close, but please check the anchor situation and try this in pg with the realname column included. I suspect you will need to break the CASE WHEN into both mentioned fields.

(In reply to comment #38) > (From update of attachment 176708 [details] [diff] [review] [edit]) > Close, but please check the anchor situation and try this in pg with the > realname column included. I suspect you will need to break the CASE WHEN into > both mentioned fields. > I do not understand what you mean by anchor. If you are questioning the regexp, I don't see anything wrong with it... With respect to the CASE WHEN, you are right, good catch, thanks. I will fix that.

The anchor question is .... should this be.... /(AVG|SUM|COUNT|MAX|MIN|VARIANCE)\s*\(/i or /^(AVG|SUM|COUNT|MAX|MIN|VARIANCE)\s*\(/i I am being a bit paranoid here. Changing this is optional but suggested IMHO.

OK, new version, fixed the CASE WHEN problem, no other changes. (In reply to comment #40) > The anchor question is .... should this be.... > /(AVG|SUM|COUNT|MAX|MIN|VARIANCE)\s*\(/i > or > /^(AVG|SUM|COUNT|MAX|MIN|VARIANCE)\s*\(/i > > I am being a bit paranoid here. Changing this is optional but suggested IMHO. > > Ahhh, this anchor :-). I think it's better as it is, we want to get rid of anything using any of the aggregate functions, as e.g. "(100*((SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id))/((SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id))+bugs.remaining_time))) AS percentage_complete". Second option would be to look for all these full expressions as we are currently using them, that has the advantage that we can throw code error if we hit expression we do not recognize (if someone edits the code and does not update the group by clause). But listing all of them here would be quite messy IMHO.

Comment on attachment 176871 [details] [diff] [review] V7.1 r=joel I can't think of a better way to do it at the moment, but I am a bit worried that the CASE WHEN handling is rather specific to the current situation with realname. If someone adds another such column, we may have to revisit this, probably by adding something to the column definition table.

Yes, a better solution would be to somehow get a recursive function and pass the contents of the CASE WHEN into it. Or, even better, require all CASE WHEN statements to have an alias, and just use the alias in the GROUP BY statement (if all our DBs allow that). However, this is good enough for now.

(In reply to comment #42) > (From update of attachment 176871 [details] [diff] [review] [edit]) > r=joel > I can't think of a better way to do it at the moment, but I am a bit worried > that the CASE WHEN handling is rather specific to the current situation with > realname. > If someone adds another such column, we may have to revisit this, probably by > adding something to the column definition table. > Yeah, that's what I was describing in comment #34. It would be great to do it in a generic way, but it can't be done without redesigning Search.pm interface (either by adding a column, or by switching from direct SQL to something more abstract, which is IMHO even cleaner solution). But that's really a different bug.

(In reply to comment #43) > Yes, a better solution would be to somehow get a recursive function and pass the > contents of the CASE WHEN into it. > > Or, even better, require all CASE WHEN statements to have an alias, and just use > the alias in the GROUP BY statement (if all our DBs allow that). > > However, this is good enough for now. Hmmm, that brings an idea... If we require all expressions to have an alias (and use the AS keyword), we can separate the alias name by very simple regex. That would solve all problems, and we could even drop the regexp for aggregate functions. And it would be more generic... So the only question is - is it supported by all DBs? I just tested that on Pg8.0 and it works. I will test it on 7.4 tonight. Can someone test it on 7.3? Does someone know what will Oracle or Sybase do with aliases as parameters to the GROUP BY clause?

Hey Ed, do you know the answer to Tomas's question in comment 45? Tomas: Re Pg 7.3, I know that aliases work in the ORDER BY, so I'd assume that they work in the GROUP BY...

(In reply to comment #45) > Does someone know what will Oracle or Sybase do with aliases as parameters to > the GROUP BY clause? Don't know about Oracle, but, as coincidence would have it, I was just writing some code for work the other day to do something very similar to what you are describing in Sybase and I came to the same conclusion. It's a lot cleaner if you require all expressions to be aliased. Anyway, for Sybase, if you alias an expression in the SELECT clause, then you must use the alias in the GROUP BY clause. I hope that answers your question, at least as far as Sybase goes.

OK, I am convinced now, I will change the Search.pm part to use aliases. It seems to be the best way to go.

(In reply to comment #48) > OK, I am convinced now, I will change the Search.pm part to use aliases. It > seems to be the best way to go. Hmmm, I implemented that using aliases, it was nice, simple, clean. But postgres said: "ERROR: aggregates not allowed in GROUP BY clause" and that's the end of the story. So here is a partial version, still skipping aggregate functions, but the CASE WHEN case is handled by aliases, as Max suggested. It's still a bit better than the previous version though.

Comment on attachment 176996 [details] [diff] [review] V7.2 I'd be a little happier if if ($field =~ /.*AS\s+(\w+)$/i) { took advantage of the fact that we know AS is upper case and has whitespace before it as well. Why not /\s+AS\s+(\w+)$/ without the case insensitivity? You can change that or not r=joel

(In reply to comment #50) > (From update of attachment 176996 [details] [diff] [review] [edit]) > I'd be a little happier if > if ($field =~ /.*AS\s+(\w+)$/i) { > > took advantage of the fact that we know AS is upper case and has whitespace > before it as well. Why not /\s+AS\s+(\w+)$/ without the case insensitivity? > > You can change that or not > r=joel > New version. I have added the white-space before the AS in the regex as you suggest. I don't want to make it case insensitive, as there are still places where SQL is lowercase (it's probably not in the AS which are coming to Search.pm, but I want to play safe - upper case is not required by SQL and if someone uses it in some new code, it would produce strange looking errors). I have also fixed full text search - it uses aggregate functions as well, so we can't use alias, we need explicit column name. No other changes done.

Comment on attachment 177052 [details] [diff] [review] V7.3 r=joel Careful on checkin - there were a few glitches in appliying the patch.

Yeah, the DB::Mysql part of the patch was corrupted, but I fixed it on checkin. Checking in buglist.cgi; /cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v <-- buglist.cgi new revision: 1.288; previous revision: 1.287 done Checking in describekeywords.cgi; /cvsroot/mozilla/webtools/bugzilla/describekeywords.cgi,v <-- describekeywords.cgi new revision: 1.14; previous revision: 1.13 done Checking in editclassifications.cgi; /cvsroot/mozilla/webtools/bugzilla/editclassifications.cgi,v <-- editclassifications.cgi new revision: 1.9; previous revision: 1.8 done Checking in editcomponents.cgi; /cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v <-- editcomponents.cgi new revision: 1.51; previous revision: 1.50 done Checking in editkeywords.cgi; /cvsroot/mozilla/webtools/bugzilla/editkeywords.cgi,v <-- editkeywords.cgi new revision: 1.26; previous revision: 1.25 done Checking in editmilestones.cgi; /cvsroot/mozilla/webtools/bugzilla/editmilestones.cgi,v <-- editmilestones.cgi new revision: 1.35; previous revision: 1.34 done Checking in editproducts.cgi; /cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi new revision: 1.75; previous revision: 1.74 done Checking in editusers.cgi; /cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v <-- editusers.cgi new revision: 1.83; previous revision: 1.82 done Checking in request.cgi; /cvsroot/mozilla/webtools/bugzilla/request.cgi,v <-- request.cgi new revision: 1.20; previous revision: 1.19 done Checking in sanitycheck.cgi; /cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v <-- sanitycheck.cgi new revision: 1.90; previous revision: 1.89 done Checking in summarize_time.cgi; /cvsroot/mozilla/webtools/bugzilla/summarize_time.cgi,v <-- summarize_time.cgi new revision: 1.3; previous revision: 1.2 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.69; previous revision: 1.68 done Checking in Bugzilla/Chart.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Chart.pm,v <-- Chart.pm new revision: 1.7; previous revision: 1.6 done Checking in Bugzilla/DB.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB.pm,v <-- DB.pm new revision: 1.32; previous revision: 1.31 done Checking in Bugzilla/Flag.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v <-- Flag.pm new revision: 1.32; previous revision: 1.31 done Checking in Bugzilla/FlagType.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v <-- FlagType.pm new revision: 1.11; previous revision: 1.10 done Checking in Bugzilla/Search.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v <-- Search.pm new revision: 1.91; previous revision: 1.90 done Checking in Bugzilla/Series.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Series.pm,v <-- Series.pm new revision: 1.9; previous revision: 1.8 done Checking in Bugzilla/DB/Mysql.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v <-- Mysql.pm new revision: 1.7; previous revision: 1.6 done

(In reply to comment #53) > Yeah, the DB::Mysql part of the patch was corrupted, but I fixed it on checkin. Max, for some reason, Bugzilla/User.pm has not been checked in. Can you, please, re-apply the patch to this file and check it in?

OK. Turns out the User.pm part of the patch was corrupted, which is why diff thought the User.pm part was corrupted (it's immediately afterward). It was just missing one line of context on the end. I fixed it and checked it in. Checking in Bugzilla/User.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v <-- User.pm new revision: 1.49; previous revision: 1.48 done