Fixes #28229: cascade Table certification PATCH to child search docs#28236
Merged
Conversation
When a Table's certification is added, changed, or removed via PATCH the existing cascadeCertificationToChildren path never fired because SearchRepository.requiresPropagation returned false on a cert-only ChangeDescription. TableRepository did not list certification in its propagation descriptors, so the gate stayed closed and the DQ dashboard's Certification filter kept returning the stale cert on test_case / test_case_result / test_case_resolution_status / test_suite / column docs until a full reindex. Add an EXTERNAL_HANDLER PropagationType for fields whose cascade is driven by a dedicated SearchRepository handler instead of the generic descriptor-driven script (cert needs full-object replace on add/update and explicit removal on delete, which RAW_REPLACE can't express because it restores the old value on delete). Register certification with this type on TableRepository so the gate opens and the existing cascadeCertificationToChildren method runs. Add no-op cases in the three appendAdd/Update/DeleteScript switches so the new enum value doesn't accidentally trigger generic auto-propagation. Adds three unit tests in SearchRepositoryBehaviorTest covering cert add/update/remove on a Table opening the propagation gate, and a new TableCertificationPropagationIT that creates a Table+TestCase, PATCHes the Table cert Gold->Silver->null, and asserts the test_case search doc cascades each transition. The IT fails on main and passes with this fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
backend
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the propagation gate for Table certification changes so certification updates performed via PATCH are cascaded to child search documents that denormalize the parent Table’s certification (e.g., test_case, test_case_result, test_case_resolution_status, test_suite, column).
Changes:
- Introduces
PropagationType.EXTERNAL_HANDLERto allowrequiresPropagation(...)to open the propagation gate for fields whose cascade is handled by dedicatedSearchRepositorylogic (not descriptor-driven scripts). - Registers Table
certificationasEXTERNAL_HANDLERinTableRepository.getSearchPropagationDescriptors()and adds no-op handling for this propagation type in script builders. - Adds unit + integration tests covering Table certification add/update/remove propagation to child search docs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java | Updates mock propagation descriptors and adds regression tests ensuring requiresPropagation returns true for cert-only Table changes. |
| openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java | Adds explicit no-op switch cases for EXTERNAL_HANDLER so descriptor-driven scripts skip fields handled by dedicated cascade logic. |
| openmetadata-service/src/main/java/org/openmetadata/service/search/PropagationDescriptor.java | Adds the new PropagationType.EXTERNAL_HANDLER enum value with rationale. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java | Registers FIELD_CERTIFICATION for search propagation gating using EXTERNAL_HANDLER. |
| openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TableCertificationPropagationIT.java | New end-to-end IT verifying Table certification PATCH cascades to test_case_search_index. |
Comment on lines
+57
to
+59
| @Execution(ExecutionMode.SAME_THREAD) | ||
| @TestInstance(TestInstance.Lifecycle.PER_CLASS) | ||
| public class TableCertificationPropagationIT { |
The new IT used SAME_THREAD execution mode but project convention is CONCURRENT. Entity names are timestamped so concurrent execution is safe, and the cert cascade is scoped by table.id so it won't collide with other tests' children. Verified the IT still passes under CONCURRENT. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code Review ✅ Approved 1 resolved / 1 findingsCascades Table certification PATCH updates to child search documents by registering certification with a custom propagation handler. The IT execution mode was updated to CONCURRENT to match project standards. ✅ 1 resolved✅ Quality: IT uses SAME_THREAD but project standard is CONCURRENT
OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Contributor
🟡 Playwright Results — all passed (11 flaky)✅ 4141 passed · ❌ 0 failed · 🟡 11 flaky · ⏭️ 86 skipped
🟡 11 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
|
mohityadav766
approved these changes
May 19, 2026
Contributor
|
Changes have been cherry-picked to the 1.13 branch. |
github-actions Bot
pushed a commit
that referenced
this pull request
May 19, 2026
…28236) * Fixes #28229: cascade Table certification PATCH to child search docs When a Table's certification is added, changed, or removed via PATCH the existing cascadeCertificationToChildren path never fired because SearchRepository.requiresPropagation returned false on a cert-only ChangeDescription. TableRepository did not list certification in its propagation descriptors, so the gate stayed closed and the DQ dashboard's Certification filter kept returning the stale cert on test_case / test_case_result / test_case_resolution_status / test_suite / column docs until a full reindex. Add an EXTERNAL_HANDLER PropagationType for fields whose cascade is driven by a dedicated SearchRepository handler instead of the generic descriptor-driven script (cert needs full-object replace on add/update and explicit removal on delete, which RAW_REPLACE can't express because it restores the old value on delete). Register certification with this type on TableRepository so the gate opens and the existing cascadeCertificationToChildren method runs. Add no-op cases in the three appendAdd/Update/DeleteScript switches so the new enum value doesn't accidentally trigger generic auto-propagation. (cherry picked from commit d6cec7e)
Contributor
|
Failed to cherry-pick changes to the 1.12.9 branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes:
Fixes #28229
When a Table's certification is added, changed, or removed via PATCH, the existing
cascadeCertificationToChildrenpath never fires becauseSearchRepository.requiresPropagationreturnsfalseon a cert-onlyChangeDescription—TableRepository.getSearchPropagationDescriptors()never listedcertification. The DQ dashboard's Certification filter therefore kept returning the stale cert ontest_case,test_case_result,test_case_resolution_status,test_suite, andcolumndocs until a full reindex.This PR adds an
EXTERNAL_HANDLERPropagationTypefor fields whose cascade is driven by a dedicatedSearchRepositoryhandler rather than the generic descriptor-driven script (cert needs full-object replace on add/update and explicit removal on delete, whichRAW_REPLACEcan't express — it restores the old value on delete), registerscertificationwith this type onTableRepository, and adds no-op cases in the threeappendAdd/Update/DeleteScriptswitches.Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
test_casesearch doc inherits the cert (already worked at index time).test_casesearch doc reflects Silver within the await window.test_casesearch doc loses the cert.Unit tests
openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java— three new tests (requiresPropagationReturnsTrueForTableCertification{Update,Added,Removed}) and updated mock descriptors to mirror production.Backend integration tests
openmetadata-integration-tests/for the cascade behavior.openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TableCertificationPropagationIT.java. The IT fails onmain(expected: <Certification.Silver> but was: <Certification.Gold>) and passes after this fix.Ingestion integration tests
Playwright (UI) tests
Manual testing performed
Ran
mvn test -pl openmetadata-service -Dtest=SearchRepositoryBehaviorTest(108/108 pass) andmvn test -pl openmetadata-integration-tests -Dtest=TableCertificationPropagationIT(passes after fix; fails onmainHEAD).UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.