Repository navigation
Fixes #34933: Match FK referred columns to the parent table's column case [Backport #34934 - 2.0] - #35311
Conversation
…n case (#34934) MySQL can keep a foreign key's referenced column as written in the FK definition (`REFERENCES limitadjustmentreasons (id)`) while the parent column is `Id`. Ingestion built referredColumns from that raw name and the server validates referred columns case-sensitively, so it rejected the whole child table with `Invalid column name id`, or silently dropped the FK when it was deferred to the end of the run. Resolve each referred column against the parent table already fetched from OpenMetadata: exact match first, then a single case-insensitive match. Use the resolved names for the relationship type too, which reported MANY_TO_MANY instead of MANY_TO_ONE. A referred column that still doesn't resolve is retried after all tables are ingested, then skipped with a warning instead of failing the table. Backport to 2.0: the fix's changed lines are the same as on main. The conflicts were in surrounding lines that main has since rewritten, so 2.0 keeps its own versions of them: the typing.Dict/List signature of get_relationship_type and the if/else that sets database_name. The unit test file also imports model_str, which main's version already did. (cherry picked from commit 8aa6ad7)
harshsoni2024
added
Ingestion
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🟡 Medium risk · Case-insensitive foreign-key resolution alters ingested relationship metadata; a unique case-only match on a case-sensitive source could link a constraint to the wrong column. Backport of the MySQL foreign key fix that matches referred columns to the parent table's stored column name, so case differences like VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledOptionsDisplay: 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 |
|
This branch was successfully deployed
2 participants
Describe your changes:
Backport of #34934 (8aa6ad7) to
2.0. Fixes #34933 on 2.0, where it was reported (2.0.3).A MySQL foreign key can keep its referenced column in a different case from the parent column (
REFERENCES parent (id)while the column isId). Ingestion sent that raw name inreferredColumns, and the server matches column names case-sensitively. It rejected the whole child table withInvalid column name id. When the FK had been deferred to the end of the run, it dropped the FK with only a warning. Ingestion now matches each referred column to the parent table's stored column name, using the parent table it already fetched from OpenMetadata. For the analysis and the manual MySQL tests, see #34933 and #34934.Differences from the main commit
The fix's added and removed lines are the same as in #34934. Two files conflicted, but only on surrounding lines that main has since rewritten, and 2.0 keeps its own versions of those lines:
common_db_source.py: 2.0 still setsdatabase_namewith anif supports_database: … else: …block, which main merged into one line in Fixes 5963: migrate BigQuery CLI E2E to v2 and fix the bugs it found #33946. The fix only movesreferred_column_fqns = []below it. MySQL's connection has nosupportsDatabase, so for MySQL both versions takeself.context.get().database.constraints.py:get_relationship_typekeeps 2.0'sDict/Listsignature.resolve_column_namesis added above it unchanged.test_common_db_source.py: applied cleanly, plus one import (model_str) that main's version of the file already had.The fix also behaves the same on 2.0 because these match main:
_prepare_foreign_constraints: the per-table call and the end-of-runyield_table_constraintspass. They differ only in type annotations.TableRepository.validateTableConstraints→EntityRepository.validateColumn).Tests
Run on this branch, with models generated from 2.0's schemas:
TestPrepareForeignConstraintsReferredColumnCase: 8 passed. With 2.0's original sources, 7 fail and the exact-match case passes, as on main. All 35 tests intest_common_db_source.pypass.tests/unit/topology/databaseandtests/unit/utils, run on 2.0 and on this branch: the only difference is the 7 new tests, which fail on 2.0 and pass here. Both runs have the same 39 failures and 54 errors, none in code this PR changes. Most come from connector packages missing in the local venv. The rest also fail on unmodified 2.0.ruff checkandruff format --checkon the changed files: clean. basedpyright 1.39.3 on the two source files, with 2.0's baseline and--baselinemode=discardas instatic-checks: 0 errors.tests/integration/mysql/test_foreign_key_case.py. Both attempts stopped in fixture setup, before the test ran. On the first, the MySQL testcontainer didn't report ready within 120 s. On the second, the local OpenMetadata server was shut down during the run. The test runs in this PR's CI. On main it fails without the fix and passes with it (Fixes #34933: Match FK referred columns to the parent table's column case #34934).Type of change:
🤖 Generated with Claude Code