[PostgreSQL] Crash when changing the displayed columns after a "specific" search

RESOLVED FIXED in Bugzilla 2.22

Status

()

Bugzilla
Query/Bug List
--
major
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: justdave)

Tracking

2.22
Bugzilla 2.22
Bug Flags:
approval2.22 +
blocking2.22.2 +
blocking2.22.1 -

Details

(Whiteboard: [doesn't affect 2.23])

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
A landfill user reported that she got the SQL error 'ORDER BY "relevance" is ambiguous' using today's cvs tip. Examining the SQL, I see in the SELECT statement:

(SUM(CASE WHEN
       (LOWER(longdescs_0.thetext) LIKE '%triad%') THEN 1 ELSE 0
       END)/COUNT(CASE WHEN (LOWER(longdescs_0.thetext) LIKE '%triad%') THEN 1
       ELSE 0 END) + CASE WHEN (LOWER(bugs.short_desc) LIKE '%triad%') THEN 1
       ELSE 0 END) AS relevance, 


(SUM(CASE WHEN (LOWER(longdescs_1.thetext)
       LIKE '%triad%') THEN 1 ELSE 0 END)/COUNT(CASE WHEN
       (LOWER(longdescs_1.thetext) LIKE '%triad%') THEN 1 ELSE 0 END) + CASE
       WHEN (LOWER(bugs.short_desc) LIKE '%triad%') THEN 1 ELSE 0 END) A
       S relevance, (SUM(CASE WHEN (LOWER(longdescs_2.thetext) LIKE '%triad%')
       THEN 1 ELSE 0 END)/COUNT(CASE WHEN (LOWER(longdescs_2.thetext) LIKE
       '%triad%') THEN 1 ELSE 0 END) + CASE WHEN (LOWER(bugs.short_desc) LIKE
       '%triad%') THEN 1 ELSE 0 END) AS relevance 

She said she was coming from colchange.cgi. So somehow it's possible to generate two "relevance" fields, which is bad, but not easy to reproduce.
(Reporter)

Comment 1

12 years ago
Okay, looking at the WHERE clause some more, I see a few parts like:

CASE WHEN (LOWER(longdescs_0.thetext) LIKE '%triad%') THEN 1 ELSE 0 END > 0
  OR bugs.short_desc ~* '(^|[^a-z0-9])triad($|[^a-z0-9])')

and:

CASE WHEN (LOWER(longdescs_1.thetext) LIKE '%triad%') THEN 1 ELSE 0 END > 0 
  OR bugs.short_desc ~* '(^|[^a-z0-9])triad($|[^a-z0-9])'

and:

CASE WHEN (LOWER(longdescs_2.thetext) LIKE '%triad%') THEN 1 ELSE 0 END > 0
  OR bugs.short_desc ~* '(^|[^a-z0-9])triad($|[^a-z0-9])'

Notice anything strange about that? It's the same boolean chart three times. Do we actually allow that?
Summary: [PostgreSQL] It's somehow possible to generate two "relevance" columns on buglist.cgi → Boolean charts can somehow generate two "relevance" columns for buglist.cgi

Comment 2

12 years ago
It's trivial to reproduce on PostgreSQL, where we get a crash:

1. Go to query.cgi?format=specific (Find a Specific Bug)
2. Enter a word, such as "bugzilla"
3. Click "Search"
4. Under the bug list, click the "Change Columns" link
5. Change your list of columns if you want, but that's optional (you will crash in both cases)
6. Click the "Change Columns" button
7. Instead of displaying your buglist again, you get:

DBD::Pg::st execute failed: ERROR:  ORDER BY "relevance" is ambiguous
 [for Statement "SELECT bugs.bug_id, bugs.bug_severity, bugs.priority, bugs.bug_status, bugs.resolution, map_products.name, bugs.delta_ts, map_assigned_to.login_name, map_reporter.login_name, bugs.bug_status, bugs.resolution, bugs.short_desc, (SUM(CASE WHEN (LOWER(longdescs_0.thetext) LIKE '%political%') THEN 1 ELSE 0 END)/COUNT(CASE WHEN (LOWER(longdescs_0.thetext) LIKE '%political%') THEN 1 ELSE 0 END) + CASE WHEN (LOWER(bugs.short_desc) LIKE '%political%') THEN 1 ELSE 0 END) AS relevance, (SUM(CASE WHEN (LOWER(longdescs_1.thetext) LIKE '%political%') THEN 1 ELSE 0 END)/COUNT(CASE WHEN (LOWER(longdescs_1.thetext) LIKE '%political%') THEN 1 ELSE 0 END) + CASE WHEN (LOWER(bugs.short_desc) LIKE '%political%') THEN 1 ELSE 0 END) AS relevance FROM bugs  INNER JOIN profiles AS map_assigned_to ON (bugs.assigned_to = map_assigned_to.userid) INNER JOIN profiles AS map_reporter ON (bugs.reporter = map_reporter.userid) INNER JOIN products AS map_products ON (bugs.product_id = map_products.id) INNER JOIN longdescs AS longdescs_0 ON (bugs.bug_id = longdescs_0.bug_id ) INNER JOIN longdescs AS longdescs_1 ON (bugs.bug_id = longdescs_1.bug_id ) LEFT JOIN bug_group_map  ON bug_group_map.bug_id = bugs.bug_id  AND bug_group_map.group_id NOT IN (8,4,16,20,9,14,18,2,17,12,3,10,11,13,21,1,6,15,5,7)  LEFT JOIN cc ON cc.bug_id = bugs.bug_id AND cc.who = 10195 WHERE ((bugs.bug_status IN ('UNCONFIRMED','NEW','ASSIGNED','REOPENED'))) AND (((CASE WHEN (LOWER(longdescs_0.thetext) LIKE '%political%') THEN 1 ELSE 0 END > 0) OR (bugs.short_desc ~* '(^|[^a-z0-9])political($|[^a-z0-9])'))) AND (((CASE WHEN (LOWER(longdescs_1.thetext) LIKE '%political%') THEN 1 ELSE 0 END > 0) OR (bugs.short_desc ~* '(^|[^a-z0-9])political($|[^a-z0-9])'))) AND bugs.creation_ts IS NOT NULL AND ((bug_group_map.group_id IS NULL)    OR (bugs.reporter_accessible = 1 AND bugs.reporter = 10195)     OR (bugs.cclist_accessible = 1 AND cc.who IS NOT NULL)     OR (bugs.assigned_to = 10195) OR (bugs.qa_contact = 10195) ) GROUP BY bugs.bug_id, bugs.short_desc, bugs.short_desc, bugs.bug_severity, bugs.priority, bugs.bug_status, bugs.resolution, map_products.name, bugs.delta_ts, map_assigned_to.login_name, map_reporter.login_name ORDER BY relevance desc LIMIT 200"] at /var/www/html/qa222pg/buglist.cgi line 917


I cannot reproduce on tip, where 'AS relevance' appears only once, nor can I reproduce on 2.22 using MySQL, despite 'AS relevance' appears twice too.

I consider this as a blocker as 1) we officially support PosgreSQL since 2.22, and 2) the "Find a specific bug" search page is the one by default for users with no privs.
Severity: minor → normal
Flags: blocking2.22.1?
Summary: Boolean charts can somehow generate two "relevance" columns for buglist.cgi → [PostgreSQL] Crash when changing the displayed columns after a "specific" search
Whiteboard: [doesn't affect 2.23]
Target Milestone: --- → Bugzilla 2.22
Version: 2.23 → 2.22

Comment 3

12 years ago
joel, could you have a look at this bug? I would really like to see it fixed for 2.22.1 as this is our latest stable release.
(Reporter)

Comment 4

12 years ago
This may be a regression since 2.20, but it's not a regression since 2.22, so I don't think it's appropriate to block the release on it at this point.

I'd certainly be fine with taking a patch for it before we release, though.
Severity: normal → major
Flags: blocking2.22.1? → blocking2.22.1-

Comment 5

12 years ago
We didn't support Pg in 2.20.
(Reporter)

Comment 6

12 years ago
(In reply to comment #5)
> We didn't support Pg in 2.20.

  Yeah, true. I'm just saying that we released 2.22 with this bug in it, and nobody died. We can release 2.22.1 with this bug in it, for now. If you requested blocking 2.22.2 (if we had that flag), I'd grant it. And of course, if somebody fixes this before 2.22.1, then that's fine, too! :-)

Updated

12 years ago
Blocks: 314490

Updated

12 years ago
Flags: blocking2.22.2?
(Reporter)

Comment 7

12 years ago
All right. Now that we have this flag, it's granted, as I said I would do. :-)
Flags: blocking2.22.2? → blocking2.22.2+
Created attachment 245074 [details] [diff] [review]
Patch v1

OK, the issue here is that the Change Columns button is getting a copy of the query string after buglist.cgi and Search.pm have been messing with it, and Search.pm on the 2.22 branch adds the chart items for content by setting the $cgi->param() for them.  So once the Change Columns button gets it, the content stuff is in it twice, once as a content= param, and once as a 'content matches' boolean chart item.   On the tip, Search.pm just pushes them onto the in-memory chart array directly without futzing with $cgi->param().

The 2.22 branch has the in-memory chart array.  So the easy fix is to backport the tip's usage of the array push instead.  Looks like this got into the tip on bug 287170 as just a code cleanup in the process of doing other things.
Assignee: query-and-buglist → justdave
Status: NEW → ASSIGNED
Attachment #245074 - Flags: review?(mkanat)
Assignee: justdave → justdave
Status: ASSIGNED → NEW
And for the record, I did test this, with it applied to qa222pg, and it worked. :) (but it's no longer applied there because I didn't want it to mess up any updates)
(Reporter)

Comment 10

12 years ago
Comment on attachment 245074 [details] [diff] [review]
Patch v1

Yeah, sir, that looks fine to me. It does the same thing, and we know it works on the tip.
Attachment #245074 - Flags: review?(mkanat) → review+
(Reporter)

Updated

12 years ago
Status: NEW → ASSIGNED
Flags: approval2.22?
Flags: approval2.22? → approval2.22+
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.121.2.4; previous revision: 1.121.2.3
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.