edk2toollib/database: remove autoincrement id (#412)

Removes autoincrementing primary keys from `instanced_fv`, `instanced_inf` and `source` tables. Tables depending on an autoincremented index as a foreign key end up breaking when doing a naive merge of two databases as the index is automatically updated in the table where it is the primary key, but it is not updated in the table where it is a foreign key, thus breaking the association.

To support merging two databases where we will never know the true tables in the database (A developer can register a subset of provided tables or create their own), core tables should not use autoincremented primary keys.

## instanced_fv table

This removal was simple as there was no existing dependencies that utilized the auto-incremented primary key. Getting the same association can be done via combining the `env` and `fv_name` columns.

## instanced_inf table

This removal is more complex as the autoincremented id was directly used when associating the inf's with each other (when forming a dependency list). Getting the same association can be done via combining the `env` and `component` columns. This change made it such that the provided `junction` table does not work, and an `instanced_inf_junction` table was created to support this.

## source table

This removal was simple as there was no existing dependencies that utilized the auto-incremented primary key. Getting the same association can be done via the `path` column.

edk2toollib/database/tables/instanced_fv_table.py

@@ -18,7 +18,6 @@
 
 CREATE_INSTANCED_FV_TABLE = """
 CREATE TABLE IF NOT EXISTS instanced_fv (
-    id INTEGER PRIMARY KEY AUTOINCREMENT,
     env INTEGER,
     fv_name TEXT,
     fdf TEXT,
@@ -49,11 +48,12 @@ def create_tables(self, db_cursor: sqlite3.Cursor) -> None:
         """Create the tables necessary for this parser."""
         db_cursor.execute(CREATE_INSTANCED_FV_TABLE)
 
-    def parse(self, db_cursor: sqlite3.Cursor, pathobj: Edk2Path, id, env) -> None:
+    def parse(self, db_cursor: sqlite3.Cursor, pathobj: Edk2Path, env_id, env) -> None:
         """Parse the workspace and update the database."""
         self.pathobj = pathobj
         self.ws = Path(self.pathobj.WorkspacePath)
         self.env = env
+        self.env_id = env_id
         self.dsc = self.env.get("ACTIVE_PLATFORM", None)
         self.fdf = self.env.get("FLASH_DEFINITION", None)
         self.arch = self.env["TARGET_ARCH"].split(" ")
@@ -78,10 +78,9 @@ def parse(self, db_cursor: sqlite3.Cursor, pathobj: Edk2Path, id, env) -> None:
                     inf = str(Path(self.pathobj.GetEdk2RelativePathFromAbsolutePath(inf)))
                 inf_list.append(Path(inf).as_posix())
 
-            row = (id, fv, Path(self.fdf).name, self.fdf)
+            row = (self.env_id, fv, Path(self.fdf).name, self.fdf)
             db_cursor.execute(INSERT_INSTANCED_FV_ROW, row)
-            fv_id = db_cursor.lastrowid
 
             for inf in inf_list:
-                row = (id, "instanced_fv", fv_id, "inf", inf)
+                row = (self.env_id, "instanced_fv", fv, "inf", inf)
                 db_cursor.execute(INSERT_JUNCTION_ROW, row)

edk2toollib/database/tables/instanced_inf_table.py

@@ -19,7 +19,6 @@
 
 CREATE_INSTANCED_INF_TABLE = '''
 CREATE TABLE IF NOT EXISTS instanced_inf (
-    id INTEGER PRIMARY KEY AUTOINCREMENT,
     env INTEGER,
     path TEXT,
     class TEXT,
@@ -28,17 +27,48 @@ class TEXT,
     dsc TEXT,
     component TEXT,
     FOREIGN KEY(env) REFERENCES environment(env)
-)
+);
+'''
+
+CREATE_INSTANCED_INF_TABLE_INDEX = '''
+CREATE INDEX IF NOT EXISTS instanced_inf_index ON instanced_inf (env);
+'''
+
+CREATE_INSTANCED_INF_TABLE_JUNCTION = '''
+CREATE TABLE IF NOT EXISTS instanced_inf_junction (
+    env INTEGER,
+    component TEXT,
+    instanced_inf1 TEXT,
+    instanced_inf2 TEXT,
+    FOREIGN KEY(env) REFERENCES environment(env)
+);
+'''
+
+CREATE_INSTANCED_INF_TABLE_JUNCTION_INDEX = '''
+CREATE INDEX IF NOT EXISTS instanced_inf_junction_index ON instanced_inf_junction (env);
+'''
+
+CREATE_INSTANCED_INF_SOURCE_TABLE_JUNCTION = '''
+CREATE TABLE IF NOT EXISTS instanced_inf_source_junction (env, component, instanced_inf, source);
+'''
+
+CREATE_INSTANCED_INF_SOURCE_TABLE_JUNCTION_INDEX = '''
+CREATE INDEX IF NOT EXISTS instanced_inf_source_junction_index ON instanced_inf_source_junction (env);
 '''
 
 INSERT_INSTANCED_INF_ROW = '''
 INSERT INTO instanced_inf (env, path, class, name, arch, dsc, component)
-VALUES (?, ?, ?, ?, ?, ?, ?)
+VALUES (?, ?, ?, ?, ?, ?, ?);
+'''
+
+INSERT_INF_TABLE_JUNCTION_ROW = '''
+INSERT INTO instanced_inf_junction (env, component, instanced_inf1, instanced_inf2)
+VALUES (?, ?, ?, ?);
 '''
 
-INSERT_JUNCTION_ROW = '''
-INSERT INTO junction (env, table1, key1, table2, key2)
-VALUES (?, ?, ?, ?, ?)
+INSERT_INF_TABLE_SOURCE_JUNCTION_ROW = '''
+INSERT INTO instanced_inf_source_junction (env, component, instanced_inf, source)
+VALUES (?, ?, ?, ?);
 '''
 
 class InstancedInfTable(TableGenerator):
@@ -55,6 +85,11 @@ def __init__(self, *args, **kwargs):
     def create_tables(self, db_cursor: Cursor) -> None:
         """Create the tables necessary for this parser."""
         db_cursor.execute(CREATE_INSTANCED_INF_TABLE)
+        db_cursor.execute(CREATE_INSTANCED_INF_TABLE_INDEX)
+        db_cursor.execute(CREATE_INSTANCED_INF_TABLE_JUNCTION)
+        db_cursor.execute(CREATE_INSTANCED_INF_TABLE_JUNCTION_INDEX)
+        db_cursor.execute(CREATE_INSTANCED_INF_SOURCE_TABLE_JUNCTION)
+        db_cursor.execute(CREATE_INSTANCED_INF_SOURCE_TABLE_JUNCTION_INDEX)
 
     def inf(self, inf: str) -> InfP:
         """Returns a parsed INF object.
@@ -102,35 +137,24 @@ def _insert_db_rows(self, db_cursor, env_id, inf_entries) -> int:
         Inserts all inf's into the instanced_inf table and links source files and used libraries via the junction
         table.
         """
-        # Must use "execute" so that db_cursor.lastrowid is updated.
+        # Insert all instanced INF rows
         rows = [
             (env_id, e["PATH"], e.get("LIBRARY_CLASS"), e["NAME"], e["ARCH"], e["DSC"], e["COMPONENT"])
             for e in inf_entries
         ]
-        for row in rows:
-            db_cursor.execute(INSERT_INSTANCED_INF_ROW, row)
-        last_inserted_id = db_cursor.lastrowid
+        db_cursor.executemany(INSERT_INSTANCED_INF_ROW, rows)
 
-        # Mapping used for linking libraries together in the junction table so that the value does not need to be
-        # queried, which reduces performance greatly.
-        id_mapping = {}
-        for idx, e in enumerate(inf_entries):
-            id_mapping[e["PATH"], e["COMPONENT"]] = last_inserted_id - len(inf_entries) + 1 + idx
+        # Link instanced INF sources
+        rows = []
+        for e in inf_entries:
+            rows += [(env_id, e["COMPONENT"], e["PATH"], source) for source in e["SOURCES_USED"]]
+        db_cursor.executemany(INSERT_INF_TABLE_SOURCE_JUNCTION_ROW, rows)
 
-        # Insert rows into the tables
+        # Link instanced INF libraries
         rows = []
-        for idx, e in enumerate(inf_entries):
-            inf_id = last_inserted_id - len(inf_entries) + 1 + idx
-
-            # Link all source files to this instanced_inf
-            rows += [(env_id, "instanced_inf", inf_id, "source", source) for source in e["SOURCES_USED"]]
-
-            # link other libraries used by this instanced_inf
-            rows += [
-                (env_id, "instanced_inf", inf_id, "instanced_inf", id_mapping.get((library, e["COMPONENT"]), None))
-                for library in e["LIBRARIES_USED"]
-            ]
-        db_cursor.executemany(INSERT_JUNCTION_ROW, rows)
+        for e in inf_entries:
+            rows += [(env_id, e["COMPONENT"], e["PATH"], library) for library in e["LIBRARIES_USED"]]
+        db_cursor.executemany(INSERT_INF_TABLE_JUNCTION_ROW, rows)
 
     def _build_inf_table(self, dscp: DscP):
         """Create the instanced inf entries, including components and libraries.

edk2toollib/database/tables/source_table.py

@@ -21,7 +21,6 @@
 
 CREATE_SOURCE_TABLE = '''
 CREATE TABLE IF NOT EXISTS source (
-    id INTEGER PRIMARY KEY AUTOINCREMENT,
     path TEXT UNIQUE,
     license TEXT,
     total_lines INTEGER,

tests.unit/database/test_instanced_fv_table.py

@@ -54,8 +54,7 @@ def test_valid_fdf(empty_tree: Tree):  # noqa: F811
     }
     db.parse(env)
 
-    fv_id = db.connection.execute("SELECT id FROM instanced_fv WHERE fv_name = 'infformat'").fetchone()[0]
-    rows = db.connection.execute("SELECT key2 FROM junction where key1 == ?", (fv_id,)).fetchall()
+    rows = db.connection.execute("SELECT key2 FROM junction where key1 == 'infformat'").fetchall()
 
     assert len(rows) == 5
     assert sorted(rows) == sorted([

tests.unit/database/test_instanced_inf_table.py

@@ -17,15 +17,13 @@
 from edk2toollib.uefi.edk2.path_utilities import Edk2Path
 
 GET_USED_LIBRARIES_QUERY = """
-SELECT i.path
-FROM instanced_inf AS i
-JOIN junction AS j ON i.id = j.key2 and j.table2 = "instanced_inf"
-WHERE j.key1 = (
-    SELECT id
-    FROM instanced_inf
-    WHERE name = ? AND arch = ?
-    LIMIT 1
-);
+SELECT ii.path
+FROM instanced_inf AS ii
+JOIN instanced_inf_junction AS iij
+ON ii.path = iij.instanced_inf2
+WHERE
+    iij.component = ?
+    AND ii.arch = ?
 """
 
 def test_valid_dsc(empty_tree: Tree):
@@ -50,7 +48,7 @@ def test_valid_dsc(empty_tree: Tree):
 
     rows = db.connection.cursor().execute("SELECT * FROM instanced_inf").fetchall()
     assert len(rows) == 1
-    assert rows[0][4] == Path(comp1).stem
+    assert rows[0][3] == Path(comp1).stem
 
 def test_no_active_platform(empty_tree: Tree, caplog):
     """Tests that the dsc table returns immediately when no ACTIVE_PLATFORM is defined."""
@@ -138,7 +136,7 @@ def test_library_override(empty_tree: Tree):
     }
     db.parse(env)
     db.connection.execute("SELECT * FROM junction").fetchall()
-    library_list = db.connection.cursor().execute(GET_USED_LIBRARIES_QUERY, ("TestDriver1", "IA32"))
+    library_list = db.connection.cursor().execute(f"SELECT instanced_inf2 FROM instanced_inf_junction WHERE instanced_inf1 = '{Path(comp1).as_posix()}'").fetchall()
 
     for path, in library_list:
         assert path in [Path(lib2).as_posix(), Path(lib3).as_posix()]
@@ -180,11 +178,11 @@ def test_scoped_libraries1(empty_tree: Tree):
     db.parse(env)
 
     for arch in ["IA32", "X64"]:
-        for component, in db.connection.execute("SELECT name FROM instanced_inf WHERE component IS NULL and arch is ?;", (arch,)):
+        for component, in db.connection.execute("SELECT path FROM instanced_inf WHERE component = path and arch is ?;", (arch,)):
             component_lib = db.connection.execute(GET_USED_LIBRARIES_QUERY, (component, arch)).fetchone()[0]
-            assert component.replace("Driver", "Lib") in component_lib
+            assert Path(component).name.replace("Driver", "Lib") == Path(component_lib).name
 
-    results = db.connection.execute('SELECT key2 FROM junction WHERE table1 = "instanced_inf" AND table2 = "source"').fetchall()
+    results = db.connection.execute('SELECT source FROM instanced_inf_source_junction').fetchall()
     assert len(results) == 3
     for source, in results:
         assert source in ["File1.c", "File2.c", "File3.c"]
@@ -222,9 +220,9 @@ def test_scoped_libraries2(empty_tree: Tree):
     db.parse(env)
 
     for arch in ["IA32", "X64"]:
-        for component, in db.connection.execute("SELECT name FROM instanced_inf WHERE component IS NULL and arch is ?;", (arch,)):
+        for component, in db.connection.execute("SELECT path FROM instanced_inf WHERE component = path and arch is ?;", (arch,)):
             component_lib = db.connection.execute(GET_USED_LIBRARIES_QUERY, (component, arch)).fetchone()[0]
-            assert component.replace("Driver", "Lib") in component_lib
+            assert Path(component).name.replace("Driver", "Lib") == Path(component_lib).name
 
 def test_missing_library(empty_tree: Tree):
     """Test when a library is missing."""
@@ -246,7 +244,7 @@ def test_missing_library(empty_tree: Tree):
         "TARGET": "DEBUG",
     }
     db.parse(env)
-    key2 = db.connection.execute("SELECT key2 FROM junction").fetchone()[0]
+    key2 = db.connection.execute("SELECT instanced_inf2 FROM instanced_inf_junction").fetchone()[0]
     assert key2 is None  # This library class does not have an instance available, so key2 should be None
 
 def test_multiple_library_class(empty_tree: Tree):
@@ -281,17 +279,19 @@ def test_multiple_library_class(empty_tree: Tree):
 
     db.parse(env)
 
-    results = db.connection.execute("SELECT key1, key2 FROM junction").fetchall()
+    results = db.connection.execute("SELECT component, instanced_inf1, instanced_inf2 FROM instanced_inf_junction").fetchall()
 
     # Verify that TestDriver1 uses TestLib acting as TestCls1
-    assert results[0] == ('2','1') # idx 2 is TestDriver1, idx1 is TestLib1 acting as TestCsl1
-    assert ("TestLib", "TestCls1") == db.connection.execute("SELECT name, class FROM instanced_inf where id = 1").fetchone()
-    assert ("TestDriver1",) == db.connection.execute("SELECT name FROM instanced_inf where id = 2").fetchone()
+    assert results[0] == (Path(comp1).as_posix(), Path(comp1).as_posix(), Path(lib1).as_posix()) # idx 2 is TestDriver1, idx1 is TestLib1 acting as TestCsl1
+    assert ("TestLib", "TestCls1") == db.connection.execute(
+        "SELECT name, class FROM instanced_inf where path = ? AND component = ?",
+        (Path(lib1).as_posix(), Path(comp1).as_posix())).fetchone()
 
     # Verify that TestDriver2 uses TestLib acting as TestCls2
-    assert results[1] == ('4', '3')  # idx 4 is TestDriver2, idx 3 is TestLib1 acting as TestCls2
-    assert ("TestLib", "TestCls2") == db.connection.execute("SELECT name, class FROM instanced_inf where id = 3").fetchone()
-    assert ("TestDriver2",) == db.connection.execute("SELECT name FROM instanced_inf where id = 4").fetchone()
+    assert results[1] == (Path(comp2).as_posix(), Path(comp2).as_posix(), Path(lib1).as_posix())  # idx 4 is TestDriver2, idx 3 is TestLib1 acting as TestCls2
+    assert ("TestLib", "TestCls2") == db.connection.execute(
+        "SELECT name, class FROM instanced_inf where path = ? AND component = ?",
+        (Path(lib1).as_posix(), Path(comp2).as_posix())).fetchone()
 
 def test_absolute_paths_in_dsc(empty_tree: Tree):
     edk2path = Edk2Path(str(empty_tree.ws), [])