launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/faster-tag-searches into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #735977 in Launchpad itself: "*:+bugs timeouts searching for tags"
  https://bugs.launchpad.net/launchpad/+bug/735977

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/faster-tag-searches/+merge/104320

This branch reworks bug tag ALL searches, changing them from a clause of the form EXISTS (foo INTERSECT bar). to (EXISTS foo AND EXISTS bar). This allows postgres to estimate and plan more correctly in all cases I've tried.
-- 
https://code.launchpad.net/~wgrant/launchpad/faster-tag-searches/+merge/104320
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/faster-tag-searches into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2012-05-01 07:28:49 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2012-05-02 04:52:20 +0000
@@ -18,7 +18,6 @@
     Desc,
     Exists,
     In,
-    Intersect,
     Join,
     LeftJoin,
     Not,
@@ -1284,7 +1283,7 @@
     if len(subselects) == 1:
         return Exists(subselects[0])
     else:
-        return Exists(Intersect(*subselects))
+        return And(*(Exists(subselect) for subselect in subselects))
 
 
 def _build_tag_set_query_all(tags, cols):

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2012-04-26 06:38:34 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2012-05-02 04:52:20 +0000
@@ -401,11 +401,12 @@
         # The WHERE clause to test for the absence of *any* of several
         # tags.
         self.assertEqualIgnoringWhitespace(
-            """NOT EXISTS
-                 ((SELECT 1 FROM BugTag
+            """NOT
+                 (EXISTS
+                  (SELECT 1 FROM BugTag
                    WHERE BugTag.bug = Bug.id
                      AND BugTag.tag = 'bob')
-                  INTERSECT
+                  AND EXISTS
                   (SELECT 1 FROM BugTag
                    WHERE BugTag.bug = Bug.id
                      AND BugTag.tag = 'fred'))""",
@@ -416,8 +417,8 @@
         self.assertEqualIgnoringWhitespace(
             """NOT EXISTS
                  (SELECT 1 FROM BugTag
-                   WHERE BugTag.bug = Bug.id
-                     AND BugTag.tag = 'fred')""",
+                  WHERE BugTag.bug = Bug.id
+                    AND BugTag.tag = 'fred')""",
             self.searchClause(any(u'-fred', u'-*')))
 
     def test_multiple_tag_presence_all(self):
@@ -425,13 +426,13 @@
         # tags.
         self.assertEqualIgnoringWhitespace(
             """EXISTS
-                 ((SELECT 1 FROM BugTag
-                   WHERE BugTag.bug = Bug.id
-                     AND BugTag.tag = 'bob')
-                  INTERSECT
-                  (SELECT 1 FROM BugTag
-                   WHERE BugTag.bug = Bug.id
-                     AND BugTag.tag = 'fred'))""",
+               (SELECT 1 FROM BugTag
+                WHERE BugTag.bug = Bug.id
+                  AND BugTag.tag = 'bob')
+               AND EXISTS
+               (SELECT 1 FROM BugTag
+                WHERE BugTag.bug = Bug.id
+                  AND BugTag.tag = 'fred')""",
             self.searchClause(all(u'fred', u'bob')))
         # In an `all` query, a positive wildcard is superfluous in the
         # presence of other positive tags because "bugs with a
@@ -482,11 +483,12 @@
                   (SELECT 1 FROM BugTag
                     WHERE BugTag.bug = Bug.id
                       AND BugTag.tag IN ('eric', 'fred'))
-                OR NOT EXISTS
-                  ((SELECT 1 FROM BugTag
+                OR NOT
+                  (EXISTS
+                    (SELECT 1 FROM BugTag
                     WHERE BugTag.bug = Bug.id
                       AND BugTag.tag = 'bob')
-                   INTERSECT
+                   AND EXISTS
                    (SELECT 1 FROM BugTag
                     WHERE BugTag.bug = Bug.id
                       AND BugTag.tag = 'harry'))""",
@@ -496,11 +498,12 @@
             """EXISTS
                   (SELECT 1 FROM BugTag
                     WHERE BugTag.bug = Bug.id)
-                OR NOT EXISTS
-                  ((SELECT 1 FROM BugTag
+                OR NOT
+                  (EXISTS
+                   (SELECT 1 FROM BugTag
                     WHERE BugTag.bug = Bug.id
                       AND BugTag.tag = 'bob')
-                   INTERSECT
+                   AND EXISTS
                    (SELECT 1 FROM BugTag
                     WHERE BugTag.bug = Bug.id
                       AND BugTag.tag = 'harry'))""",
@@ -557,13 +560,13 @@
             self.searchClause(all(u'fred', u'-bob')))
         self.assertEqualIgnoringWhitespace(
             """EXISTS
-                  ((SELECT 1 FROM BugTag
-                    WHERE BugTag.bug = Bug.id
-                      AND BugTag.tag = 'eric')
-                   INTERSECT
-                   (SELECT 1 FROM BugTag
-                    WHERE BugTag.bug = Bug.id
-                      AND BugTag.tag = 'fred'))
+                 (SELECT 1 FROM BugTag
+                  WHERE BugTag.bug = Bug.id
+                    AND BugTag.tag = 'eric')
+                AND EXISTS
+                 (SELECT 1 FROM BugTag
+                  WHERE BugTag.bug = Bug.id
+                    AND BugTag.tag = 'fred')
                 AND NOT EXISTS
                   (SELECT 1 FROM BugTag
                     WHERE BugTag.bug = Bug.id
@@ -595,16 +598,16 @@
         # The negative wildcard is dominant over other negative tags.
         self.assertEqualIgnoringWhitespace(
             """EXISTS
-                  ((SELECT 1 FROM BugTag
-                    WHERE BugTag.bug = Bug.id
-                      AND BugTag.tag = 'eric')
-                   INTERSECT
-                   (SELECT 1 FROM BugTag
-                    WHERE BugTag.bug = Bug.id
-                      AND BugTag.tag = 'fred'))
-                AND NOT EXISTS
-                  (SELECT 1 FROM BugTag
-                    WHERE BugTag.bug = Bug.id)""",
+                 (SELECT 1 FROM BugTag
+                  WHERE BugTag.bug = Bug.id
+                    AND BugTag.tag = 'eric')
+               AND EXISTS
+                 (SELECT 1 FROM BugTag
+                  WHERE BugTag.bug = Bug.id
+                    AND BugTag.tag = 'fred')
+               AND NOT EXISTS
+                 (SELECT 1 FROM BugTag
+                  WHERE BugTag.bug = Bug.id)""",
             self.searchClause(all(u'fred', u'-bob', u'eric', u'-*')))
         # The positive wildcard is superfluous in the presence of
         # other positive tags, and the negative wildcard is dominant