Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

edk2toollib/database: remove autoincrement id #412

Merged
merged 7 commits into from
Sep 25, 2023

Conversation

Javagedes
Copy link
Contributor

@Javagedes Javagedes commented Sep 22, 2023

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.

autoincrementing id's are not needed or used in the tables and add
unecessary complexity when merging databases.
@Javagedes Javagedes added this to the v0.18.1 milestone Sep 22, 2023
@Javagedes Javagedes self-assigned this Sep 22, 2023
@Javagedes Javagedes added the enhancement New feature or request label Sep 22, 2023
@Javagedes Javagedes marked this pull request as ready for review September 22, 2023 20:20
@Javagedes Javagedes marked this pull request as draft September 22, 2023 20:27
@Javagedes Javagedes changed the title remote autoincrement id remove autoincrement id Sep 25, 2023
@Javagedes Javagedes changed the title remove autoincrement id edk2toollib/database: remove autoincrement id Sep 25, 2023
@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (a2c523d) 80.90% compared to head (d0ff2f8) 80.93%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #412      +/-   ##
==========================================
+ Coverage   80.90%   80.93%   +0.02%     
==========================================
  Files          57       57              
  Lines        7276     7284       +8     
==========================================
+ Hits         5887     5895       +8     
  Misses       1389     1389              
Files Coverage Δ
edk2toollib/database/tables/instanced_fv_table.py 100.00% <100.00%> (ø)
edk2toollib/database/tables/instanced_inf_table.py 98.60% <100.00%> (+0.08%) ⬆️
edk2toollib/database/tables/source_table.py 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Javagedes Javagedes marked this pull request as ready for review September 25, 2023 18:55
@Javagedes Javagedes merged commit e8eb612 into tianocore:master Sep 25, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants