launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05789
[Merge] lp:~jtv/launchpad/bug-849683-cloner into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-849683-cloner into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #849683 in Launchpad itself: "{B,S}PPH Populator are temporary"
https://bugs.launchpad.net/launchpad/+bug/849683
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-849683-cloner/+merge/84222
= Summary =
The new, denormalized column SourcePackagePublishingHistory.sourcepackagename was left uninitialized in one place: copying through a temporary table in the package cloner.
== Proposed fix ==
Add a column to the temporary table to represent the missing data. There's a lot of code that manipulates it, but the NOT NULL constraint should make sure that it's always initialized.
== Pre-implementation notes ==
Chatted briefly with StevenK. He had no time to fix this, so I did. Once this branch has rolled out we can slap a NOT NULL constraint on the column and retire the garbo job that populates it.
== Implementation details ==
You'll note that I reformatted the SQL a bit. This stuff is hard enough without obfuscated SQL. I did not, however, look into replacing the existing sourcepackagename column in the temp table (which holds the source package's name, as text) with use of the new column. Knuth's Law and all that.
== Tests ==
{{{
./bin/test -vvc lp.soyuz.tests.test_packagecloner
./bin/test -vvc lp.soyuz.tests.test_copyarchivejob
./bin/test -vvc p.soyuz.scripts.tests.test_populatearchive
}}}
== Demo and Q/A ==
Run the package cloner to ensure that referential integrity is maintained. Good to know that I didn't mess up a column ordering somewhere or something.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/model/packagecloner.py
--
https://code.launchpad.net/~jtv/launchpad/bug-849683-cloner/+merge/84222
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-849683-cloner into lp:launchpad.
=== modified file 'lib/lp/soyuz/model/packagecloner.py'
--- lib/lp/soyuz/model/packagecloner.py 2011-11-29 05:15:07 +0000
+++ lib/lp/soyuz/model/packagecloner.py 2011-12-02 07:34:52 +0000
@@ -235,13 +235,15 @@
store.execute("""
INSERT INTO SourcePackagePublishingHistory (
sourcepackagerelease, distroseries, status, component,
- section, archive, datecreated, datepublished, pocket)
+ section, archive, datecreated, datepublished, pocket,
+ sourcepackagename)
SELECT
mcd.s_sourcepackagerelease AS sourcepackagerelease,
%s AS distroseries, mcd.s_status AS status,
mcd.s_component AS component, mcd.s_section AS section,
%s AS archive, %s AS datecreated, %s AS datepublished,
- %s AS pocket
+ %s AS pocket,
+ sourcepackagename_id
FROM tmp_merge_copy_data mcd
WHERE mcd.obsoleted = True OR mcd.missing = True
""" % sqlvalues(
@@ -294,8 +296,9 @@
s_component = secsrc.component,
s_section = secsrc.section
FROM
- sourcepackagepublishinghistory secsrc,
- sourcepackagerelease spr, sourcepackagename spn
+ SourcePackagePublishingHistory secsrc,
+ SourcePackageRelease spr,
+ SourcePackageName spn
WHERE
secsrc.archive = %s AND secsrc.status IN (%s, %s) AND
secsrc.distroseries = %s AND secsrc.pocket = %s AND
@@ -318,22 +321,29 @@
# the target archive.
find_origin_only_packages = """
INSERT INTO tmp_merge_copy_data (
- s_sspph, s_sourcepackagerelease, sourcepackagename, s_version,
- missing, s_status, s_component, s_section)
+ s_sspph, s_sourcepackagerelease, sourcepackagename,
+ sourcepackagename_id, s_version, missing, s_status,
+ s_component, s_section)
SELECT
secsrc.id AS s_sspph,
secsrc.sourcepackagerelease AS s_sourcepackagerelease,
- spn.name AS sourcepackagename, spr.version AS s_version,
- True AS missing, secsrc.status AS s_status,
- secsrc.component AS s_component, secsrc.section AS s_section
- FROM
- sourcepackagepublishinghistory secsrc,
- sourcepackagerelease spr, sourcepackagename spn
+ spn.name AS sourcepackagename,
+ spn.id AS sourcepackagename_id,
+ spr.version AS s_version,
+ True AS missing,
+ secsrc.status AS s_status,
+ secsrc.component AS s_component,
+ secsrc.section AS s_section
+ FROM SourcePackagePublishingHistory secsrc
+ JOIN SourcePackageRelease AS spr ON
+ spr.id = secsrc.sourcepackagerelease
+ JOIN SourcePackageName AS spn ON
+ spn.id = spr.sourcepackagename
WHERE
- secsrc.archive = %s AND secsrc.status IN (%s, %s) AND
- secsrc.distroseries = %s AND secsrc.pocket = %s AND
- secsrc.sourcepackagerelease = spr.id AND
- spr.sourcepackagename = spn.id AND
+ secsrc.archive = %s AND
+ secsrc.status IN (%s, %s) AND
+ secsrc.distroseries = %s AND
+ secsrc.pocket = %s AND
spn.name NOT IN (
SELECT sourcepackagename FROM tmp_merge_copy_data)
""" % sqlvalues(
@@ -383,7 +393,8 @@
-- recent source package.
obsoleted boolean DEFAULT false NOT NULL,
missing boolean DEFAULT false NOT NULL,
- sourcepackagename text NOT NULL
+ sourcepackagename text NOT NULL,
+ sourcepackagename_id integer NOT NULL
);
CREATE INDEX source_name_index
ON tmp_merge_copy_data USING btree (sourcepackagename);
@@ -392,19 +403,24 @@
# archive considering the distroseries, pocket and component.
pop_query = """
INSERT INTO tmp_merge_copy_data (
- t_sspph, t_sourcepackagerelease, sourcepackagename, t_version)
+ t_sspph, t_sourcepackagerelease, sourcepackagename,
+ sourcepackagename_id, t_version)
SELECT
secsrc.id AS t_sspph,
secsrc.sourcepackagerelease AS t_sourcepackagerelease,
- spn.name AS sourcepackagerelease, spr.version AS t_version
- FROM
- sourcepackagepublishinghistory secsrc,
- sourcepackagerelease spr, sourcepackagename spn
+ spn.name AS sourcepackagerelease,
+ spn.id AS sourcepackagename_id,
+ spr.version AS t_version
+ FROM SourcePackagePublishingHistory secsrc
+ JOIN SourcePackageRelease AS spr ON
+ spr.id = secsrc.sourcepackagerelease
+ JOIN SourcePackageName AS spn ON
+ spn.id = spr.sourcepackagename
WHERE
- secsrc.archive = %s AND secsrc.status IN (%s, %s) AND
- secsrc.distroseries = %s AND secsrc.pocket = %s AND
- secsrc.sourcepackagerelease = spr.id AND
- spr.sourcepackagename = spn.id
+ secsrc.archive = %s AND
+ secsrc.status IN (%s, %s) AND
+ secsrc.distroseries = %s AND
+ secsrc.pocket = %s
""" % sqlvalues(
destination.archive,
PackagePublishingStatus.PENDING,
@@ -464,23 +480,22 @@
query += '''
AND spph.sourcepackagerelease IN (
SELECT spr.id
- FROM SourcePackageRelease AS spr, SourcePackageName AS spn
- WHERE
- spr.sourcepackagename = spn.id AND spn.name IN %s
+ FROM SourcePackageRelease AS spr
+ JOIN SourcePackageName AS spn ON
+ spn.id = spr.sourcepackagename
+ WHERE spn.name IN %s
)''' % sqlvalues(sourcepackagenames)
if origin.packagesets:
query += '''
AND spph.sourcepackagerelease IN (
SELECT spr.id
- FROM
- SourcePackageRelease AS spr,
- packagesetsources AS pss,
- flatpackagesetinclusion AS fpsi
- WHERE
- spr.sourcepackagename = pss.sourcepackagename AND
- pss.packageset = fpsi.child AND
- fpsi.parent in %s
+ FROM SourcePackageRelease AS spr
+ JOIN PackagesetSources AS pss ON
+ PSS.sourcepackagename = spr.sourcepackagename
+ JOIN FlatPackagesetInclusion AS fpsi ON
+ fpsi.child = pss.packageset
+ WHERE fpsi.parent in %s
)
''' % sqlvalues([p.id for p in origin.packagesets])
@@ -526,14 +541,16 @@
"""
fresher_info = sorted(store.execute("""
SELECT sourcepackagename, s_version, t_version
- FROM tmp_merge_copy_data WHERE obsoleted = True;
+ FROM tmp_merge_copy_data
+ WHERE obsoleted = True;
"""))
logger.info('Fresher packages: %d' % len(fresher_info))
for info in fresher_info:
logger.info('* %s (%s > %s)' % info)
new_info = sorted(store.execute("""
SELECT sourcepackagename, s_version
- FROM tmp_merge_copy_data WHERE missing = True;
+ FROM tmp_merge_copy_data
+ WHERE missing = True;
"""))
logger.info('New packages: %d' % len(new_info))
for info in new_info:
Follow ups
