Containers: batch container data-model column tag retrieval to avoid subtree fan-out

[harshach]

Summary

Container retrieval was running a depth-unbounded LIKE 'hashPrefix.%' against tag_usage to fetch tags for a container's data-model columns. Because Containers can contain other
Containers indefinitely (S3/ADLS/GCS folder hierarchies), this prefix match returned the entire descendant subtree of every nested container, then filtered in memory by exact hash match.
For a instance with ~500K assets under one bucket, a single top-level container GET matched ~595K rows out of 7.4M.

This PR replaces the prefix LIKE with a batched exact-match IN over the columns' hashes — Container-only, since it's the only entity type with unbounded child nesting.

Background

Companion to #27745 / #27824, which restored the partial-index match by adding back the WHERE state = 1 predicate. Those PRs unblock the seq scan; this PR removes the unnecessary fan-out
so the indexed seek doesn't return 595K rows we don't need.

Why Container only

Surveyed all 8 callers of Entity.populateEntityFieldTags (Table, Container, File, Topic, DashboardDataModel, APIEndpoint, Worksheet, TestCase). Only Container's children can themselves
nest — the rest have schema-bounded child sets where the prefix LIKE is fine. No shared-helper change, no blast radius on the other 7 entity types.

Changes

CollectionDAO.TagUsageDAO

• Added getTagsInternalByTargetHashes — same SQL as getTagsInternalByPrefix (with the tag / glossary_term_entity JSON join) but WHERE targetFQNHash IN () instead of LIKE.
• Added default Map<String, List> getTagsByTargetFQNHashes(List) — chunks input at 1000 hashes per query, returns a hash → tags map.

ContainerRepository

• Replaced both populateEntityFieldTags call sites (single-entity GET in setFields, PII path in getSampleData) with a private populateDataModelColumnTags(setTags, columns) that flattens
  columns (including struct children), hashes their FQNs, and calls the new batched DAO.
• Removed the now-unused populateEntityFieldTags import and the unused fqnPrefix parameter.

Untouched:

• Entity.populateEntityFieldTags (still used by Table, Topic, Dashboard, APIEndpoint, Worksheet, File, TestCase — fine for them).
• bulkPopulateEntityFieldTags (LIST path) — already uses chunked IN-list batching via getTagsInternalBatch.

Behavior

Same observable semantics (derived-tag wrapping, setTags=false skip, struct-child flattening). Difference is purely in how the data is fetched:

┌───────────────────────────────────────┬───────────────────────────────────────────────┬─────────────────────────────────────────────────┐
│ │ Before │ After │
├───────────────────────────────────────┼───────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ Query │ WHERE targetfqnhash_lower LIKE 'hash.%' │ WHERE targetFQNHash IN (...) │
├───────────────────────────────────────┼───────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ Match scope │ Entire descendant subtree │ Exact column FQNs only │
├───────────────────────────────────────┼───────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ Rows fetched on a top-level container │ All tags below it (595K in the reported case) │ Tag count of the data model's flattened columns │
└───────────────────────────────────────┴───────────────────────────────────────────────┴─────────────────────────────────────────────────┘

Tests

Added to ContainerResourceIT:

1. get_parentDataModelTags_doesNotLeakChildContainerTags — creates a parent container with tagged columns plus a tagged child container nested under it, GETs the parent, asserts (a)
   parent's column tags are correct and (b) the child container's tag does not leak into any of the parent's columns. Regression test for the over-fetch.
2. get_dataModelStructColumnTags_areReturned — tags a column nested inside a STRUCT, verifies the batched fetch returns it through the flattened-children traversal.

Verification

• ✅ mvn -pl openmetadata-service install -DskipTests — clean
• ✅ Both new IT tests pass individually
• ✅ All 30 ContainerResourceIT#get_* tests pass (existing + new)
• ✅ test_sampleData* + patch_dataModelColumnTags* pass (covers the PII call site)

---

Summary by Gitar

• SDK Refactoring:
  • Updated ContainerService to use the base class method buildPathWithEncodedName instead of a manual URLEncoder implementation.
  • Promoted buildPathWithEncodedName in EntityServiceBase to protected to enable access by subclasses.

_{This will update automatically on new commits.}

[Copilot]

Pull request overview

This PR improves how Container dataModel column tags are fetched to address performance problems and incorrect tag leakage in deeply nested container hierarchies. It replaces prefix-based tag retrieval with an exact-match (by target FQN hash) batched lookup, and adds integration tests to validate correct tag isolation and nested-struct tag handling.

Changes:

• Update ContainerRepository to fetch dataModel column tags via flattened columns + exact target FQN hash lookups.
• Add TagUsageDAO query/helper to retrieve tag usages by a list of target hashes with chunking.
• Add integration tests to ensure parent containers don’t receive child container column tags and that nested struct child column tags are returned.

Reviewed changes

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

File	Description
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContainerRepository.java	Switch column-tag population from prefix scanning to flattened-column exact hash batching.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java	Add TagUsageDAO query + chunked helper for fetching tags by target FQN hashes.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ContainerResourceIT.java	Add regression tests for tag leakage across container hierarchy and nested-struct column tag retrieval.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

[github-actions]

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

openmetadata-ui/src/main/resources/ui/src/components/DataAssets/DataAssetsHeader/DataAssetsHeader.component.tsx:355

• fetchContainerAncestors does not guard against stale in-flight requests. If the user navigates quickly between containers (or an earlier request resolves after a later navigation), the older response can call setParentContainers(...) and overwrite the newer container’s breadcrumbs.

Consider tracking the latest requested FQN in a useRef (or using an AbortController) and only applying ancestors/setIsBreadcrumbLoading(false) when the resolving request still matches the current dataAsset.fullyQualifiedName.

  const fetchContainerAncestors = async (containerFqn: string) => {
    // Always reset state at the top so a navigation to a container without an FQN
    // (or to one whose ancestor fetch fails) doesn't keep painting the previous
    // container's breadcrumbs. Without this reset, switching between containers
    // could leave stale ancestors visible after either an early return or an error.
    setParentContainers([]);
    if (isEmpty(containerFqn)) {
      return;
    }
    setIsBreadcrumbLoading(true);
    try {
      // Single batched server call replaces what used to be one
      // getContainerByName per ancestor level.
      const ancestors = await getContainerAncestors(containerFqn);
      setParentContainers(ancestors ?? []);
    } catch (error) {
      showErrorToast(error as AxiosError, t('server.unexpected-response'));
    } finally {
      setIsBreadcrumbLoading(false);

[Copilot]

With the dedicated org.openmetadata.slowrequest logger removed from the sample config, the RequestLatencyContext slow-request logs (topic org.openmetadata.slowrequest) will now inherit the root logger settings and flow into the main appenders by default. Combined with the switch to LOG.info(...), this can significantly increase log volume and also removes the previous ability to toggle/route these logs separately.

If the intent is to keep slow-request logging optional or isolated, consider re-adding an explicit org.openmetadata.slowrequest logger entry here (potentially with its own appender/level) or documenting the new recommended override mechanism for operators.

[Copilot]

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.

[gitar-bot]

Code Review ✅ Approved 11 resolved / 11 findings

Optimizes container tag retrieval by replacing unbounded prefix lookups with a batched exact-match strategy for column FQN hashes. All identified issues, including redundant hash computation, authorization gaps, and duplicate tag processing logic, have been resolved.

✅ 11 resolved

✅ Performance: FQN hash computed twice per column in populateDataModelColumnTags

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContainerRepository.java:259-268
In populateDataModelColumnTags, FullyQualifiedName.buildHash(c.getFullyQualifiedName()) is called once when building the targetHashes list (line 261) and again when looking up each column in the result map (line 267). For large data models this doubles the hashing work unnecessarily. Pre-computing a Map<Column, String> or Map<String, Column> would avoid the redundant computation.

✅ Bug: Bulk-fetched children skip setFields/setDefaultFields hydration

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContainerRepository.java:565-579 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContainerRepository.java:84-96 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContainerRepository.java:326-330
The old code called Entity.getEntity(ref, EMPTY_FIELDS, ALL) for each child, which routes through EntityRepository.get() → setFields(). Even with empty fields, setFields unconditionally calls setDefaultFields(container) (line 87), which populates the service reference via a relationship lookup (line 326-329).

The new code at line 567 uses dao.findEntitiesByIds(ids, Include.ALL), which only deserializes the stored JSON blob and never calls setFields or setDefaultFields. As a result, every child container returned by listChildren will be missing its service EntityReference (and any other relationship-based fields that setDefaultFields populates).

This is an observable API regression — callers of the listChildren endpoint will receive child containers with a null service field.

✅ Security: listAncestors endpoint has no authorization check

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/storages/ContainerResource.java:780-786
The new listAncestors endpoint at line 780 bypasses the base-class getInternal/listInternal machinery and directly calls repository.getAncestors(fqn) with no authorization gate. While listChildren follows the same pattern (also no auth), the ancestors endpoint is new code being added now. An unauthenticated caller could enumerate the full container hierarchy for any FQN.

Consider adding an OperationContext + authorizer.authorize() check, or routing through the base-class helper that enforces VIEW_BASIC permissions, similar to how get() delegates to getInternal().

✅ Quality: getAncestors returns mutable lists instead of unmodifiable

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContainerRepository.java:603 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContainerRepository.java:628
Both the early-return at line 603 (new ArrayList<>()) and the normal return at line 628 return mutable ArrayList instances. Per project conventions, repository methods should return unmodifiable collections to prevent accidental mutation by callers.

✅ Quality: EntityReferenceRow should be a Java 21 record

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityDAO.java:263-277
The new EntityReferenceRow inner class (lines 263-288) is a plain data carrier with public final fields and a constructor — the exact use-case for a Java record. Per the project's Java 21 guidelines, this should be record EntityReferenceRow(UUID id, String name, String displayName, String fqn, boolean deleted) with the toEntityReference method kept as an instance method on the record. This would eliminate ~15 lines of boilerplate and align with the existing EntityNameColumnHashJsonPair and EntityIdJsonPair records already declared in the same file (lines 836, 845).

...and 6 more resolved from earlier reviews

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

openmetadata-service/src/main/resources/logback.xml:27

• RequestLatencyContext logs to the org.openmetadata.slowrequest logger topic. With the dedicated logger removed here, slow-request diagnostics will inherit the root logger configuration (STDOUT + main file) and can no longer be routed to a separate file / disabled via a dedicated logger level. If this was not intentional, consider re-adding an explicit org.openmetadata.slowrequest logger configuration (level + optional file appender) so operators can control noise and retention for slow-request logs independently of the main application logs.

    <logger name="org.openmetadata.service.Entity" level="ERROR" />
    <!-- Add specific logger for SearchIndexApp and ReindexingJobLogger -->
    <logger name="org.openmetadata.service.apps.bundles.searchIndex" level="INFO" />
    <root level="${LOG_LEVEL:-INFO}">
        <appender-ref ref="STDOUT" />
        <appender-ref ref="file"/>
    </root>

openmetadata-sdk/src/main/java/org/openmetadata/sdk/services/storages/ContainerService.java:79

• encodeFqn uses URLEncoder.encode(...) directly for a URL path segment. URLEncoder is intended for query-string encoding and will encode spaces as +, which is not a valid path encoding and is inconsistent with other SDK services (e.g. PipelineService replaces + with %20). This can break requests for FQNs containing spaces (or other characters that need path encoding).

Consider switching to a shared path-segment encoder (or at minimum .replace("+", "%20")) to keep SDK URL construction correct and consistent across services.

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
1059 New issues
0 Accepted issues

Measures
17 Security Hotspots
0.0% Coverage on New Code
3.8% Duplication on New Code

See analysis details on SonarQube Cloud

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
41.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud