Merge lp:~al-maisan/launchpad/apapi-fix into lp:launchpad/db-devel

[Muharem Hrnjadovic]

[Muharem Hrnjadovic (al-maisan)]

While testing the archive permission methods newly exposed on the LP API
it became apparent that the database queries pertaining to the
TeamParticipation table were incorrect.

That resulted in loooong query execution times and OOPSs.

The branch at hand corrects the queries in question according to
advice received from Stuart Bishop (thanks stub!).

Please see also: https://bugs.edge.launchpad.net/soyuz/+bug/402108

Tests to run:

    bin/test -vv -t archive -t packageset

[Julian Edwards (julian-edwards)]

Looks good Muharem. Please ask for an RC from Francis when he's around and then land it.

review: Approve

[Francis J. Lacoste (flacoste)]

review: Approve (release-critical)

=== modified file 'lib/lp/soyuz/model/archivepermission.py'
--- lib/lp/soyuz/model/archivepermission.py	2009-07-17 00:26:05 +0000
+++ lib/lp/soyuz/model/archivepermission.py	2009-07-21 08:27:00 +0000
@@ -121,10 +121,8 @@
         clauses = ["""
             ArchivePermission.archive = %s AND
             ArchivePermission.permission = %s AND
-            EXISTS (SELECT TeamParticipation.person
-                    FROM TeamParticipation
-                    WHERE TeamParticipation.person = %s AND
-                          TeamParticipation.team = ArchivePermission.person)
+            ArchivePermission.person = TeamParticipation.team AND
+            TeamParticipation.person = %s
             """ % sqlvalues(archive, permission, person)
             ]
 
@@ -149,7 +147,7 @@
 
         query = " AND ".join(clauses)
         auth = ArchivePermission.select(
-            query, clauseTables=["TeamParticipation"], distinct=True,
+            query, clauseTables=["TeamParticipation"],
             prejoins=prejoins)
 
         return auth
@@ -337,11 +335,11 @@
             SELECT ap.id
             FROM archivepermission ap, teamparticipation tp
             WHERE
-                (ap.person = ? OR (ap.person = tp.team AND tp.person = ?))
+                ap.person = tp.team AND tp.person = ?
                 AND ap.archive = ?
                 AND ap.packageset IS NOT NULL
         '''
-        query = SQL(query, (person.id, person.id, archive.id))
+        query = SQL(query, (person.id, archive.id))
         return store.find(ArchivePermission, In(ArchivePermission.id, query))
 
     def uploadersForPackageset(
@@ -375,10 +373,10 @@
             SELECT ap.id
             FROM archivepermission ap, teamparticipation tp
             WHERE
-                (ap.person = ? OR (ap.person = tp.team AND tp.person = ?))
+                ap.person = tp.team AND tp.person = ?
                 AND ap.packageset = ? AND ap.archive = ?
         '''
-        query = SQL(query, (person.id, person.id, packageset.id, archive.id))
+        query = SQL(query, (person.id, packageset.id, archive.id))
         permissions = list(
             store.find(ArchivePermission, In(ArchivePermission.id, query)))
         if len(permissions) > 0:
@@ -444,14 +442,14 @@
                 archivepermission ap, teamparticipation tp,
                 packagesetsources pss, flatpackagesetinclusion fpsi
             WHERE
-                (ap.person = ? OR (ap.person = tp.team AND tp.person = ?))
+                ap.person = tp.team AND tp.person = ?
                 AND ap.packageset = fpsi.parent
                 AND pss.packageset = fpsi.child
                 AND pss.sourcepackagename = ?
                 AND ap.archive = ?
         '''
         query = SQL(
-            query, (person.id, person.id, sourcepackagename.id, archive.id))
+            query, (person.id, sourcepackagename.id, archive.id))
         return store.find(ArchivePermission, In(ArchivePermission.id, query))
 
     def packagesetsForSource(
@@ -491,9 +489,9 @@
             # Query parameters for the first WHERE clause.
             (archive.id, sourcepackagename.id) +
             # Query parameters for the second WHERE clause.
-            (sourcepackagename.id,) + (person.id,)*2 + archive_params + 
+            (sourcepackagename.id,) + (person.id,) + archive_params + 
             # Query parameters for the third WHERE clause.
-            (sourcepackagename.id,) + (person.id,)*2 + archive_params)
+            (sourcepackagename.id,) + (person.id,) + archive_params)
 
         query = '''
         SELECT CASE
@@ -511,7 +509,7 @@
               teamparticipation tp
             WHERE
               pss.sourcepackagename = %s
-              AND (ap.person = %s OR (ap.person = tp.team AND tp.person = %s))
+              AND ap.person = tp.team AND tp.person = %s
               AND pss.packageset = ap.packageset AND ap.explicit = TRUE
               AND ap.permission = %s AND ap.archive = %s)
           ELSE (
@@ -521,7 +519,7 @@
               teamparticipation tp, flatpackagesetinclusion fpsi
             WHERE
               pss.sourcepackagename = %s
-              AND (ap.person = %s OR (ap.person = tp.team AND tp.person = %s))
+              AND ap.person = tp.team AND tp.person = %s
               AND pss.packageset = fpsi.child AND fpsi.parent = ap.packageset
               AND ap.permission = %s AND ap.archive = %s)
         END AS number_of_permitted_package_sets;