Use left joins in subset queries for nullable foreign keys #3034
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3034 +/- ##
==========================================
- Coverage 34.80% 34.71% -0.10%
==========================================
Files 345 345
Lines 39713 40113 +400
==========================================
+ Hits 13824 13926 +102
- Misses 24273 24564 +291
- Partials 1616 1623 +7 ☔ View full report in Codecov by Sentry. |
| @@ -322,6 +334,12 @@ func (b *sqlSyncBuilder) BuildDestinationConfig(ctx context.Context, params *bb_ | |||
| return nil, fmt.Errorf("unable to parse destination options: %w", err) | |||
| } | |||
|
|
|||
| // skip foreign key violations if the query could return rows that violate foreign key constraints | |||
| skipForeignKeyViolations := destOpts.SkipForeignKeyViolations | |||
There was a problem hiding this comment.
I think this can actually be more concisely written as well as short circuited:
skipForeignKeyViolations := destOpts.SkipForeignKeyViolations || b.isNotForeignKeySafeSubsetMap[tableKey]
| @@ -148,6 +152,14 @@ func (b *sqlSyncBuilder) BuildSourceConfigs(ctx context.Context, params *bb_inte | |||
| return nil, fmt.Errorf("unable to build select queries: %w", err) | |||
| } | |||
|
|
|||
| for table, querymap := range tableRunTypeQueryMap { | |||
| for runtype, q := range querymap { | |||
| if runtype == tabledependency.RunTypeInsert { | |||
There was a problem hiding this comment.
Isn't this slightly inaccurate? Won't it only be not FK subset safe if it contains nullable FKs?
Maybe just my context here is too limited.
There was a problem hiding this comment.
not sure I understand. this map is tracking whether a tables query is fk subset safe. the t/f value comes from the querybuilder
There was a problem hiding this comment.
so in this case youre only using the value if it is an insert, is that what's going on here?
There was a problem hiding this comment.
yeah I mean I could separate it by runtype but the isNotForeignKeySafeSubsetMap value is the same for insert or update. I could remove the runtype check and just have it overwrite the value since the insert and update are the same
There was a problem hiding this comment.
Idk, maybe just a nice comment would suffice
No description provided.