Merge lp:~edwin-grubbs/launchpad/bug-535430-needspackaging-timeout-part1 into lp:launchpad/db-devel

[Edwin Grubbs]

This branch adds the following columns the DistributionSourcePackage table:
    total_bug_heat INTEGER
    bug_count INTEGER
    po_message_count INTEGER
    section INTEGER REFERENCES Section(id)

This table will be used to cache data so the $distroseries/+needs-packaging
page won't timeout. A DistributionSourcePackage row is only needed for
packages with a SourcePackagePublishingHistory record.

The section will be updated by the trigger, since it only needs to be
changed when a new SourcePackagePublishingHistory record is added. The
other three columns will be populated by a cronjob.

The total_bug_heat differs from the existing max_bug_heat column in that
the max_bug_heat column is the highest heat value for a single bug related
to the package, and the total_bug_heat is the sum of all the bugs related
to the package.

[Edwin Grubbs (edwin-grubbs)]

After talking with Julian, I have removed the trigger. Updating the DistributionSourcePackage table when a new SourcePackagePublishingHistory record is added will instead be handled in lp.soyuz.model.publishing.PublishingSet.newSourcePublication().

[Björn Tillenius (bjornt)]

Ok, removing the trigger makes it easier to keep things sane. How do you intend to keep the other values up-to-date? Maybe it makes sense to rename them, so that it's clear that they are caches only, and might not be correct? Unless you find a way of making sure they are up to date, of course. Also, for the bug values, will it include private bugs as well?

The comments should be added to comments.sql.

review: Needs Information

[Edwin Grubbs (edwin-grubbs)]

> Ok, removing the trigger makes it easier to keep things sane. How do you
> intend to keep the other values up-to-date?

The other values will be kept up-to-date with a cronjob.

> Maybe it makes sense to rename
> them, so that it's clear that they are caches only, and might not be correct?
> Unless you find a way of making sure they are up to date, of course.

Although I was originally thinking of updating the values with a cronjob, the max_bug_heat is currently updated with a hook in addTask(), so it would be easy to add that. I assume that I could do something similar for po_message_count.

> Also, for
> the bug values, will it include private bugs as well?

Yes.

> The comments should be added to comments.sql.

The comments have been moved.

[Björn Tillenius (bjornt)]

< BjornT> EdwinGrubbs: well, i guess the db patch is fine. you should.
          have a talk with deryck, as to whether include private bugs in.
          the count, since that might not be desired. also talk to him.
          about how to keep the bug heat values up-to-date
< BjornT> EdwinGrubbs: doing it in addTask() only won't be enough

review: Approve (db)

[Stuart Bishop (stub)]

I assume NULL values mean 'has not been calculated yet'. This should be documented in comments.sql.

If we need to retrieve or order rows by bug_count or po_message_count we will need an index on those columns.

Otherwise all looks fine. patch-2207-56-0.sql

review: Approve (db)

=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2010-06-03 22:21:24 +0000
+++ database/schema/comments.sql	2010-06-04 17:54:29 +0000
@@ -472,7 +472,11 @@
 
 COMMENT ON TABLE DistributionSourcePackage IS 'Representing a sourcepackage in a distribution across all distribution series.';
 COMMENT ON COLUMN DistributionSourcePackage.bug_reporting_guidelines IS 'Guidelines to the end user for reporting bugs on a particular a source package in a distribution.';
-COMMENT ON COLUMN DistributionSourcePackage.max_bug_heat IS 'The highest heat value across bugs for this source package.';
+COMMENT ON COLUMN DistributionSourcePackage.max_bug_heat IS 'The highest heat value across bugs for this source package. NULL means it has not yet been calculated.';
+COMMENT ON COLUMN DistributionSourcePackage.total_bug_heat IS 'Sum of bug heat matching the package distribution and sourcepackagename. NULL means it has not yet been calculated.';
+COMMENT ON COLUMN DistributionSourcePackage.bug_count IS 'Number of bugs matching the package distribution and sourcepackagename. NULL means it has not yet been calculated.';
+COMMENT ON COLUMN DistributionSourcePackage.po_message_count IS 'Number of translations matching the package distribution and sourcepackagename. NULL means it has not yet been calculated.';
+COMMENT ON COLUMN DistributionSourcePackage.section IS 'Cached section matching the latest SourcePackagePublishingHistory record by distribution and sourcepackagename whose archive purpose is PRIMARY and whose distroseries releasestatus is CURRENT.';
 COMMENT ON COLUMN DistributionSourcePackage.bug_reported_acknowledgement IS 'A message of acknowledgement to display to a bug reporter after they\'ve reported a new bug.';
 
 -- DistributionSourcePackageCache

=== added file 'database/schema/patch-2207-56-0.sql'
--- database/schema/patch-2207-56-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-56-0.sql	2010-06-04 17:54:29 +0000
@@ -0,0 +1,24 @@
+-- Copyright 2009 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 56, 0);
+
+/*
+Existing Schema:
+
+CREATE TABLE distributionsourcepackage (
+    id integer NOT NULL,
+    distribution integer NOT NULL,
+    sourcepackagename integer NOT NULL,
+    bug_reporting_guidelines text,
+    max_bug_heat integer
+);
+*/
+
+ALTER TABLE DistributionSourcePackage ADD COLUMN total_bug_heat INTEGER;
+ALTER TABLE DistributionSourcePackage ADD COLUMN bug_count INTEGER;
+ALTER TABLE DistributionSourcePackage ADD COLUMN po_message_count INTEGER;
+ALTER TABLE DistributionSourcePackage
+    ADD COLUMN section INTEGER NOT NULL REFERENCES section(id);