OpenMetadata/openmetadata-service
Sriharsha Chintalapani 8cec97b52c
Containers: FQN-driven hierarchy listings + cascade-delete orphan fix (#27878)
* Containers: FQN-driven hierarchy listings + cascade-delete orphan fix

Stops `?root=true&service=...` and `/containers/.../children` from leaking
deeply-nested orphans, fixes the source bug that produced them, and corrects
the 1.13.0 fqnHash pattern index opclass.

Listing path
- ListFilter.getFqnPrefixCondition now binds both <param>Hash and
  <param>HashChild ('<hash>.%' and '<hash>.%.%') so depth-aware listings
  can require "exactly one segment below the prefix" via a single LIKE +
  NOT LIKE pair on fqnHash. Same shape works at any tree depth.
- ContainerDAO.listRoot{Before,After,Count} swap the NOT EXISTS anti-join
  on entity_relationship for fqnHash NOT LIKE :serviceHashChild. The FQN
  is the canonical hierarchy in OpenMetadata; the relationship table is
  no longer consulted for hierarchical listings.
- ContainerRepository.listChildren rewritten: no parent-by-name lookup, no
  findToWithOffset/countFindTo on entity_relationship, no second-hop
  hydration. Single SQL roundtrip + slim projection via
  listDirectChildSummariesByParentHash. Orphans whose parent CONTAINS row
  is missing are now correctly placed under their FQN-implied parent.
- Both endpoints honour ?include=non-deleted|all|deleted; ChildrenPageCache
  key includes the include tag so toggling the UI Deleted switch doesn't
  return a stale page from the other side.
- ContainerResource.listChildren accepts ?include= for parity with the
  root listing.

Cascade-delete orphan source (EntityRepository.processDeletionBatch)
- Removed the redundant pre-batch-delete of relationships and the
  swallow-all try/catch in the per-child loop. cleanup() per entity now
  owns row removal AND relationship deletion atomically; exceptions
  propagate so the loop stops on first failure with per-child atomicity.
  Stops the orphan-without-relationships pattern that the listing change
  defends against.

Migration correction (1.13.0 postgres fqnHash pattern indexes)
- Recreate 23 idx_*_fqnhash_pattern indexes with text_pattern_ops instead
  of varchar_pattern_ops. The planner casts the column to text when the
  LIKE RHS is text-typed (every JDBC setString call), so
  varchar_pattern_ops doesn't match the resulting (fqnhash)::text ~~
  expression. Confirmed via EXPLAIN ANALYZE on a 580k-row table: the same
  query drops from ~470ms cold (Parallel Seq Scan) to <1ms (Index Scan).

Tests
- ListFilterTest: 3 unit tests covering both binds, dotted/quoted service
  name special-char handling, and include= flowing through alongside the
  service prefix.
- ContainerResourceIT: 8 integration tests covering depth correctness at
  every level (5-level chain), orphan exclusion at root, orphan
  discoverability under FQN-implied parent, sibling subtree isolation,
  the include toggle on both endpoints, and large-batch hard-delete
  leaving no orphan rows or relationships.

Closes #27870 (subset of its listing-side intent shipped here as a single
FQN-depth predicate; PR's cascade fix and both new tests picked up
verbatim).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Address review comments on #27878

- ContainerDAO.listRoot* override now defaults :serviceHashChild to '%.%.%'
  via rootListingParams() when ?service= is absent. Previous code
  unconditionally referenced the bind, so ?root=true without a service
  filter crashed at runtime with a missing-named-parameter error.
- Migration 1.13.0/postgres/schemaChanges.sql now DROP INDEX CONCURRENTLY
  IF EXISTS before each CREATE so already-upgraded environments (which
  have the original varchar_pattern_ops indexes) get the index recreated
  with text_pattern_ops on next deploy. Fresh installs see the DROP as
  a no-op. Comment block updated to record the recreate intent.
- ChildrenPageCache include tag for ALL changed from "all" to "a" so the
  CacheKeys.childrenPage Javadoc's "1-2 char" promise holds (now nd/a/d
  are all <=2 chars).
- ContainerRepository.includeToBindString Javadoc corrected: it described
  the SQL as a CASE expression, but listDirectChildSummariesByParentHash
  actually uses a three-branch OR chain.
- ListFilterTest: added test_noServiceFilter_doesNotBindServicePatterns
  as a regression guard for the missing-bind bug.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Fix java style

* Address second review pass on #27878

- EntityRepository.processDeletionBatch wraps per-child cleanup exceptions
  with entityType + entityId context before re-throwing. The exception
  still propagates (so the loop still stops, failure-semantics contract
  unchanged); operators now get a stack trace that names the row that
  blocked a large recursive delete instead of an opaque error.
- CacheKeys.childrenPage Javadoc now lists the actual include tags
  ("nd" / "a" / "d") and points at ChildrenPageCache.includeTag as the
  authoritative source. Earlier comment still mentioned "all" after the
  switch to single-letter tags.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Test: ?root=true without service filter end-to-end (#27878 review)

Adds test_rootListing_withoutServiceFilter_returnsRootsAcrossAllServices
to ContainerResourceIT. Creates two distinct storage services, each with
a root container and a child container, then asserts that GET
/containers?root=true (no service filter):

- Succeeds (rootListingParams() defaults :serviceHashChild to '%.%.%' so
  the SQL has its bind even when ListFilter.getServiceCondition didn't
  add it).
- Includes root containers from both services (cross-service listing
  works without a service prefix narrowing the candidate set).
- Excludes child containers from either service (depth check still
  applied via the default bind).

Regression guard for the bug Copilot's review pass flagged at
CollectionDAO.java:784: 'GET /containers?root=true (no service) crashes
at runtime due to a missing named parameter.'

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Use generated name column instead of JSON extract in container summary queries

storage_container_entity has 'name' as a STORED generated column derived
from json->>'name' (see bootstrap/sql/schema/postgres.sql). Both slim
projection queries (findContainerSummaryRows and listDirectChildSummariesByParentHash)
were redundantly extracting it via JSON_UNQUOTE(JSON_EXTRACT(...)) on MySQL
and json->>'name' on Postgres — work the database had already done at insert
time.

Reading 'name' as a column directly:
  - Saves one JSON op per row on every page fetch
  - Lets ORDER BY name sort on the indexed generated column rather than a
    per-row JSON-extracted expression

displayName, fullyQualifiedName, and description stay as JSON extracts —
they aren't generated columns. (description in particular shouldn't be:
free-text fields can be many KB and a STORED generated column would
double the row size on disk.)

Row mapper unchanged — column labels in the SELECT list still match.

* Fix inaccurate ListFilterTest comment and Javadoc link to private method

ListFilterTest: the prefix-pattern comment said the LIKE patterns 'exclude'
direct/grandchildren — patterns themselves match, the SQL's NOT LIKE is
what excludes. Rewrote to show how ContainerDAO.listRoot* combines LIKE
and NOT LIKE on the two binds.

CacheKeys.childrenPage: the @link pointed at ChildrenPageCache#includeTag
which is private static; Javadoc tooling renders that as an unresolved
link. Redirected to the public Include enum the tag is derived from.

* Log original exception in recursive batch delete catch before wrapping

Wrapping the caught RuntimeException into a new one (with entity context
in the message) preserves the original via the cause chain, but the outer
exception mapper sees the wrapper and renders a generic 500 — the original
type information doesn't surface to operators investigating a failed
delete.

Adds a LOG.error before the wrap so the original exception (with full type
and stack) lands in the logs adjacent to the entity context, giving
operators enough signal to diagnose what actually blocked the delete.

* Restore failure-semantics comment block on recursive batch delete wrap

* use Entity.SEPARATOR instead of hard-coding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix check style

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: sonika-shah <58761340+sonika-shah@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-05-04 18:44:42 +05:30
..
src Containers: FQN-driven hierarchy listings + cascade-delete orphan fix (#27878) 2026-05-04 18:44:42 +05:30
.swp Fixes #9259 Change Tags APIs to conform with rest of the APIs (#9260) 2022-12-26 12:32:17 -08:00
LEARNING_RESOURCES_DESIGN.md Learning Resources (#25005) 2026-01-25 07:20:14 -08:00
lombok.config Issue-19251: Upgrade dropwizard to 4.x and Jetty to 11.x (#19252) 2025-05-27 20:31:59 +05:30
pom.xml Chore(deps): Bump org.pac4j:pac4j-core in /openmetadata-service (#27503) 2026-04-20 19:51:56 -07:00