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

Proposed by Muharem Hrnjadovic
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~al-maisan/launchpad/apapi-fix
Merge into: lp:launchpad/db-devel
Diff against target: None lines
To merge this branch: bzr merge lp:~al-maisan/launchpad/apapi-fix
Reviewer Review Type Date Requested Status
Francis J. Lacoste (community) release-critical Approve
Julian Edwards (community) Approve
Review via email: mp+9089@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

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

Revision history for this message
Julian Edwards (julian-edwards) wrote :

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

review: Approve
Revision history for this message
Francis J. Lacoste (flacoste) :
review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
121 clauses = ["""121 clauses = ["""
122 ArchivePermission.archive = %s AND122 ArchivePermission.archive = %s AND
123 ArchivePermission.permission = %s AND123 ArchivePermission.permission = %s AND
124 EXISTS (SELECT TeamParticipation.person124 ArchivePermission.person = TeamParticipation.team AND
125 FROM TeamParticipation125 TeamParticipation.person = %s
126 WHERE TeamParticipation.person = %s AND
127 TeamParticipation.team = ArchivePermission.person)
128 """ % sqlvalues(archive, permission, person)126 """ % sqlvalues(archive, permission, person)
129 ]127 ]
130128
@@ -149,7 +147,7 @@
149147
150 query = " AND ".join(clauses)148 query = " AND ".join(clauses)
151 auth = ArchivePermission.select(149 auth = ArchivePermission.select(
152 query, clauseTables=["TeamParticipation"], distinct=True,150 query, clauseTables=["TeamParticipation"],
153 prejoins=prejoins)151 prejoins=prejoins)
154152
155 return auth153 return auth
@@ -337,11 +335,11 @@
337 SELECT ap.id335 SELECT ap.id
338 FROM archivepermission ap, teamparticipation tp336 FROM archivepermission ap, teamparticipation tp
339 WHERE337 WHERE
340 (ap.person = ? OR (ap.person = tp.team AND tp.person = ?))338 ap.person = tp.team AND tp.person = ?
341 AND ap.archive = ?339 AND ap.archive = ?
342 AND ap.packageset IS NOT NULL340 AND ap.packageset IS NOT NULL
343 '''341 '''
344 query = SQL(query, (person.id, person.id, archive.id))342 query = SQL(query, (person.id, archive.id))
345 return store.find(ArchivePermission, In(ArchivePermission.id, query))343 return store.find(ArchivePermission, In(ArchivePermission.id, query))
346344
347 def uploadersForPackageset(345 def uploadersForPackageset(
@@ -375,10 +373,10 @@
375 SELECT ap.id373 SELECT ap.id
376 FROM archivepermission ap, teamparticipation tp374 FROM archivepermission ap, teamparticipation tp
377 WHERE375 WHERE
378 (ap.person = ? OR (ap.person = tp.team AND tp.person = ?))376 ap.person = tp.team AND tp.person = ?
379 AND ap.packageset = ? AND ap.archive = ?377 AND ap.packageset = ? AND ap.archive = ?
380 '''378 '''
381 query = SQL(query, (person.id, person.id, packageset.id, archive.id))379 query = SQL(query, (person.id, packageset.id, archive.id))
382 permissions = list(380 permissions = list(
383 store.find(ArchivePermission, In(ArchivePermission.id, query)))381 store.find(ArchivePermission, In(ArchivePermission.id, query)))
384 if len(permissions) > 0:382 if len(permissions) > 0:
@@ -444,14 +442,14 @@
444 archivepermission ap, teamparticipation tp,442 archivepermission ap, teamparticipation tp,
445 packagesetsources pss, flatpackagesetinclusion fpsi443 packagesetsources pss, flatpackagesetinclusion fpsi
446 WHERE444 WHERE
447 (ap.person = ? OR (ap.person = tp.team AND tp.person = ?))445 ap.person = tp.team AND tp.person = ?
448 AND ap.packageset = fpsi.parent446 AND ap.packageset = fpsi.parent
449 AND pss.packageset = fpsi.child447 AND pss.packageset = fpsi.child
450 AND pss.sourcepackagename = ?448 AND pss.sourcepackagename = ?
451 AND ap.archive = ?449 AND ap.archive = ?
452 '''450 '''
453 query = SQL(451 query = SQL(
454 query, (person.id, person.id, sourcepackagename.id, archive.id))452 query, (person.id, sourcepackagename.id, archive.id))
455 return store.find(ArchivePermission, In(ArchivePermission.id, query))453 return store.find(ArchivePermission, In(ArchivePermission.id, query))
456454
457 def packagesetsForSource(455 def packagesetsForSource(
@@ -491,9 +489,9 @@
491 # Query parameters for the first WHERE clause.489 # Query parameters for the first WHERE clause.
492 (archive.id, sourcepackagename.id) +490 (archive.id, sourcepackagename.id) +
493 # Query parameters for the second WHERE clause.491 # Query parameters for the second WHERE clause.
494 (sourcepackagename.id,) + (person.id,)*2 + archive_params + 492 (sourcepackagename.id,) + (person.id,) + archive_params +
495 # Query parameters for the third WHERE clause.493 # Query parameters for the third WHERE clause.
496 (sourcepackagename.id,) + (person.id,)*2 + archive_params)494 (sourcepackagename.id,) + (person.id,) + archive_params)
497495
498 query = '''496 query = '''
499 SELECT CASE497 SELECT CASE
@@ -511,7 +509,7 @@
511 teamparticipation tp509 teamparticipation tp
512 WHERE510 WHERE
513 pss.sourcepackagename = %s511 pss.sourcepackagename = %s
514 AND (ap.person = %s OR (ap.person = tp.team AND tp.person = %s))512 AND ap.person = tp.team AND tp.person = %s
515 AND pss.packageset = ap.packageset AND ap.explicit = TRUE513 AND pss.packageset = ap.packageset AND ap.explicit = TRUE
516 AND ap.permission = %s AND ap.archive = %s)514 AND ap.permission = %s AND ap.archive = %s)
517 ELSE (515 ELSE (
@@ -521,7 +519,7 @@
521 teamparticipation tp, flatpackagesetinclusion fpsi519 teamparticipation tp, flatpackagesetinclusion fpsi
522 WHERE520 WHERE
523 pss.sourcepackagename = %s521 pss.sourcepackagename = %s
524 AND (ap.person = %s OR (ap.person = tp.team AND tp.person = %s))522 AND ap.person = tp.team AND tp.person = %s
525 AND pss.packageset = fpsi.child AND fpsi.parent = ap.packageset523 AND pss.packageset = fpsi.child AND fpsi.parent = ap.packageset
526 AND ap.permission = %s AND ap.archive = %s)524 AND ap.permission = %s AND ap.archive = %s)
527 END AS number_of_permitted_package_sets;525 END AS number_of_permitted_package_sets;

Subscribers

People subscribed via source and target branches

to status/vote changes: