From e8eb612f019b00cd7df13e938b39c3e6e664e0d3 Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Mon, 25 Sep 2023 15:24:24 -0700 Subject: [PATCH] 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. --- .../database/tables/instanced_fv_table.py | 9 +-- .../database/tables/instanced_inf_table.py | 80 ++++++++++++------- edk2toollib/database/tables/source_table.py | 1 - .../database/test_instanced_fv_table.py | 3 +- .../database/test_instanced_inf_table.py | 48 +++++------ 5 files changed, 81 insertions(+), 60 deletions(-) diff --git a/edk2toollib/database/tables/instanced_fv_table.py b/edk2toollib/database/tables/instanced_fv_table.py index 8d9e5edf..e1b54fdc 100644 --- a/edk2toollib/database/tables/instanced_fv_table.py +++ b/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) diff --git a/edk2toollib/database/tables/instanced_inf_table.py b/edk2toollib/database/tables/instanced_inf_table.py index a3f76ae8..f433d1f7 100644 --- a/edk2toollib/database/tables/instanced_inf_table.py +++ b/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. diff --git a/edk2toollib/database/tables/source_table.py b/edk2toollib/database/tables/source_table.py index 87b37667..05e680f5 100644 --- a/edk2toollib/database/tables/source_table.py +++ b/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, diff --git a/tests.unit/database/test_instanced_fv_table.py b/tests.unit/database/test_instanced_fv_table.py index 919cb602..df0b8ab2 100644 --- a/tests.unit/database/test_instanced_fv_table.py +++ b/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([ diff --git a/tests.unit/database/test_instanced_inf_table.py b/tests.unit/database/test_instanced_inf_table.py index 00e4194b..3f011821 100644 --- a/tests.unit/database/test_instanced_inf_table.py +++ b/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), [])