[DNM]: test#12656
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
[FORMAT CHECKER NOTIFICATION] Notice: To remove the 📖 For more info, you can check the "Contribute Code" section in the development guide. |
|
Closing this test PR because it was based on #12654. I will create a separate master-based CI baseline PR instead. |
There was a problem hiding this comment.
Code Review
This pull request introduces route-aware foreign key causality tracking by adding a TableRouteResolver parameter to the downstream foreign key initialization and relation-building processes. This allows the schema tracker to correctly resolve and align routed source tables with their downstream counterparts. Additionally, the caching mechanism was updated to use source table IDs, and comprehensive unit tests were added. The reviewer feedback suggests simplifying a table name string conversion and removing the precheckForeignKeyCausality method, which has become dead code after its call was removed from the syncer.
|
|
||
| parentTableID := utils.GenTableID(targetParentTable) | ||
| parentTableName := (&cdcmodel.TableName{Schema: targetParentSchema, Table: fk.RefTable.O}).String() | ||
| parentTableName := (&cdcmodel.TableName{Schema: sourceParentSchema, Table: sourceFK.RefTable.O}).String() |
There was a problem hiding this comment.
The parentTableName can be simplified by calling sourceParentTable.String() directly, which avoids allocating a new cdcmodel.TableName struct and achieves the exact same string representation.
| parentTableName := (&cdcmodel.TableName{Schema: sourceParentSchema, Table: sourceFK.RefTable.O}).String() | |
| parentTableName := sourceParentTable.String() |
| return nil, err | ||
| } | ||
| return s.schemaTracker.InitDownStreamForeignKeyRelations(tctx, sourceTable, targetTable, originTI) | ||
| return s.schemaTracker.InitDownStreamForeignKeyRelations(tctx, sourceTable, targetTable, originTI, s.route) |
Test-fix PR based on #12654.
Purpose: validate whether the failing DM integration CI is related to the route-aware FK relation path not being wired into syncer.
Change: pass syncer's route resolver into schema tracker FK relation initialization, so the new route-aware builder can validate route alignment instead of always using identity route.
Issue Number: ref #12654
Tests:
Release note: None