bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Advanced search results sorted incorrectly when sorted by importance

VERIFIED FIXED in Bugzilla 3.0

Status

()

Bugzilla
Query/Bug List
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: Little Nemo, Assigned: Frédéric Buclin)

Tracking

3.0.1
Bugzilla 3.0
Bug Flags:
approval +
approval3.0 +

Details

Attachments

(1 attachment, 1 obsolete attachment)

789 bytes, patch
justdave
: review+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
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.
(Assignee)

Comment 1

11 years ago
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.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 2

11 years ago
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.
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
(Assignee)

Comment 3

11 years ago
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
Severity: major → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Version: unspecified → 3.0.1
(Assignee)

Comment 4

11 years ago
Created attachment 280477 [details] [diff] [review]
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!
Assignee: query-and-buglist → LpSolit
Status: NEW → ASSIGNED
Attachment #280477 - Flags: review?(wicked)
(Assignee)

Updated

11 years ago
Target Milestone: --- → Bugzilla 3.0
(Reporter)

Comment 5

11 years ago
Sorry for the pain. I appreciate the effort.
(Reporter)

Comment 6

11 years ago
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.
(Assignee)

Comment 7

11 years ago
(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 on attachment 280477 [details] [diff] [review]
patch, v1

I like this fix.  Potentially fixes other things besides this one, too.
Attachment #280477 - Flags: review?(wicked) → review+
(Assignee)

Updated

11 years ago
Flags: approval3.0+
Flags: approval+
(Assignee)

Comment 9

11 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
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

 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 11

11 years ago
Created attachment 281727 [details] [diff] [review]
patch, v2

Passing SQL fragments to buglist.cgi is definitely a weird idea.
Attachment #280477 - Attachment is obsolete: true
Attachment #281727 - Flags: review?(justdave)
Comment on attachment 281727 [details] [diff] [review]
patch, v2

yup, this is safer.
Attachment #281727 - Flags: review?(justdave) → review+
(Assignee)

Comment 13

11 years ago
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
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 14

11 years ago
(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.
(Reporter)

Comment 15

9 years ago
Correction of the described sort problem is verified.

(I am sure I should have verified this long ago.)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.