[2.0] fix(ingestion): group deferred FKs by target table to prevent single-FK overwrite (#34057) - #34171
[2.0] fix(ingestion): group deferred FKs by target table to prevent single-FK overwrite (#34057)#34171ulixius9 wants to merge 1 commit into
Conversation
…FK overwrite (#34057) (#34099) * fix(ingestion): group deferred FKs by target table to prevent single-FK overwrite (#34057) When a table has multiple FKs whose referred tables are not yet ingested, all FK entries land in context.get_global().foreign_tables and are later processed by yield_table_constraints. The previous implementation fetched the target table and sent a PATCH for each FK individually. Because every iteration fetched the pre-patch table state as original_entity, the PATCH diff was always "add one FK from empty", so each patch *replaced* the previous FK with the new one — leaving only the last FK alphabetically. Fix: group deferred FK entries by target table FQN before processing, then fetch the target table exactly once per group and build a single PATCH that contains all FK constraints for that table. Also adds TestYieldTableConstraintsGrouping with two cases: - three deferred FKs for the same table yield exactly one patch with all three constraints - deferred FKs for different tables each produce their own patch Fixes #34057 * fix(ingestion): fetch tableConstraints before patching deferred FKs (#34057) get_by_name without fields returns tableConstraints=None (not a default field), so the patch became `add /tableConstraints` and replaced every stored constraint — PK/UNIQUE and FKs resolved at table creation — with only the deferred ones. Fetch the field so the patch appends instead. Also fix the basedpyright errors from the re-indented block, skip entries whose FQN cannot be built, and make the grouping tests actually run: the spec'd MagicMock had no `metadata` attribute and MagicMock tables failed PatchRequest validation, so every result was a StackTraceError. Adds a regression test that fails without the fields fix. --------- Co-authored-by: ulixius9 <mayursingal9@gmail.com> (cherry picked from commit 10ef554)
ulixius9
added
the
safe to test
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
Code Review ✅ Approved🔴 High risk · Deferred-FK PATCHes change persisted table-constraint metadata alongside existing constraints. Backport of deferred foreign key grouping fix to the 2.0 branch. Tables with multiple FKs to deferred targets no longer lose all but the last one. Unit tests pass with no issues found. OptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
✅ Playwright Results — workflow succeededValidated commit ✅ 110 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 50m 28s ⏱️ Max setup 3m 0s · max shard execution 12m 36s · max shard-job elapsed before upload 17m 35s · reporting 3s 🌐 210.25 requests/attempt · 1.79 app boots/UI scenario · 0.00% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
This branch was successfully deployed
2 participants
Backport of #34099 to
2.0(cherry-picked with-xfrom 10ef554, applied cleanly).Fixes #34057 — deferred foreign keys are now grouped by target table, so a table with multiple FKs to deferred targets no longer keeps only the last one.
Verified:
tests/unit/topology/database/test_common_db_source.py— 27 passed on the 2.0 branch.🤖 Generated with Claude Code