Repository navigation
[2.0] Fixes #34954: keep the parent of every glossary search match in the hierarchy (#35221, #35284) - #35287
Conversation
…ierarchy (#35221) Hierarchy search (getHierarchy=true) ranks matches and their parent terms in one page. With more matches than the page holds, the parents rank below the cut and buildGlossaryTermSearchHierarchy drops every match whose parent is missing, so the glossary comes back with no children. - Fetch the missing parents of the page's matches in one exact-FQN lookup after the cut (Approved only, same as the search). On failure, log and fall back to the previous behaviour. - Score the parent-term clauses 0 for glossary terms, so parents never take a page slot from a real match. Domain search is unchanged. Same change in the Elasticsearch and OpenSearch managers, plus an IT that fails without it. (cherry picked from commit cda0297)
…her tests The hierarchy search IT waited for "search(word) total == 30" over the whole shared index. Glossary search matches fuzzily and on name n-grams, so other tests' terms counted too and it failed in the parallel lane with 33. Its page and child-count checks had the same exposure, and it never cleaned up its terms. - No wait or global count: entity create indexes synchronously with a refresh, so the terms are searchable when create returns. - Every search is post-filtered to the test's own glossary. - The search word uses letters absent from all generated names, so parents can never match it or take a page slot from a match. - The glossary is hard-deleted recursively in finally. - Adds the same check for matches nested four levels deep. (cherry picked from commit 72b7051)
…erarchy ITs The hierarchy search ITs depend on the shared glossary-term search settings, so take SEARCH_SETTINGS in READ mode: they never overlap a test that rewrites the settings under the READ_WRITE lock, and still run alongside other readers. A failed cleanup delete no longer fails a test whose assertions passed; the glossary is namespace-scoped. (cherry picked from commit 9bcc2ce)
❌ PR checklist incompleteThis 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 |
github-actions
Bot
added
backend
safe to test
Code Review ✅ Approved🟡 Medium risk · Changing glossary-term search scoring and fetching missing parents could change page results or leave hierarchy paths incomplete if the lookup fails. Backports the glossary hierarchy search fix so every matching term keeps its parent in the hierarchy, even when parents rank below the page cut. Missing parents are fetched in one exact-FQN lookup and parent clauses score 0 for glossary terms, with tests made independent of other test data. No issues found. VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledOptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
This branch was successfully deployed
2 participants
Describe your changes:
Fixes #34954
Backport to
2.0of:Bug: glossary term search with
getHierarchy=true(the glossary term picker) can return a glossary with no terms under it. With more matches than the page holds, parent terms rank below the cut, and the hierarchy builder drops every match whose parent is missing.What changed:
ElasticSearchSearchManager,OpenSearchSearchManager): missing parents of the page's matches are fetched after the cut in one exact-FQN lookup (Approved only; logs and falls back on failure), and parent clauses score 0 for glossary terms so they never take a page slot from a match. Domain search unchanged.GlossaryTermResourceIT): one level and four levels of non-matching parents over 30 matches, page of 25. No waits and no counts over shared data; every search is post-filtered to the test's own glossary; the search word shares no letters with generated names;SEARCH_SETTINGSread lock; best-effort recursive cleanup.Three commits, cherry-picked with
-x; all applied cleanly (the #35221 commit with identical +228/−6).Type of change:
Tests:
GlossaryTermResourceITclass (concurrent) on 2.0: MySQL + Elasticsearch 298 run, 0 failures; Postgres + OpenSearch 298 run, 0 failures.openmetadata-service+openmetadata-integration-testson 2.0.Why the tests don't depend on other tests (verified on 2.0)
post_filteron the test's glossary, applied before the top-N cutSearchIndexHandler.isAsync()is false) withRefresh.True; nothing to wait forAPPROVEDwhen the glossary/parent has no reviewers (GlossaryTermRepository); the test glossary has nonejknoqstuvwxyz) never occur in the hex/hier_*names, so no n-gram can match; parents score 0@ResourceLock(SEARCH_SETTINGS, READ); writers holdREAD_WRITEfinally, best-effort