395771 - Advanced search results sorted incorrectly when sorted by importance

Status: VERIFIED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Little Nemo]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4
Build Identifier: 3.0.1

On the Advanced Search page, when I choose "Importance" at the "Sort results by:" option, the resulting bug list is not sorted as it was in version 2.18 (the last version I used). Specifically, the secondary sort on the Severity column is an alphabetic sort, rather than a sort on the sortkey.

Reproducible: Always

Steps to Reproduce:
1. Using Advanced Search, specify a search that will return a wide variety of bugs.
2. Be sure to set "Sort results by:" option to "Importance."
3. Click the Search button.
Actual Results:  
Listed bugs are sorted by Priority (primary) and by Severity (secondary).  The Severity column is sorted alphabetically. This puts enhancements ahead of major bugs. It puts minor bugs ahead of normal bugs.

Expected Results:  
The Severity column should be sorted by sortkey, so that normal bugs precede minor bugs, and so that bugs of all severities precede enhancements.

I don't know how this sort has been handled in version 2.20 or 2.22, but in version 2.18, the Severity sequence (within each Priority level) was: blocker, critical, major, normal, minor, trivial, enhancement. Changing sortkey values (which version 3.0 permits, but version 2.18 did not permit) does not solve this problem.

Comment 1

[Frédéric Buclin]

Here is the SQL fragment used to order bugs:

ORDER BY priority.sortkey,priority.value,bug_severity.sortkey,bug_severity.value

This is the same order list in both 3.0.1 and 3.1.1. So they are correctly ordered by sortkey and is really what I see in my buglists.

Comment 2

[Little Nemo]

I would like to give you a specific case, and let you see if your results are consistent with mine.

Using http://bugzilla.mozilla.org (so that we will not have differences attributable to different installations), go to the Advanced Search page. Specify a search as follows:
1) Under Product, select Bugzilla
2) Under Version select 2.23 thru 3.1.1 inclusive
3) Under Resolution, deselect all (or select all)
4) At Priority, select P1 thru P5 inclusive
5) At "Sort results by:" select Importance.
This should return about 123 bugs.

The bugs are sorted primarily by Priority. Within each Priority, the Severity sort sequence (in my results set) is:
critical
enhancement
major
minor
normal
trivial
(there are no blockers in this results set). Is this consistent with what you see? Note that the sequence is in alphabetic order.

I believe that within each Priority, the Severity sort sequence ought to be:
blocker
critical
major
normal
minor
trivial
enhancement
as they were in v2.18.

Comment 3

[Frédéric Buclin]

OK, here is what I found:

The reporter is right that the "Importance" order sorts the severity alphabetically. The reason I said it was sorted by the sortkey (as it should be) is because changing columns (e.g. to add the priority and severity columns, but simply visiting colchange.cgi works too) reorders bugs correctly, i.e. based on the sortkey. As I indeed changed the list of columns while testing, my list was correctly reordered. Sorry Little Nemo. :)

Also, using QuickSearch seems to display bugs as expected, i.e. based on the sortkey. joel, Teemu, what's so magic in QS and colchange that the "default" query hasn't?

Adding debug=1 to the URL shows that QS and the "updated" list have:

 LEFT JOIN bug_severity ON (bug_severity.value = bugs.bug_severity) + ORDER BY priority.sortkey,priority.value,bug_severity.sortkey,bug_severity.value

while the "default" list doesn't have this LEFT JOIN and the order is ORDER BY priority.sortkey,priority.value,bugs.bug_severity

Comment 4

[Frédéric Buclin]

Attachment: patch, v1

OK, I could figure it myself. buglist.cgi has:
        /^Importance$/ && do {
            $order = "bugs.priority, bugs.bug_severity";

But later in the code, we have:
$db_order = $order;
my @orderstrings = split(',', $db_order);

my $search = new Bugzilla::Search('fields' => \@selectnames, 
                                  'params' => $params,
                                  'order' => \@orderstrings);

And so Search.pm gets " bugs.bug_severity" instead of "bugs.bug_severity". This leading whitespace confuses both BuildOrderBy(), which is unable to parse it correctly, and %special_order_join which doesn't recognize " bugs.bug_severity" as a known key as its key is "bugs.bug_severity", and so doesn't LEFT JOIN the bug_severity table, as reported in my previous comment. Wow, that was painful to debug!

Comment 5

[Little Nemo]

Sorry for the pain. I appreciate the effort.

Comment 6

[Little Nemo]

Unless I overlooked something, the patch fixing this bug was not included in Tuesday’s 3.0.2 release.

This is the very first bug that I have reported to any Mozilla project, so my expectations are not informed by experience.  Will this patch work its way into a near-future release automatically?  Or am I expected to first incorporate the patch into my local Bugzilla and report my results?

I am flying blind here.  I hope that more experienced participants will share their wisdom.

Comment 7

[Frédéric Buclin]

(In reply to comment #6)
> Unless I overlooked something, the patch fixing this bug was not included in
> Tuesday’s 3.0.2 release.

Correct. That's because wicked didn't review this patch yet, and so it cannot be approved for checkin. I will try to find someone else to review this one-liner and have it for Bugzilla 3.0.3. Note that you can apply my patch on your local 3.0.2 installation meanwhile.

Comment 8

[Dave Miller [:justdave]]

Comment on attachment 280477 [details] [diff] [review]
patch, v1

I like this fix.  Potentially fixes other things besides this one, too.

Comment 9

[Frédéric Buclin]

tip:

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.363; previous revision: 1.362
done

3.0.2:

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.351.2.6; previous revision: 1.351.2.5
done

Comment 10

[Dave Miller [:justdave]]

Grr.  Guess I overlooked something... this broke landfill when it deployed.

DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'desc LIMIT 200' at line 1 [for Statement "..... ORDER BY relevance,desc LIMIT 200"] at /var/www/html/bugzilla-3.0-branch/buglist.cgi line 1003

 

Comment 11

[Frédéric Buclin]

Attachment: patch, v2

Passing SQL fragments to buglist.cgi is definitely a weird idea.

Comment 12

[Dave Miller [:justdave]]

Comment on attachment 281727 [details] [diff] [review]
patch, v2

yup, this is safer.

Comment 13

[Frédéric Buclin]

tip:

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.364; previous revision: 1.363
done

3.0.2:

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.351.2.7; previous revision: 1.351.2.6
done

Comment 14

[Little Nemo]

(In reply to comment #7)
> I will try to find someone else to review this
> one-liner and have it for Bugzilla 3.0.3. Note that you can apply my patch on
> your local 3.0.2 installation meanwhile.

I look forward to version 3.0.3, and I thank you very much.

Comment 15

[Little Nemo]

Correction of the described sort problem is verified.

(I am sure I should have verified this long ago.)