Merge lp:~wgrant/launchpad/faster-binary-search into lp:launchpad

[William Grant]

Fix some BinaryPackagePublishingHistory queries to filter by BPPH.binarypackagename rather than joining through BinaryPackageRelease.

This branch fixes most of the remaining BinaryPackagePublishingHistory queries that filter on BinaryPackageRelease.binarypackagename to use the denormalised column instead. This gets the bad cases of methods like Archive.getAllPublishedBinaries down from ~5000ms to ~10ms.

DistroSeries.getBinaryPackagePublishing and getSourcePackagePublishingHistory also had the issue, but none of their callsites use the name filtering any more. I stripped them right down to just serve NMAF's needs, but they should eventually be replaced with methods on Archive.

[Steve Kowalik (stevenk)]

review: Approve (code)

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2012-08-17 11:15:35 +0000
+++ lib/lp/archivepublisher/publishing.py	2012-09-07 05:49:18 +0000
@@ -414,8 +414,7 @@
             source_index_root, self._config.temproot, 'Sources')
 
         for spp in distroseries.getSourcePackagePublishing(
-            PackagePublishingStatus.PUBLISHED, pocket=pocket,
-            component=component, archive=self.archive):
+                pocket, component, self.archive):
             stanza = spp.getIndexStanza().encode('utf8') + '\n\n'
             source_index.write(stanza)
 
@@ -441,8 +440,7 @@
                 di_index_root, self._config.temproot, 'Packages')
 
             for bpp in distroseries.getBinaryPackagePublishing(
-                archtag=arch.architecturetag, pocket=pocket,
-                component=component, archive=self.archive):
+                    arch.architecturetag, pocket, component, self.archive):
                 stanza = bpp.getIndexStanza().encode('utf-8') + '\n\n'
                 if (bpp.binarypackagerelease.binpackageformat in
                     (BinaryPackageFormat.DEB, BinaryPackageFormat.DDEB)):

=== modified file 'lib/lp/archivepublisher/tests/test_generate_extra_overrides.py'
--- lib/lp/archivepublisher/tests/test_generate_extra_overrides.py	2012-08-17 11:15:35 +0000
+++ lib/lp/archivepublisher/tests/test_generate_extra_overrides.py	2012-09-07 05:49:18 +0000
@@ -172,8 +172,8 @@
             source_index = RepositoryIndexFile(
                 source_index_root, script.config.temproot, "Sources")
             for spp in distroseries.getSourcePackagePublishing(
-                PackagePublishingStatus.PUBLISHED,
-                PackagePublishingPocket.RELEASE, component=component):
+                    PackagePublishingPocket.RELEASE, component,
+                    distroseries.main_archive):
                 stanza = spp.getIndexStanza().encode("utf-8") + "\n\n"
                 source_index.write(stanza)
             source_index.close()
@@ -184,9 +184,8 @@
                 package_index = RepositoryIndexFile(
                     package_index_root, script.config.temproot, "Packages")
                 for bpp in distroseries.getBinaryPackagePublishing(
-                    archtag=arch.architecturetag,
-                    pocket=PackagePublishingPocket.RELEASE,
-                    component=component):
+                        arch.architecturetag, PackagePublishingPocket.RELEASE,
+                        component, distroseries.main_archive):
                     stanza = bpp.getIndexStanza().encode("utf-8") + "\n\n"
                     package_index.write(stanza)
                 package_index.close()

=== modified file 'lib/lp/registry/doc/distroseries.txt'
--- lib/lp/registry/doc/distroseries.txt	2012-06-19 03:26:57 +0000
+++ lib/lp/registry/doc/distroseries.txt	2012-09-07 05:49:18 +0000
@@ -637,85 +637,47 @@
 instance. it can also be used to generate the archive packages list in
 the future.
 
-    >>> ubuntu = getUtility(IDistributionSet)['ubuntu']
-    >>> hoary = ubuntu['hoary']
-
     >>> from lp.soyuz.enums import PackagePublishingStatus
-    >>> hoary_pub_sources = hoary.getSourcePackagePublishing(
-    ...     PackagePublishingStatus.PUBLISHED,
-    ...     PackagePublishingPocket.RELEASE)
-
-    >>> hoary_pub_sources.count()
-    6
-
-    >>> for pub in hoary_pub_sources:
-    ...     print pub.displayname
-    alsa-utils 1.0.9a-4ubuntu1 in hoary
-    cnews cr.g7-37 in hoary
-    evolution 1.0 in hoary
-    libstdc++ b8p in hoary
-    linux-source-2.6.15 2.6.15.3 in hoary
-    pmount 0.1-2 in hoary
-
-    >>> hoary_pub_source = hoary_pub_sources[0]
-
-    >>> hoary_pub_source.sourcepackagerelease.name
-    u'alsa-utils'
-
-    >>> hoary_pub_source.sourcepackagerelease.version
-    u'1.0.9a-4ubuntu1'
-
-    >>> hoary_pub_source.component.name
-    u'main'
-
-    >>> hoary_pub_source.section.name
-    u'base'
-
-    >>> hoary.getSourcePackagePublishing(
-    ...     PackagePublishingStatus.PUBLISHED,
-    ...     PackagePublishingPocket.UPDATES).count()
-    0
-
-This method also allow us to restrict the results to a given
-component:
-
+    >>> ubuntu = getUtility(IDistributionSet)['ubuntu']
+    >>> hoary = ubuntu['hoary']
     >>> component_main = getUtility(IComponentSet)['main']
-    >>> hoary.getSourcePackagePublishing(
-    ...     PackagePublishingStatus.PUBLISHED,
-    ...     PackagePublishingPocket.RELEASE,
-    ...     component=component_main).count()
-    5
-
     >>> component_multiverse = getUtility(IComponentSet)['multiverse']
-    >>> hoary.getSourcePackagePublishing(
-    ...     PackagePublishingStatus.PUBLISHED,
-    ...     PackagePublishingPocket.RELEASE,
-    ...     component=component_multiverse).count()
-    0
-
-By default the IDistribution 'main_archive' is considered, but It also
-allows the callsite to specify one and then the result will be
-restricted to it.
-
     >>> debian_archive = getUtility(IDistributionSet)['debian'].main_archive
-    >>> print debian_archive.purpose.title
-    Primary Archive
 
-    >>> hoary.getSourcePackagePublishing(
-    ...     PackagePublishingStatus.PUBLISHED,
-    ...     PackagePublishingPocket.RELEASE,
-    ...     component=component_main, archive=debian_archive).count()
+    >>> spphs = hoary.getSourcePackagePublishing(
+    ...     PackagePublishingPocket.RELEASE, component_main,
+    ...     warty.main_archive)
+    >>> spphs.count()
+    5 
+    >>> for name in sorted(set(
+    ...         pkgpub.sourcepackagerelease.sourcepackagename.name
+    ...         for pkgpub in spphs)):
+    ...     print name
+    alsa-utils
+    evolution
+    libstdc++
+    linux-source-2.6.15
+    pmount
+    >>> hoary.getSourcePackagePublishing(
+    ...     PackagePublishingPocket.RELEASE, component_multiverse,
+    ...     hoary.main_archive).count()
+    0
+    >>> hoary.getSourcePackagePublishing(
+    ...     PackagePublishingPocket.BACKPORTS, component_main,
+    ...     hoary.main_archive).count()
+    0
+    >>> hoary.getSourcePackagePublishing(
+    ...     PackagePublishingPocket.RELEASE, component_main,
+    ...     debian_archive).count()
     0
 
 ISPP.getPublishedBinaries returns all the binaries generated by the
 publication in question:
 
     >>> warty = ubuntu['warty']
-    >>> warty_pub_sources = warty.getSourcePackagePublishing(
-    ...     PackagePublishingStatus.PUBLISHED,
-    ...     PackagePublishingPocket.RELEASE)
-
-    >>> warty_pub_source = warty_pub_sources[4]
+    >>> warty_pub_source = warty.main_archive.getPublishedSources(
+    ...     distroseries=warty, name=u'mozilla-firefox',
+    ...     status=PackagePublishingStatus.PUBLISHED).one()
     >>> warty_pub_source.sourcepackagerelease.name
     u'mozilla-firefox'
     >>> warty_pub_source.sourcepackagerelease.version
@@ -749,66 +711,30 @@
 BinaryPackagePublishingHistory objects for the DistroSeries:
 
     >>> warty = ubuntu['warty']
-    >>> bpphs = warty.getBinaryPackagePublishing()
+    >>> bpphs = warty.getBinaryPackagePublishing(
+    ...     "i386", PackagePublishingPocket.RELEASE, component_main,
+    ...     warty.main_archive)
     >>> bpphs.count()
-    10
+    8
     >>> 'mozilla-firefox' in set(
     ...     pkgpub.binarypackagerelease.binarypackagename.name
     ...     for pkgpub in bpphs)
     True
-
-It also allows us to pass wanted strings like: name, version, archtag and
-sourcename.
-
-    >>> warty.getBinaryPackagePublishing(
-    ...     name="nosuchpackage").count()
-    0
-    >>> warty.getBinaryPackagePublishing(
-    ...     version="nosuchversion").count()
-    0
-    >>> warty.getBinaryPackagePublishing(
-    ...     archtag="nosucharch").count()
-    0
-    >>> warty.getBinaryPackagePublishing(
-    ...     sourcename="nosuchsource").count()
-    0
-
-We can restrict the results by component:
-
-    >>> warty.getBinaryPackagePublishing(
-    ...     component=component_main).count()
-    10
-    >>> warty.getBinaryPackagePublishing(
-    ...     component=component_multiverse).count()
-    0
-
-By pocket:
-
-    >>> warty.getBinaryPackagePublishing(
-    ...     pocket=PackagePublishingPocket.RELEASE).count()
-    10
-    >>> warty.getBinaryPackagePublishing(
-    ...     pocket=PackagePublishingPocket.BACKPORTS).count()
-    0
-
-Or any combination of them:
-
-    >>> warty.getBinaryPackagePublishing(
-    ...     sourcename="alsa", pocket=PackagePublishingPocket.RELEASE,
-    ...     component=component_main).count()
-    0
-
-    >>> warty.getBinaryPackagePublishing(
-    ...     name="mozilla-firefox", archtag='i386',
-    ...     component=component_main).count()
-    2
-
-As getSourcePackagePublishing, getBinaryPublishing accepts 'archive'
-parameter for result restriction:
-
-    >>> warty.getBinaryPackagePublishing(
-    ...     name="mozilla-firefox", archtag='i386',
-    ...     component=component_main, archive=debian_archive).count()
+    >>> warty.getBinaryPackagePublishing(
+    ...     "nope", PackagePublishingPocket.RELEASE, component_main,
+    ...     warty.main_archive).count()
+    0
+    >>> warty.getBinaryPackagePublishing(
+    ...     "i386", PackagePublishingPocket.RELEASE, component_multiverse,
+    ...     warty.main_archive).count()
+    0
+    >>> warty.getBinaryPackagePublishing(
+    ...     "i386", PackagePublishingPocket.BACKPORTS, component_main,
+    ...     warty.main_archive).count()
+    0
+    >>> warty.getBinaryPackagePublishing(
+    ...     "i386", PackagePublishingPocket.RELEASE, component_main,
+    ...     debian_archive).count()
     0
 
 getAllPublishedSources will return all publications with status PUBLISHED

=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py	2012-07-03 08:04:35 +0000
+++ lib/lp/registry/interfaces/distroseries.py	2012-09-07 05:49:18 +0000
@@ -733,23 +733,18 @@
     def addSection(section):
         """SQLObject provided method to fill a related join key section."""
 
-    def getBinaryPackagePublishing(
-        name=None, version=None, archtag=None, sourcename=None, orderBy=None,
-        pocket=None, component=None, archive=None):
+    def getBinaryPackagePublishing(archtag, pocket, component, archive):
         """Get BinaryPackagePublishings in a DistroSeries.
 
-        Can optionally restrict the results by name, version,
-        architecturetag, pocket and/or component.
+        Can optionally restrict the results by architecturetag, pocket and/or
+        component.
 
-        If sourcename is passed, only packages that are built from
-        source packages by that name will be returned.
         If archive is passed, restricted the results to the given archive,
         if it is suppressed the results will be restricted to the
         distribution 'main_archive'.
         """
 
-    def getSourcePackagePublishing(status, pocket, component=None,
-                                   archive=None):
+    def getSourcePackagePublishing(pocket, component, archive):
         """Return a selectResult of ISourcePackagePublishingHistory.
 
         According status and pocket.

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2012-08-08 05:36:44 +0000
+++ lib/lp/registry/model/distroseries.py	2012-09-07 05:49:18 +0000
@@ -1141,91 +1141,30 @@
         return SourcePackagePublishingHistory.select(
             query, clauseTables=['Archive'], orderBy="id")
 
-    def getSourcePackagePublishing(self, status, pocket, component=None,
-                                   archive=None):
-        """See `IDistroSeries`."""
-        archives = self.distribution.getArchiveIDList(archive)
-
-        clause = """
-            SourcePackagePublishingHistory.sourcepackagename=
-                SourcePackageName.id AND
-            SourcePackagePublishingHistory.distroseries=%s AND
-            SourcePackagePublishingHistory.archive IN %s AND
-            SourcePackagePublishingHistory.status=%s AND
-            SourcePackagePublishingHistory.pocket=%s
-            """ % sqlvalues(self, archives, status, pocket)
-
-        if component:
-            clause += (
-                " AND SourcePackagePublishingHistory.component=%s"
-                % sqlvalues(component))
-
-        orderBy = ['SourcePackageName.name']
-        clauseTables = ['SourcePackageName']
-
-        return SourcePackagePublishingHistory.select(
-            clause, orderBy=orderBy, clauseTables=clauseTables)
-
-    def getBinaryPackagePublishing(
-        self, name=None, version=None, archtag=None, sourcename=None,
-        orderBy=None, pocket=None, component=None, archive=None):
-        """See `IDistroSeries`."""
-        archives = self.distribution.getArchiveIDList(archive)
-
-        query = ["""
-        BinaryPackagePublishingHistory.binarypackagerelease =
-            BinaryPackageRelease.id AND
-        BinaryPackagePublishingHistory.distroarchseries =
-            DistroArchSeries.id AND
-        BinaryPackageRelease.binarypackagename =
-            BinaryPackageName.id AND
-        BinaryPackageRelease.build =
-            BinaryPackageBuild.id AND
-        BinaryPackageBuild.source_package_release =
-            SourcePackageRelease.id AND
-        SourcePackageRelease.sourcepackagename =
-            SourcePackageName.id AND
-        DistroArchSeries.distroseries = %s AND
-        BinaryPackagePublishingHistory.archive IN %s AND
-        BinaryPackagePublishingHistory.status = %s
-        """ % sqlvalues(self, archives, PackagePublishingStatus.PUBLISHED)]
-
-        if name:
-            query.append('BinaryPackageName.name = %s' % sqlvalues(name))
-
-        if version:
-            query.append('BinaryPackageRelease.version = %s'
-                      % sqlvalues(version))
-
-        if archtag:
-            query.append('DistroArchSeries.architecturetag = %s'
-                      % sqlvalues(archtag))
-
-        if sourcename:
-            query.append(
-                'SourcePackageName.name = %s' % sqlvalues(sourcename))
-
-        if pocket:
-            query.append(
-                'BinaryPackagePublishingHistory.pocket = %s'
-                % sqlvalues(pocket))
-
-        if component:
-            query.append(
-                'BinaryPackagePublishingHistory.component = %s'
-                % sqlvalues(component))
-
-        query = " AND ".join(query)
-
-        clauseTables = ['BinaryPackagePublishingHistory', 'DistroArchSeries',
-                        'BinaryPackageRelease', 'BinaryPackageName',
-                        'BinaryPackageBuild', 'SourcePackageRelease',
-                        'SourcePackageName']
-
-        result = BinaryPackagePublishingHistory.select(
-            query, distinct=False, clauseTables=clauseTables, orderBy=orderBy)
-
-        return result
+    def getSourcePackagePublishing(self, pocket, component, archive):
+        """See `IDistroSeries`."""
+        return Store.of(self).find(
+            SourcePackagePublishingHistory,
+            SourcePackagePublishingHistory.archive == archive,
+            SourcePackagePublishingHistory.distroseries == self,
+            SourcePackagePublishingHistory.pocket == pocket,
+            SourcePackagePublishingHistory.component == component,
+            SourcePackagePublishingHistory.status ==
+                PackagePublishingStatus.PUBLISHED)
+
+    def getBinaryPackagePublishing(self, archtag, pocket, component, archive):
+        """See `IDistroSeries`."""
+        return Store.of(self).find(
+            BinaryPackagePublishingHistory,
+            DistroArchSeries.distroseries == self,
+            DistroArchSeries.architecturetag == archtag,
+            BinaryPackagePublishingHistory.archive == archive,
+            BinaryPackagePublishingHistory.distroarchseries ==
+                DistroArchSeries.id,
+            BinaryPackagePublishingHistory.pocket == pocket,
+            BinaryPackagePublishingHistory.component == component,
+            BinaryPackagePublishingHistory.status ==
+                PackagePublishingStatus.PUBLISHED)
 
     def getBuildRecords(self, build_state=None, name=None, pocket=None,
                         arch_tag=None, user=None, binary_only=True):

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2012-08-20 14:31:11 +0000
+++ lib/lp/soyuz/model/archive.py	2012-09-07 05:49:18 +0000
@@ -722,7 +722,7 @@
             BinaryPackagePublishingHistory.archive = %s AND
             BinaryPackagePublishingHistory.binarypackagerelease =
                 BinaryPackageRelease.id AND
-            BinaryPackageRelease.binarypackagename =
+            BinaryPackagePublishingHistory.binarypackagename =
                 BinaryPackageName.id
         """ % sqlvalues(self)]
         clauseTables = ['BinaryPackageRelease', 'BinaryPackageName']
@@ -988,7 +988,8 @@
         return store.find(
             BinaryPackagePublishingHistory,
             BinaryPackageName.name == dep_name,
-            BinaryPackageRelease.binarypackagename == BinaryPackageName.id,
+            BinaryPackagePublishingHistory.binarypackagename ==
+                BinaryPackageName.id,
             BinaryPackagePublishingHistory.binarypackagerelease ==
                 BinaryPackageRelease.id,
             BinaryPackagePublishingHistory.distroarchseries ==