-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add support to secondary tables relationships #218
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis PR adds support for secondary tables relationships in SQLAlchemy mapper by modifying how relationships with secondary tables are handled in the loader and mapper components. The changes primarily focus on adjusting the query construction and data retrieval logic to properly handle relationships that use junction/association tables. ER diagram for Employee and Department with secondary tableerDiagram
EMPLOYEE {
int id PK
string name
string role
}
DEPARTMENT {
int id PK
string name
}
EMPLOYEE_DEPARTMENT_JOIN_TABLE {
int employee_id PK
int department_id PK
}
EMPLOYEE ||--o{ EMPLOYEE_DEPARTMENT_JOIN_TABLE : ""
DEPARTMENT ||--o{ EMPLOYEE_DEPARTMENT_JOIN_TABLE : ""
ER diagram for Employee, Department, and Building with secondary tableserDiagram
EMPLOYEE {
int id PK
string name
string role
}
DEPARTMENT {
int id PK
string name
}
BUILDING {
int id PK
string name
}
EMPLOYEE_DEPARTMENT_JOIN_TABLE {
int employee_id PK
int department_id PK
}
EMPLOYEE_BUILDING_JOIN_TABLE {
int employee_id PK
int building_id PK
}
EMPLOYEE ||--o{ EMPLOYEE_DEPARTMENT_JOIN_TABLE : ""
DEPARTMENT ||--o{ EMPLOYEE_DEPARTMENT_JOIN_TABLE : ""
EMPLOYEE ||--o{ EMPLOYEE_BUILDING_JOIN_TABLE : ""
BUILDING ||--o{ EMPLOYEE_BUILDING_JOIN_TABLE : ""
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #218 +/- ##
==========================================
+ Coverage 85.51% 87.03% +1.51%
==========================================
Files 16 16
Lines 1629 2105 +476
Branches 139 148 +9
==========================================
+ Hits 1393 1832 +439
- Misses 173 207 +34
- Partials 63 66 +3
|
CodSpeed Performance ReportMerging #218 will not alter performanceComparing Summary
|
Fixes #19
Description
This PR fixes the relationships involving secondary tables by primarily updating the logic in
loader.py
.Notably, a new flag,
disabled_optimization_to_secondary_tables
, has been introduced in theself._scalars_all
function. This flag is required to disable SQLAlchemy's default optimization, which retrieves only therelated_model
values. In our case, this optimization must be disabled to retrieve values for both theself_model
and therelated_model
.This adjustment is necessary because the
keys
variable is used to match data from both theself_model
and therelated_model
. Please review the implementation of this flag and its impact on the overall query behavior.IMPORTANT
During development, I discovered an issue that causes the
ModelConnection
type to be duplicated when we have relationship. After investigation, it seems this problem is related to the Relay implementation rather than the secondary table logic.To address this, I will create a separate issue. I’ve included a failing test in this PR,
test_query_with_secondary_table_with_values_list
, which demonstrates the issue.Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Add support for secondary table relationships in the SQLAlchemy mapper, addressing a bug and enhancing the loader to handle these relationships efficiently. Include comprehensive tests to ensure the correct implementation of these features.
New Features:
Bug Fixes:
Enhancements:
Tests: