fix(snowflake): stop FK collision across tables sharing a constraint name

[ulixius9]

Describe your changes:

Fixes #

I fixed Snowflake foreign-key reflection so constraints no longer collide across tables that share a constraint name. Snowflake constraint names are unique per table (not per schema), and cloned tables (CREATE TABLE ... CLONE / LIKE) copy the name, so SHOW IMPORTED KEYS IN SCHEMA can return the same fk_name for several tables. get_schema_foreign_keys grouped rows by fk_name alone, merging distinct tables' columns into one constraint filed under whichever table appeared first — producing FK constraints that reference another table's columns (e.g. CUSTOMER acquiring SUPPLIER's S_NATIONKEY), which the server rejects with 400 "Invalid column name" and skips the table patch. The grouping is now keyed on (fk_name, table_name) so same-named constraints on different tables stay separate, while genuine composite FKs on a single table still group together.

Type of change:

• Bug fix

High-level design:

N/A — small, self-contained change in the Snowflake SQLAlchemy dialect override.

Tests:

Use cases covered

• Ingesting a Snowflake schema containing cloned tables whose FK constraints share a name no longer leaks columns between tables, and each table keeps its own foreign key (previously one table's patch failed with 400 Invalid column name and the clone lost its FK entirely).

Unit tests

• Added ingestion/tests/unit/topology/database/test_snowflake_foreign_key_collision.py (4 cases: no cross-table merge, no column leak, composite FK still groups, distinct FKs on one table stay separate).
• Fail on the old code, pass on the fix; existing Snowflake suite (29 tests) still green.

Ingestion integration tests

• Not applicable — covered by unit tests plus live validation below.

Manual testing performed

Validated against a live Snowflake account: reproduced the exact leak (CUSTOMER → ['C_NATIONKEY','S_NATIONKEY'], SUPPLIER → []) with the current release code, then confirmed correct per-table reflection (CUSTOMER → ['C_NATIONKEY'], SUPPLIER → ['S_NATIONKEY']) with the fix, order-independently.

UI screen recording / screenshots:

Not applicable.

Checklist:

• I have read the CONTRIBUTING document.
• I have added tests that cover the exact scenario being fixed.

Greptile Summary

Fixes Snowflake foreign-key reflection collisions by:

• Grouping reflected constraints by both constraint name and table name.
• Adding regression coverage for cloned tables, column isolation, composite foreign keys, and distinct same-table constraints.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failures remain.

Important Files Changed

Filename	Overview
ingestion/src/metadata/ingestion/source/database/snowflake/utils.py	Scopes reflected foreign-key grouping to each table so repeated Snowflake constraint names do not merge columns across tables.
ingestion/tests/unit/topology/database/test_snowflake_foreign_key_collision.py	Adds regression cases covering cross-table constraint-name collisions and preservation of valid grouping behavior.

_{Reviews (2): Last reviewed commit: "Merge branch 'main' into port-of-spain" | Re-trigger Greptile}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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 skip-pr-checks label.

[gitar-bot]

💡 Quality: New test inherits from unittest.TestCase

The new test module test_snowflake_foreign_key_collision.py defines SnowflakeForeignKeyCollisionTest(TestCase) and uses setUp, which violates the project's testing convention to use pytest with plain assert, fixtures, and unittest.mock (no TestCase inheritance). It already uses plain assert, so convert it to a module-level pytest style: replace the class/setUp with a @pytest.fixture that builds the dialect and mock connection, and make the tests plain functions accepting that fixture.

_{Was this helpful? React with 👍 / 👎}

[greptile-apps]

Test uses unittest lifecycle

The new test class inherits from unittest.TestCase and uses setUp, contrary to the repository's pytest fixture convention. This introduces a second lifecycle style and makes fixture composition and parametrization harder when extending these regression cases.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[github-actions]

🔴 Playwright Results — workflow failed

Validated commit 884486f7086646a81b2822331063fecf708632bb in Playwright run 30109093885, attempt 1.

✅ 104 passed · ❌ 2 failed · 🟡 1 flaky · ⏭️ 3 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ❌ unmet · 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) 52m 47s

⏱️ Max setup 3m 3s · max shard execution 14m 28s · max shard-job elapsed before upload 19m 42s · reporting 7s

🌐 204.72 requests/attempt · 1.79 app boots/UI scenario · 0.00% common-shard skew

Optimization targets still in progress:

• Browser traffic was 204.72 requests per attempt (convergence target: fewer than 200).
• Application boot ratio was 1.79 per UI scenario (227 boots / 127 scenarios; convergence target: at most 1).

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky
✅ Shard chromium-01	46	0	0	3	0	0
✅ Shard ingestion-01	28	0	0	0	0	0
🔴 Shard ingestion-02	30	2	1	0	0	0

Genuine Failures (failed on all attempts)

❌ Features/TestSuiteMultiPipeline.spec.ts › TestSuite multi pipeline support (shard ingestion-02)

Error: locator.click: Error: strict mode violation: getByText('Data Observability') resolved to 2 elements:     1) <div class="d-flex justify-between gap-1">Data Observability</div> aka getByTestId('profiler').getByText('Data Observability')     2) <span class="tw:truncate tw:text-xs tw:font-medium tw:text-primary">Data Observability</span> aka getByTestId('asset-health-row-dataObservability').getByText('Data Observability')  Call log: �[2m  - waiting for getByText('Data Observability')�[22m 

❌ Features/TestSuiteMultiPipeline.spec.ts › Edit the pipeline's test case (shard ingestion-02)

Error: locator.click: Error: strict mode violation: getByText('Data Observability') resolved to 2 elements:     1) <div class="d-flex justify-between gap-1">Data Observability</div> aka getByTestId('profiler').getByText('Data Observability')     2) <span class="tw:truncate tw:text-xs tw:font-medium tw:text-primary">Data Observability</span> aka getByTestId('asset-health-row-dataObservability').getByText('Data Observability')  Call log: �[2m  - waiting for getByText('Data Observability')�[22m 

🟡 1 flaky test(s) (passed on retry)

• Features/IncidentManager.spec.ts › Resolving incident & re-run pipeline (shard ingestion-02, 1 retry)

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[gitar-bot]

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Updates the Snowflake SQLAlchemy dialect to group foreign keys by table name and constraint name, preventing cross-table collisions for cloned tables with shared names. Consider modernizing the new test module to avoid inheriting directly from unittest.TestCase.

💡 Quality: New test inherits from unittest.TestCase

📄 ingestion/tests/unit/topology/database/test_snowflake_foreign_key_collision.py:22-36

The new test module test_snowflake_foreign_key_collision.py defines SnowflakeForeignKeyCollisionTest(TestCase) and uses setUp, which violates the project's testing convention to use pytest with plain assert, fixtures, and unittest.mock (no TestCase inheritance). It already uses plain assert, so convert it to a module-level pytest style: replace the class/setUp with a @pytest.fixture that builds the dialect and mock connection, and make the tests plain functions accepting that fixture.

🤖 Prompt for agents

Code Review: Updates the Snowflake SQLAlchemy dialect to group foreign keys by table name and constraint name, preventing cross-table collisions for cloned tables with shared names. Consider modernizing the new test module to avoid inheriting directly from unittest.TestCase.

1. 💡 Quality: New test inherits from unittest.TestCase
   Files: ingestion/tests/unit/topology/database/test_snowflake_foreign_key_collision.py:22-36

   The new test module `test_snowflake_foreign_key_collision.py` defines `SnowflakeForeignKeyCollisionTest(TestCase)` and uses `setUp`, which violates the project's testing convention to use pytest with plain `assert`, fixtures, and `unittest.mock` (no `TestCase` inheritance). It already uses plain `assert`, so convert it to a module-level pytest style: replace the class/`setUp` with a `@pytest.fixture` that builds the dialect and mock connection, and make the tests plain functions accepting that fixture.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}