Merge lp:~lifeless/launchpad/bug-615647 into lp:launchpad

[Robert Collins]

Make determining unmerged revisions a whole lot cheaper.

Use SQL that limits the revisions of source considered - rather than a full set difference, we know that we have a DAG so we can use the information - if X isn't visible, neither is anything with a lower sequence.

[Ian Booth (wallyworld)]

Looks ok to me.

review: Approve (*code)

[Robert Collins (lifeless)]

FTW

review: Approve

=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py	2011-03-04 01:16:03 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py	2011-03-21 04:43:40 +0000
@@ -469,7 +469,7 @@
     def getUnlandedSourceBranchRevisions():
         """Return a sequence of `BranchRevision` objects.
 
-        Returns those revisions that are in the revision history for the
+        Returns up to 10 revisions that are in the revision history for the
         source branch that are not in the revision history of the target
         branch.  These are the revisions that have been committed to the
         source branch since it branched off the target branch.

=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py	2011-03-03 13:12:37 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2011-03-21 04:43:40 +0000
@@ -27,6 +27,7 @@
     LeftJoin,
     Or,
     Select,
+    SQL,
     )
 from storm.info import ClassAlias
 from storm.locals import (
@@ -647,19 +648,19 @@
     def getUnlandedSourceBranchRevisions(self):
         """See `IBranchMergeProposal`."""
         store = Store.of(self)
-        SourceRevision = ClassAlias(BranchRevision)
-        TargetRevision = ClassAlias(BranchRevision)
-        target_join = LeftJoin(
-            TargetRevision, And(
-                TargetRevision.branch_id == self.target_branch.id,
-                TargetRevision.revision_id == SourceRevision.revision_id))
-        origin = [SourceRevision, target_join]
-        result = store.using(*origin).find(
-            SourceRevision,
-            SourceRevision.branch_id == self.source_branch.id,
-            SourceRevision.sequence != None,
-            TargetRevision.branch_id == None)
-        return result.order_by(Desc(SourceRevision.sequence)).config(limit=10)
+        source = SQL("""source AS (SELECT BranchRevision.branch,
+            BranchRevision.revision, Branchrevision.sequence FROM
+            BranchRevision WHERE BranchRevision.branch = %s and
+            BranchRevision.sequence IS NOT NULL ORDER BY BranchRevision.branch
+            DESC, BranchRevision.sequence DESC
+            LIMIT 10)""" % self.source_branch.id)
+        where = SQL("""BranchRevision.revision NOT IN (SELECT revision from
+            BranchRevision AS target where target.branch = %s and
+            BranchRevision.revision = target.revision)""" % self.target_branch.id)
+        using = SQL("""source as BranchRevision""")
+        revisions = store.with_(source).using(using).find(BranchRevision, where)
+        return list(revisions.order_by(
+            Desc(BranchRevision.sequence)).config(limit=10))
 
     def createComment(self, owner, subject, content=None, vote=None,
                       review_type=None, parent=None, _date_created=DEFAULT,