754732 - Consolidate people checkboxes in Search.pm

Status: NEW

(Bugzilla :: Query/Bug List, defect)

Description

[David Marshall]

Attachment: patch for Search.pm

A user once contacted me about a really slow search. Among other problems was one in which they'd filled out some people checkboxes with the same name, type, and checkboxes! This joined in some tables twice and generated SQL that included something like this in the predicate: (a OR b OR c) AND (a OR b). Unfortunate.

Furthermore, if two checkbox groups are filled out with the same name/type with one checkbox set that's a subset of the other checkbox set, the one with the superset can be disregarded, because (a OR b) AND (a OR b OR c) is logically equivalent to (a OR b).

Patch for Search.pm is attached.

If one checkbox group is filled out with just assignee "is" [e-mail] and another is filled out with all the checkboxes "is" [e-mail], here's the SQL that trunk@8224 produces:


SELECT bugs.bug_id AS bug_id, bugs.bug_severity AS bug_severity, bugs.priority AS priority, bugs.bug_status AS bug_status, bugs.resolution AS resolution, map_product.name AS product
  FROM bugs
LEFT JOIN bug_group_map AS security_map ON bugs.bug_id = security_map.bug_id AND NOT ( security_map.group_id IN (1,10,11,14,12,13,9,4,8,5,6,7,3,2) )
LEFT JOIN cc AS security_cc ON bugs.bug_id = security_cc.bug_id AND security_cc.who = 1
INNER JOIN products AS map_product ON bugs.product_id = map_product.id
INNER JOIN priority AS map_priority ON bugs.priority = map_priority.value
INNER JOIN bug_severity AS map_bug_severity ON bugs.bug_severity = map_bug_severity.value
INNER JOIN profiles AS name_assigned_to_1 ON bugs.assigned_to = name_assigned_to_1.userid
INNER JOIN profiles AS name_reporter_2 ON bugs.reporter = name_reporter_2.userid
LEFT JOIN (SELECT bug_id FROM cc AS cc_3 INNER JOIN profiles AS name_cc_3 ON cc_3.who = name_cc_3.userid AND  name_cc_3.login_name IN ('foo@bar.com') ) AS cc_3_3 ON bugs.bug_id = cc_3_3.bug_id
LEFT JOIN (SELECT bug_id,isprivate FROM longdescs AS commenter_4 INNER JOIN profiles AS name_commenter_4 ON commenter_4.who = name_commenter_4.userid AND  name_commenter_4.login_name IN ('foo@bar.com') ) AS commenter_4_4 ON bugs.bug_id = commenter_4_4.bug_id AND commenter_4_4.isprivate = 0
INNER JOIN profiles AS name_assigned_to_5 ON bugs.assigned_to = name_assigned_to_5.userid
 WHERE bugs.creation_ts IS NOT NULL
   AND (security_map.group_id IS NULL
        OR (bugs.reporter_accessible = 1 AND bugs.reporter = 1)
        OR (bugs.cclist_accessible = 1 AND security_cc.who IS NOT NULL)
        OR bugs.assigned_to = 1
)
   AND  bugs.resolution IN ('')  AND ( (  name_assigned_to_1.login_name IN ('foo@bar.com')  OR  name_reporter_2.login_name IN ('foo@bar.com')  OR cc_3_3.bug_id IS NOT NULL OR commenter_4_4.bug_id IS NOT NULL ) AND  name_assigned_to_5.login_name IN ('foo@bar.com')  )
GROUP BY bugs.bug_id
ORDER BY map_priority.sortkey, map_priority.value, map_bug_severity.sortkey, map_bug_severity.value
LIMIT 500

This query, produced by my patch, has the same result and executes more quickly:


SELECT bugs.bug_id AS bug_id, bugs.bug_severity AS bug_severity, bugs.priority AS priority, bugs.bug_status AS bug_status, bugs.resolution AS resolution, map_product.name AS product
  FROM bugs
LEFT JOIN bug_group_map AS security_map ON bugs.bug_id = security_map.bug_id AND NOT ( security_map.group_id IN (1,10,11,14,12,13,9,4,8,5,6,7,3,2) )
LEFT JOIN cc AS security_cc ON bugs.bug_id = security_cc.bug_id AND security_cc.who = 1
INNER JOIN products AS map_product ON bugs.product_id = map_product.id
INNER JOIN priority AS map_priority ON bugs.priority = map_priority.value
INNER JOIN bug_severity AS map_bug_severity ON bugs.bug_severity = map_bug_severity.value
INNER JOIN profiles AS name_assigned_to_1 ON bugs.assigned_to = name_assigned_to_1.userid
 WHERE bugs.creation_ts IS NOT NULL
   AND (security_map.group_id IS NULL
        OR (bugs.reporter_accessible = 1 AND bugs.reporter = 1)
        OR (bugs.cclist_accessible = 1 AND security_cc.who IS NOT NULL)
        OR bugs.assigned_to = 1
)
   AND  bugs.resolution IN ('')  AND  name_assigned_to_1.login_name IN ('foo@bar.com') 
GROUP BY bugs.bug_id
ORDER BY map_priority.sortkey, map_priority.value, map_bug_severity.sortkey, map_bug_severity.value
LIMIT 500

Comment 1

[Frédéric Buclin]

Do you have some perf data (including the number of bugs and user accounts in your DB)? If the win is minor, then it doesn't worth the code complexity. If the win is significant, then it must be considered.

Comment 2

[David Marshall]

The win occurs only if a user is particularly sloppy about filling out the advanced search form, so it's a win that is hopefully realized only rarely. I made this change in our old 2.22 Bugzilla a long time ago and was porting it to our new 3.6 over the weekend.

mysql> select count(*) from bugs\G
*************************** 1. row ***************************
count(*): 5582573

mysql> select count(*) from profiles\G
*************************** 1. row ***************************
count(*): 70536

mysql> select count(*) from longdescs\G
*************************** 1. row ***************************
count(*): 30184635


The top query took 24.86 seconds, the lower query took 0.02 seconds, although it's an extreme example, of course, in which the top query does a LEFT JOIN on longdescs and the bottom does not.

Comment 3

[Max Kanat-Alexander]

It would also be good to know whether xt/search.t still passes with this patch applied. (See its perldoc for info on how to run it.)

Comment 4

[Dave Miller [:justdave]]

Comment on attachment 623542 [details] [diff] [review]
patch for Search.pm

Since this is a UI change, and we're in the middle of redoing all of that, reassigning review to kohei to evaluate how this fits in the current scheme of things.  My apologies for letting this sit so long :-(

Comment 5

[Kohei Yoshino [:kohei]]

Comment on attachment 623542 [details] [diff] [review]
patch for Search.pm

I would hesitate to review Perl bits to be honest.