launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:archive-fk-indexes into launchpad:master.

Commit message:
Add various indexes for FKs referring to Archive

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400538

Also add a test for this and for FKs referring to Job, since this is otherwise very easy to forget.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:archive-fk-indexes into launchpad:master.

diff --git a/database/schema/patch-2210-11-3.sql b/database/schema/patch-2210-11-3.sql
new file mode 100644
index 0000000..3552870
--- /dev/null
+++ b/database/schema/patch-2210-11-3.sql
@@ -0,0 +1,25 @@
+-- Copyright 2021 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+CREATE INDEX binarypackagepublishinghistory__copied_from_archive__idx
+    ON BinaryPackagePublishingHistory(copied_from_archive)
+    WHERE copied_from_archive IS NOT NULL;
+
+CREATE INDEX packagecopyjob__source_archive__idx
+    ON PackageCopyJob(source_archive);
+
+CREATE INDEX packagecopyrequest__source_archive__idx
+    ON PackageCopyRequest(source_archive);
+
+CREATE INDEX snap__auto_build_archive__idx
+    ON Snap(auto_build_archive)
+    WHERE auto_build_archive IS NOT NULL;
+
+CREATE INDEX sourcepackagepublishinghistory__copied_from_archive__idx
+    ON SourcePackagePublishingHistory(copied_from_archive)
+    WHERE copied_from_archive IS NOT NULL;
+
+CREATE INDEX sourcepackagerecipebuild__archive__idx
+    ON SourcePackageRecipeBuild(archive);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 11, 3);
diff --git a/lib/lp/services/database/postgresql.py b/lib/lp/services/database/postgresql.py
index b735476..f11afac 100644
--- a/lib/lp/services/database/postgresql.py
+++ b/lib/lp/services/database/postgresql.py
@@ -127,41 +127,47 @@ def listReferences(cur, table, column, indirect=True, _state=None):
     return _state
 
 
-def listUniques(cur, table, column):
-    '''Return a list of unique indexes on `table` that include the `column`
+def listIndexes(cur, table, column, only_unique=False):
+    '''Return a list of indexes on `table` that include the `column`
 
     `cur` must be an open DB-API cursor.
 
     Returns [ (column, [...]) ]. The column passed in will always be
     included in the tuple.
 
-    Simple UNIQUE index
+    Simple indexes
 
-    >>> print(_py3ish_repr(listUniques(cur, 'b', 'aid')))
+    >>> print(_py3ish_repr(listIndexes(cur, 'b', 'aid')))
     [('aid',)]
+    >>> print(_py3ish_repr(listIndexes(cur, 'a', 'name')))
+    [('name',)]
 
-    Primary keys are UNIQUE indexes too
+    Primary keys are indexes too
 
-    >>> print(_py3ish_repr(listUniques(cur, 'a', 'aid')))
+    >>> print(_py3ish_repr(listIndexes(cur, 'a', 'aid')))
     [('aid',)]
 
     Compound indexes
 
-    >>> print(_py3ish_repr(listUniques(cur, 'c', 'aid')))
+    >>> print(_py3ish_repr(listIndexes(cur, 'c', 'aid')))
     [('aid', 'bid')]
-    >>> print(_py3ish_repr(listUniques(cur, 'c', 'bid')))
+    >>> print(_py3ish_repr(listIndexes(cur, 'c', 'bid')))
     [('aid', 'bid')]
+    >>> print(_py3ish_repr(listIndexes(cur, 'c', 'name')))
+    [('name', 'description')]
+    >>> print(_py3ish_repr(listIndexes(cur, 'c', 'description')))
+    [('name', 'description')]
 
     And any combination
 
-    >>> l = listUniques(cur, 'd', 'aid')
+    >>> l = listIndexes(cur, 'd', 'aid')
     >>> l.sort()
     >>> print(_py3ish_repr(l))
     [('aid',), ('aid', 'bid')]
 
-    If there are no UNIQUE indexes using the secified column
+    If there are no indexes using the secified column
 
-    >>> listUniques(cur, 'a', 'selfref')
+    >>> listIndexes(cur, 'a', 'selfref')
     []
 
     '''
@@ -185,16 +191,17 @@ def listUniques(cur, table, column):
     # Initialize our return value
     rv = []
 
-    # Retrive the UNIQUE indexes.
+    # Retrieve the indexes.
     sql = '''
         SELECT
             i.indkey
         FROM
             pg_class AS t JOIN pg_index AS i ON i.indrelid = t.oid
         WHERE
-            i.indisunique = true
-            AND t.relname = %(table)s
+            t.relname = %(table)s
         '''
+    if only_unique:
+        sql += ' AND i.indisunique = true'
     cur.execute(sql, dict(table=table))
     for indkey, in cur.fetchall():
         # We have a space separated list of integer keys into the attribute
@@ -210,6 +217,53 @@ def listUniques(cur, table, column):
     return rv
 
 
+def listUniques(cur, table, column):
+    '''Return a list of unique indexes on `table` that include the `column`
+
+    `cur` must be an open DB-API cursor.
+
+    Returns [ (column, [...]) ]. The column passed in will always be
+    included in the tuple.
+
+    Simple UNIQUE index
+
+    >>> print(_py3ish_repr(listUniques(cur, 'b', 'aid')))
+    [('aid',)]
+
+    Primary keys are UNIQUE indexes too
+
+    >>> print(_py3ish_repr(listUniques(cur, 'a', 'aid')))
+    [('aid',)]
+
+    Compound indexes
+
+    >>> print(_py3ish_repr(listUniques(cur, 'c', 'aid')))
+    [('aid', 'bid')]
+    >>> print(_py3ish_repr(listUniques(cur, 'c', 'bid')))
+    [('aid', 'bid')]
+
+    And any combination
+
+    >>> l = listUniques(cur, 'd', 'aid')
+    >>> l.sort()
+    >>> print(_py3ish_repr(l))
+    [('aid',), ('aid', 'bid')]
+
+    If there are no UNIQUE indexes using the secified column
+
+    >>> listUniques(cur, 'a', 'selfref')
+    []
+    >>> listUniques(cur, 'a', 'name')
+    []
+    >>> listUniques(cur, 'c', 'name')
+    []
+    >>> listUniques(cur, 'c', 'description')
+    []
+
+    '''
+    return listIndexes(cur, table, column, only_unique=True)
+
+
 def listSequences(cur):
     """Return a list of (schema, sequence, table, column) tuples.
 
diff --git a/lib/lp/services/database/tests/test_indexes.py b/lib/lp/services/database/tests/test_indexes.py
new file mode 100644
index 0000000..f19c9ff
--- /dev/null
+++ b/lib/lp/services/database/tests/test_indexes.py
@@ -0,0 +1,52 @@
+# Copyright 2021 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test database index correctness."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from testscenarios import (
+    load_tests_apply_scenarios,
+    WithScenarios,
+    )
+
+from lp.services.database.postgresql import (
+    listIndexes,
+    listReferences,
+    )
+from lp.services.database.sqlbase import cursor
+from lp.testing import TestCase
+from lp.testing.layers import ZopelessDatabaseLayer
+
+
+class TestIndexedReferences(WithScenarios, TestCase):
+    """Test that all references to certain tables are indexed.
+
+    Without this, we may run into problems deleting rows from those tables.
+    """
+
+    layer = ZopelessDatabaseLayer
+
+    scenarios = [
+        ("Archive", {"table": "archive", "column": "id"}),
+        ("Job", {"table": "job", "column": "id"}),
+        ]
+
+    def test_references_are_indexed(self):
+        cur = cursor()
+        self.addCleanup(cur.close)
+        references = list(
+            listReferences(cur, self.table, self.column, indirect=False))
+        missing = []
+        for src_tab, src_col, _, _, _, _ in references:
+            for index in listIndexes(cur, src_tab, src_col):
+                if index[0] == src_col:
+                    break
+            else:
+                missing.append((src_tab, src_col))
+        self.assertEqual([], missing)
+
+
+load_tests = load_tests_apply_scenarios
diff --git a/lib/lp/services/database/tests/test_postgresql.py b/lib/lp/services/database/tests/test_postgresql.py
index 158a8f7..410801e 100644
--- a/lib/lp/services/database/tests/test_postgresql.py
+++ b/lib/lp/services/database/tests/test_postgresql.py
@@ -20,10 +20,14 @@ def setUp(test):
     cur.execute("""
         CREATE TABLE A (
             aid     serial PRIMARY KEY,
-            selfref integer CONSTRAINT a_selfref_fk REFERENCES A(aid)
+            selfref integer CONSTRAINT a_selfref_fk REFERENCES A(aid),
+            name    text
             )
         """)
     cur.execute("""
+        CREATE INDEX a__name__idx ON A(name)
+        """)
+    cur.execute("""
         CREATE TABLE B (
             bid integer PRIMARY KEY,
             aid integer UNIQUE CONSTRAINT b_aid_fk REFERENCES A(aid)
@@ -35,10 +39,15 @@ def setUp(test):
             cid integer PRIMARY KEY,
             aid integer CONSTRAINT c_aid_fk REFERENCES B(aid),
             bid integer CONSTRAINT c_bid_fk REFERENCES B(bid),
-            CONSTRAINT c_aid_bid_key UNIQUE (aid, bid)
+            CONSTRAINT c_aid_bid_key UNIQUE (aid, bid),
+            name text,
+            description text
             )
         """)
     cur.execute("""
+        CREATE INDEX c__name__description__idx ON C(name, description)
+        """)
+    cur.execute("""
         CREATE TABLE D (
             did integer PRIMARY KEY,
             aid integer UNIQUE CONSTRAINT d_aid_fk REFERENCES B(aid),