Skip to content

Fixes #31495: key data product ports by entity id, not request index - #32939

Queued
sonika-shah wants to merge 1 commit into
mainfrom
fix/31495-dataproduct-ports-index
Queued

Fixes #31495: key data product ports by entity id, not request index#32939
sonika-shah wants to merge 1 commit into
mainfrom
fix/31495-dataproduct-ports-index

Conversation

@sonika-shah

Copy link
Copy Markdown
Collaborator

Describe your changes:

Fixes #31495

DataProductRepository.getPaginatedPorts (and the sibling bulkAssetsOperation) correlated the entities fetched by Entity.getEntities(...) with the request list by list index. That underlying call resolves to EntityDAO.findByIds, which runs WHERE id IN (...) with no ORDER BY and applies the Include filter (NON_DELETED for the ports view). So the returned rows can arrive in a different order than requested and can be fewer than requested. Zipping them back to the request list by position then attributes each port's data to the wrong id and silently drops the trailing entries — surfacing as an Input/Output Ports badge showing a port while the lineage panel renders empty.

The fix keys each fetched entity by its own id (entity.getId()) instead of the request-list index, so the subsequent order-preserving lookup by relationship id is always correct regardless of how findByIds orders or filters its rows. The same index-zipping existed in bulkAssetsOperation, where it could feed the wrong asset entity into add-time validation; it is fixed identically.


Type of change:

  • Bug fix

High-level design:

N/A — small change.


Tests:

Use cases covered

  • Data product input/output ports keep their own identity and are never misattributed when a port asset is soft-deleted (the NON_DELETED fetch returns fewer rows than the relationship list).

Unit / integration tests

  • Added DataProductResourceIT#test_getPorts_survivingPortsKeepIdentityWhenOneDeleted: creates three input ports, soft-deletes the middle one, and asserts the two surviving ports are returned with their own ids and the deleted port is absent.
  • mvn clean install -pl openmetadata-service,openmetadata-integration-tests -am -DskipTests → BUILD SUCCESS.

UI

  • N/A — backend only.

getPaginatedPorts and bulkAssetsOperation zipped getEntities results to the
request list by index. getEntities -> findByIds runs WHERE id IN (...) with no
ORDER BY and applies the include filter, so it may return fewer rows and in a
different order than requested. Positional zipping then misattributed each
port's data to the wrong id and silently dropped the trailing entries, showing
a non-empty ports badge with an empty lineage panel. Key each fetched entity by
its own id instead.
Copilot AI lite review requested due to automatic review settings September 8, 2026 09:12
@sonika-shah
sonika-shah requested a review from a team as a code owner September 8, 2026 09:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@gitar-bot

gitar-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Fixes data product ports being keyed by request index instead of entity id, which caused misattribution when findByIds returned rows in a different order or filtered out soft-deleted entries. The fix keys fetched entities by their own id, ensuring correct correlation regardless of query result ordering. Includes a new test verifying surviving ports retain their identity when a sibling port is soft-deleted. No issues found.

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 👍 / 👎 | Powered by Gitar — free for open source

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

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

@github-actions github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 5ad803b80d113219401777acf23b69e6aa0a302a in Playwright run 34208680091, attempt 1.

✅ 4475 passed · ❌ 0 failed · 🟡 8 flaky · ⏭️ 1 skipped · 🧰 0 lifecycle flaky

Performance

Blocking 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) 47m 47s

⏱️ Max setup 4m 33s · max shard execution 23m 9s · max shard-job elapsed before upload 26m 47s · reporting 26s

🌐 215.84 requests/attempt · 2.31 app boots/UI scenario · 39.81% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 39.81% (convergence target: at most 15%).
  • Browser traffic was 215.84 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.31 per UI scenario (10941 boots / 4740 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard advanced-search-01 130 0 0 0 0 0
✅ Shard chromium-01 174 0 0 0 0 0
✅ Shard chromium-02 167 0 0 0 0 0
🟡 Shard chromium-03 156 0 1 0 0 0
✅ Shard chromium-04 186 0 0 0 0 0
✅ Shard chromium-05 192 0 0 0 0 0
🟡 Shard chromium-06 151 0 1 0 0 0
🟡 Shard chromium-07 174 0 1 0 0 0
🟡 Shard chromium-08 166 0 1 0 0 0
✅ Shard chromium-09 140 0 0 0 0 0
✅ Shard chromium-10 226 0 0 0 0 0
✅ Shard chromium-11 201 0 0 0 0 0
✅ Shard chromium-12 211 0 0 0 0 0
🟡 Shard chromium-13 183 0 1 1 0 0
✅ Shard chromium-14 167 0 0 0 0 0
✅ Shard chromium-15 215 0 0 0 0 0
🟡 Shard chromium-16 163 0 1 0 0 0
✅ Shard chromium-17 160 0 0 0 0 0
✅ Shard chromium-18 216 0 0 0 0 0
🟡 Shard chromium-19 144 0 1 0 0 0
✅ Shard chromium-20 180 0 0 0 0 0
✅ Shard chromium-21 187 0 0 0 0 0
✅ Shard chromium-22 167 0 0 0 0 0
✅ Shard data-asset-rules-01 65 0 0 0 0 0
✅ Shard domain-isolation-01 16 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard import-export-01 80 0 0 0 0 0
✅ Shard import-export-02 70 0 0 0 0 0
🟡 Shard ingestion-01 38 0 1 0 0 0
✅ Shard ingestion-02 47 0 0 0 0 0
✅ Shard reindex-01 28 0 0 0 0 0
✅ Shard search-01 12 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0
🟡 8 flaky test(s) (passed on retry)
  • Features/Topic.spec.tsCopy field link button should copy the field URL to clipboard (shard chromium-03, 1 retry)
  • Pages/Teams.spec.tsTeams Page Flow (shard chromium-06, 1 retry)
  • Pages/TasksUIFlow.spec.tsCreate and reject tag task for Dashboard via UI (shard chromium-07, 1 retry)
  • Features/OntologyStudioCardinality.spec.tsMANY_TO_ONE relation type should have "M" at source and "1" at target (shard chromium-08, 1 retry)
  • Features/Workflows/RecognizerFeedbackSchemaNodeLock.spec.tsView mode: Apply Feedback (Approved) node name and description inputs are disabled (shard chromium-13, 1 retry)
  • Pages/DataProducts.spec.tsSearch Data Products (shard chromium-16, 1 retry)
  • Flow/CustomizeWidgets.spec.tsKPI Widget (shard chromium-19, 1 retry)
  • Features/TestSuitePipelineRedeploy.spec.tsRe-deploy all test-suite ingestion pipelines (shard ingestion-01, 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

@sonika-shah
sonika-shah added this pull request to the merge queue Sep 8, 2026
Any commits made after this event will not be merged.
@sonika-shah sonika-shah added the To release Will cherry-pick this PR into the release branch label Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data Product portsView returns ports keyed by list index, so rows can be misattributed or silently dropped

3 participants