Merge lp:~jml/pkgme-devportal/uniqueness-constraint into lp:pkgme-devportal

[Jonathan Lange]

Relax the uniqueness constraint to include architecture.

This relaxes the uniqueness constraint so that when we rollout a patch
that populates architecture, we will be able to add the same dependency
for multiple architectures.

Oh, I'm not convinced that landing this is a good idea, so please don't Approve it without discussing with me.

[James Westby (james-w)]

Hi,

  1. If this is going in the patch-00001.sql file then we want to land it before applying that patch?
  2. What makes you think this isn't a good idea.
  3. Perhaps we want a XFAIL test for now?
  4. In fact I'm not sure why the test passes? Is NULL always considered unique?

Thanks,

James

review: Needs Information

[Jonathan Lange (jml)]

On 22 August 2012 19:45, James Westby <email address hidden> wrote:
> Review: Needs Information
>
> Hi,
>
> 1. If this is going in the patch-00001.sql file then we want to land it before applying that patch?

Yes. I don't think it really matters though, as could easily make this
patch-00002.

> 2. What makes you think this isn't a good idea.

See below.

> 3. Perhaps we want a XFAIL test for now?

I'm not sure, tbh.

> 4. In fact I'm not sure why the test passes? Is NULL always considered unique?

Yes. That's what got me. Specifically (a, b, c, NULL) is considered
not equal to (a, b, c, NULL) for the purposes of uniqueness, even
though (a, b, c) _is_ considered equal to (a, b, c).

I don't currently see a path forward that doesn't involve potential
data corruption or downtime.

jml

[James Westby (james-w)]

We decided not to do this as part of the first patch, and the second patch
makes it slightly different.

Thanks,

James

Unmerged revisions

84. By Jonathan Lange on 2012-08-17

Flakes

83. By Jonathan Lange on 2012-08-17

Update the uniqueness constraints. Change the expected behaviour for double inserts.

=== modified file 'db/patch-00001.sql'
--- db/patch-00001.sql	2012-08-16 10:09:58 +0000
+++ db/patch-00001.sql	2012-08-17 17:19:18 +0000
@@ -1,3 +1,10 @@
 
 ALTER TABLE libdep
   ADD COLUMN architecture TEXT;
+
+ALTER TABLE libdep
+  DROP CONSTRAINT libdep_uniq;
+
+ALTER TABLE libdep
+  ADD CONSTRAINT libdep_uniq
+  UNIQUE (source_package_name, library, dependency, architecture);

=== modified file 'devportalbinary/tests/test_database.py'
--- devportalbinary/tests/test_database.py	2012-07-26 17:51:15 +0000
+++ devportalbinary/tests/test_database.py	2012-08-17 17:19:18 +0000
@@ -3,7 +3,6 @@
 from fixtures import (
     TempDir,
     )
-from storm.databases.postgres import psycopg2
 from testresources import ResourcedTestCase
 from testtools import TestCase
 from testtools.matchers import (
@@ -53,9 +52,9 @@
     def test_double_insert(self):
         db = self.get_package_db()
         db.insert_new_library('foo-src', 'libfoo.so.0', 'foo')
-        self.assertRaises(
-            psycopg2.IntegrityError,
-            db.insert_new_library, 'foo-src', 'libfoo.so.0', 'foo')
+        db.insert_new_library('foo-src', 'libfoo.so.0', 'foo')
+        deps = db.get_dependencies('libfoo.so.0')
+        self.assertEqual(deps, set(['foo']))
 
     def test_differing_dependencies(self):
         db = self.get_package_db()