Fixes #30678: propagate inherited domain to descendants in search on asset move

[sonika-shah]

Fixes #30678

What

When an asset's domain changes via an asset-move route, descendants that inherit their domain updated on the entity page but not in the search index, so Explore, domain filters and asset counts kept showing them under the old domain until a reindex.

Two routes were affected:

• Data Product domain change — DataProductRepository.updateDataProductDomains reindexed only the directly-attached assets (updateAssetDomainsByIds, an ids-query).
• Domain page asset move — DomainRepository.bulkAssetsOperation calls SearchRepository.updateEntity(ref), which nulls the change description, so requiresPropagation is false and the child fan-out never runs.

A normal single-entity domain PATCH already fans the change out to inherited descendants in search via propagateInheritedFieldsToChildren. The two move routes bypassed that step; the DB side stayed correct only because inherited domains are recomputed at read time (EntityRepository.inheritDomains), while the search index stores a materialized domains array that needs an explicit push.

Fix

New SearchRepository.propagateInheritedDomainsToChildren(entityType, assetId, newDomains) that pushes the moved asset's new domains onto its children in search, wired into both move routes.

It reuses the existing ADD_DOMAINS_SCRIPT, which overwrites a child's domains only when that child's domain is empty or inherited — so descendants that carry an explicit domain are left untouched, exactly as today. Child matching and service-vs-entity keying reuse the existing filterChildAliasesByCapability / resolveParentFieldName helpers, so multi-level hierarchies (database → schema → table) are covered through the denormalized parent-id fields, the same way normal-PATCH propagation already works.

Behaviour (per the issue)

For a schema asset in Finance moved to HR:

entity	before	after (search)
schema (direct asset)	HR	HR
child inheriting its domain	Finance (stale)	HR
child with explicit Finance	Finance	Finance (unchanged)
child with explicit Marketing	Marketing	Marketing (unchanged)

Tests

Integration test DataProductResourceIT#test_changeDataProductDomain_propagatesInheritedDomainToChildTablesInSearch: assigns a schema (with inheriting + explicit-domain child tables) as a data product asset, moves the data product's domain, and asserts each child table's domains in the search index — inheriting child follows, explicit-domain children stay put. Fails without the fix.

Out of scope

This is issue #1 of #30676 (targeted for an early release). The remaining items — warning when a moved asset's own explicit domain is overwritten, and the conflicting-data-product-assignment cleanup (#30679) — are separate.

---

Also fixes #31162 — over-append to explicit-domain descendants on a domain change

While validating the above, a related search-propagation bug surfaced on the normal entity-update path (changing an entity's own domain, e.g. a schema's domain from its entity page): the parent's new domain was appended (as inherited) to descendants that already carry their own explicit domain — a search-only divergence (the entity/DB stays correct).

Root cause: a domain (or owner) change is recorded as delete-old + add-new, so child propagation runs generateRemoveListScript (from the delete) then generateAddListScript (from the add). The add script is guarded and skips explicit children, but the remove script did removeIf(inherited); addAll(removed<Field>) unconditionally, and removed<Field> resolves to the parent's new domains marked inherited — appending them onto explicit children. Fix: only re-add when no explicit (non-inherited) value remains after removing inherited refs, mirroring the add script. Inherited descendants still follow the change; explicit ones keep their own domain. The shared path corrects the same latent behaviour for owners and dataProducts.

Regression test: DomainResourceIT#test_entityDomainChange_doesNotOverAppendToExplicitDescendantInSearch.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[github-actions]

❌ PR checklist incomplete

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

• Issue Domain change: inherited descendants not propagated to the search index #30678 is missing Shipping → Source.
• Issue Domain change: inherited descendants not propagated to the search index #30678 is missing Shipping → Priority.
• Linked issue Domain change on a parent over-appends the new domain to explicit-domain descendants in the search index (entity page correct) #31162 is not added to the Shipping project.

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.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java:2576

• propagateInheritedDomainsToChildren uses the raw searchClient field and does not handle the “client unavailable” path. Both ES/OS clients’ updateChildren(...) short-circuit when unavailable (no IOException), so this propagation can be silently dropped without enqueueing to SearchIndexRetryQueue (unlike updateAssetDomainsByIds, updateDomainFqnByPrefix, etc., which explicitly enqueue when the client is unavailable). Using getSearchClient() and enqueueing on unavailable keeps behavior consistent and makes the move recoverable via retries.

    deferIfFlushScopeActive(
        () -> {
          try {
            searchClient.updateChildren(childAliases, parentMatch, updates);
          } catch (IOException e) {
            SearchIndexRetryQueue.enqueue(

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[sonika-shah]

Correct — clearing descendants on remove is wrong, since after detaching a domain the asset (and its descendants) may re-inherit from its own parent rather than go empty. Restricted the propagation to the add/move path (isAdd && !dryRun), which is the route the stale-descendant bug actually occurs on (a Domain-page move comes through as an add after cleanupOldDomain). The remove path keeps its prior behavior (no child propagation) — a correct remove-time re-inheritance would be a separate change.

[gitar-bot]

💡 Quality: Negative assertion on child domain not wrapped in Awaitility

In test_domainAssetRemove_doesNotClearReinheritingDescendantInSearch, the final check that the inheriting child was NOT cleared (lines 1641-1646) runs once, immediately after the schema re-inherit await. Because remove-propagation to descendants is asynchronous, a regression that erroneously clears the child's domain could land after this snapshot, letting the test pass while the bug exists — a weak negative assertion. Consider re-checking the child inside a short Awaitility poll (or a fixed settle delay after the schema converges) so the assertion holds across the full propagation window.

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

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (3)

openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DomainResourceIT.java:1597

• This test name suggests it verifies that the descendant re-inherits the correct domain in search after removal, but the assertion only checks that the descendant domains field is non-empty. Renaming the test to match what it actually asserts will avoid misleading future readers.

  void test_domainAssetRemove_doesNotClearReinheritingDescendantInSearch(TestNamespace ns)

docker/development/docker-compose.omfix.yml:4

• This override compose file relies on the custom !override YAML tag. If a developer’s Docker Compose version doesn’t support this tag/merge behavior, the file may not work as intended (ports/volumes may merge instead of replacing, or parsing may fail). Consider adding a brief note about the required Docker Compose capability/version in the header comment, or provide an alternative override approach for older compose versions.

# `!override` replaces the base list instead of merging (compose merges by default).

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DomainRepository.java:388

• domainRef is fetched even for dry-run requests, but it is only used on the non-dry-run add/move path (relationship write + lineage + search propagation). This adds an unnecessary DB read to dry-run validation requests.

    EntityReference domainRef = isAdd ? getEntityReferenceById(DOMAIN, entityId, ALL) : null;

[gitar-bot]

Code Review 👍 Approved with suggestions 3 resolved / 4 findings

Propagates inherited domains to descendants in the search index during asset moves, addressing the missing search propagation finding. Consider wrapping the negative assertion in Awaitility for the domain removal test.

💡 Quality: Negative assertion on child domain not wrapped in Awaitility

📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DomainResourceIT.java:1638-1646

In test_domainAssetRemove_doesNotClearReinheritingDescendantInSearch, the final check that the inheriting child was NOT cleared (lines 1641-1646) runs once, immediately after the schema re-inherit await. Because remove-propagation to descendants is asynchronous, a regression that erroneously clears the child's domain could land after this snapshot, letting the test pass while the bug exists — a weak negative assertion. Consider re-checking the child inside a short Awaitility poll (or a fixed settle delay after the schema converges) so the assertion holds across the full propagation window.

✅ 3 resolved

✅ Bug: Child domain propagation bypasses search-write deferral in @transaction

📄 openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java:2524-2538 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DomainRepository.java:366 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DomainRepository.java:416-418 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DataProductRepository.java:1028-1032
Both call sites run inside a @Transaction (DomainRepository.bulkAssetsOperation is annotated; DataProductRepository.updateDataProductDomains runs in the update flush). The neighbouring ES writes — updateEntity(ref) and updateAssetDomainsByIds(...) — go through the search-write deferral scope (deferSearchWrite/deferIfFlushScopeActive), so their blocking Elasticsearch round trips are captured and drained only after the DB commit, per the mechanism documented at SearchRepository.java:179-191. The new propagateInheritedDomainsToChildren instead calls searchClient.updateChildren(...) inline. This issues a blocking ES call while a pooled DB connection is held (the exact deadlock the deferral scope was built to avoid), and mutates child docs before commit, so a transaction rollback leaves the search index showing the new domain on children while the DB is reverted. Route the write through deferIfFlushScopeActive like the sibling calls.

✅ Quality: Retry enqueue drops known entityType for failed child propagation

📄 openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java:2558-2561
On IOException the code calls the 3-arg SearchIndexRetryQueue.enqueue(entityId, null, failureReason) overload, which sets entityType to "" (SearchIndexRetryQueue.java:70-72). The moved asset's entityType is available in propagateInheritedDomainsToChildren but is discarded, forcing the retry worker's resolveEntityReference to fall back to a hint-less id scan (SearchIndexRetryWorker resolveById). Pass entityType via the 4-arg enqueue overload so resolution uses the type hint. Also note the retry re-indexes the asset itself rather than re-running child propagation; confirm reindexEntityCascade covers descendants, otherwise the retry will not actually repair stale children.

✅ Edge Case: Batched terms query may hit index.max_terms_count on large moves

📄 openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java:2544-2557 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchEntityManager.java:556-570
propagateInheritedDomainsForType now issues a single update-by-query whose terms query enumerates every moved parent id (anyOfFieldQuery in ElasticSearchEntityManager/OpenSearchEntityManager). A very large bulk domain move or a data product with a large asset set could exceed Elasticsearch's default index.max_terms_count (65536), causing the update-by-query to fail and fall back to the retry queue. This is unlikely in practice but consider chunking parentIds if bulk moves can be large.

🤖 Prompt for agents

Code Review: Propagates inherited domains to descendants in the search index during asset moves, addressing the missing search propagation finding. Consider wrapping the negative assertion in Awaitility for the domain removal test.

1. 💡 Quality: Negative assertion on child domain not wrapped in Awaitility
   Files: openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DomainResourceIT.java:1638-1646

   In test_domainAssetRemove_doesNotClearReinheritingDescendantInSearch, the final check that the inheriting child was NOT cleared (lines 1641-1646) runs once, immediately after the schema re-inherit await. Because remove-propagation to descendants is asynchronous, a regression that erroneously clears the child's domain could land after this snapshot, letting the test pass while the bug exists — a weak negative assertion. Consider re-checking the child inside a short Awaitility poll (or a fixed settle delay after the schema converges) so the assertion holds across the full propagation window.

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}

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.