Skip to content

fix(bulk-ops): exclude soft-deleted entities from column grid (#28653) [1.13] - #30433

Draft
sonika-shah wants to merge 2 commits into
1.13from
cherry-pick-28653-to-1.13
Draft

fix(bulk-ops): exclude soft-deleted entities from column grid (#28653) [1.13]#30433
sonika-shah wants to merge 2 commits into
1.13from
cherry-pick-28653-to-1.13

Conversation

@sonika-shah

@sonika-shah sonika-shah commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Cherry-pick of #28653 into 1.13.

The original fix merged to main and was picked into the 1.12.x line, but 1.12.11 was cut after 1.13.0 branched out — so 1.13 never received it. Confirmed the deleted:false filter was absent from both the ES and OS ColumnAggregator on 1.13.

Fixes #28437 for the 1.13 line.

Cherry-picked commit: 1079291 (clean, no conflicts).

Changes

  • ElasticSearchColumnAggregator.javadeleted:false filter in buildFilters() and buildTagFilterQuery()
  • OpenSearchColumnAggregator.java — same
  • ColumnBulkUpdateIT.java — regression test test_columnGrid_excludesSoftDeletedTables

Greptile Summary

Adds consistent active-entity filtering to the column grid.

  • Excludes soft-deleted parent entities from Elasticsearch column queries.
  • Applies the same filtering to OpenSearch queries.
  • Adds an integration regression test covering table soft deletion.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failures remain.

Important Files Changed

Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchColumnAggregator.java Adds deleted:false filters to Elasticsearch column aggregation and tag-filter queries.
openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchColumnAggregator.java Adds equivalent deleted:false filters to OpenSearch column aggregation and tag-filter queries.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ColumnBulkUpdateIT.java Adds an integration test verifying that columns disappear from the grid after their table is soft-deleted.

Reviews (2): Last reviewed commit: "Merge branch '1.13' into cherry-pick-286..." | Re-trigger Greptile

* fix(bulk-ops): exclude soft-deleted entities from column grid search

The /v1/columns/grid endpoint was missing a `deleted: false` term filter
in its Elasticsearch/OpenSearch query, so soft-deleted table columns
appeared in the Column Bulk Operations grid.

Both query-building methods in ElasticSearchColumnAggregator and
OpenSearchColumnAggregator (buildFilters and buildTagFilterQuery)
now include the filter, matching the convention used by /v1/search/query.

Adds a regression test that seeds a soft-deleted table with a column,
confirms the column disappears from the grid after soft-deletion.

Fixes #28437

* fix(bulk-ops): address review comments on soft-delete column grid fix

- Use FieldValue.of(false) in OpenSearch aggregator to match the
  FieldValue-based term API used throughout OsUtils
- Add assertNotNull guards on getColumns() before streaming in the
  regression test to produce clear failures instead of NPEs

* test(bulk-ops): add soft-delete regression test to enabled IT class

Move the regression test for issue #28437 into ColumnSearchIndexIT
(which is not @disabled) so it runs in CI immediately, rather than
waiting for the unrelated metadataStatus aggregation flake in
ColumnGridResourceIT to be resolved.

* Revert "test(bulk-ops): add soft-delete regression test to enabled IT class"

This reverts commit 7081832.

* test(bulk-ops): add soft-delete regression test to ColumnBulkUpdateIT

Add the regression test for issue #28437 to the enabled ColumnBulkUpdateIT
class so it runs in CI, rather than ColumnGridResourceIT which is
currently @disabled for an unrelated aggregation flake.

* test(bulk-ops): remove duplicate regression test from @disabled class

Test already lives in ColumnBulkUpdateIT where it runs in CI.

* test(bulk-ops): remove redundant sleeps, rely on Awaitility polling

The Awaitility blocks already poll for eventual consistency; the
preceding TimeUnit.SECONDS.sleep(3) calls only added fixed delay and
violated the no-Thread.sleep test convention.
@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

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

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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.

@github-actions github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Jul 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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.

@github-actions

Copy link
Copy Markdown
Contributor

🔴 Playwright Results — 6 test failure(s), 2 pipeline/setup failure(s)

✅ 3965 passed · ❌ 6 failed · 🟡 14 flaky · ⏭️ 30 skipped

Pipeline and setup failures

  • Shard 2 reported zero tests.
  • Shard 2 test execution finished with status failure without a reported test failure.
Shard Passed Failed Flaky Skipped
✅ Shard 1 291 0 0 4
✅ Shard 2 0 0 0 0
🟡 Shard 3 735 0 2 7
🟡 Shard 4 782 0 1 2
🟡 Shard 5 752 0 5 9
✅ Shard 6 693 0 0 0
🔴 Shard 7 712 6 6 8

Genuine Failures (failed on all attempts)

Pages/GlossaryTermRelationSettings.spec.ts › creates a custom relation type via the drawer (shard 7)
TypeError: (0 , _admin.authenticateAdminPage) is not a function
Pages/GlossaryTermRelationSettings.spec.ts › edits a custom relation type and keeps the name immutable (shard 7)
TypeError: (0 , _admin.authenticateAdminPage) is not a function
Pages/GlossaryTermRelationSettings.spec.ts › rejects duplicate relation-type names with an inline error (shard 7)
TypeError: (0 , _admin.authenticateAdminPage) is not a function
Pages/GlossaryTermRelationSettings.spec.ts › deletes a custom relation type (shard 7)
TypeError: (0 , _admin.authenticateAdminPage) is not a function
Pages/GlossaryTermRelationSettings.spec.ts › locks system-defined relation types from edit and delete (shard 7)
TypeError: (0 , _admin.authenticateAdminPage) is not a function
Pages/GlossaryTermRelationSettings.spec.ts › paginates relation types when they exceed a page (shard 7)
TypeError: (0 , _admin.authenticateAdminPage) is not a function
🟡 14 flaky test(s) (passed on retry)
  • Features/BulkEditEntity.spec.ts › Database Schema (shard 3, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should show no results when searching for nonexistent column (shard 3, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 4, 1 retry)
  • Pages/CustomProperties.spec.ts › Enum (shard 5, 1 retry)
  • Pages/CustomProperties.spec.ts › Entity Reference (shard 5, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for MlModel (shard 5, 1 retry)
  • Pages/DataContracts.spec.ts › Contract Status badge should be visible on condition if Contract Tab is present/hidden by Persona (shard 5, 1 retry)
  • Pages/DataContractsSemanticRules.spec.ts › Validate Description Rule Is_Set (shard 5, 1 retry)
  • Pages/Glossary.spec.ts › Glossary Term Update in Glossary Page should persist tree (shard 7, 1 retry)
  • Pages/InputOutputPorts.spec.ts › Output port drawer quick filter - behaviour matrix (shard 7, 2 retries)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 7, 1 retry)
  • Pages/TestSuite.spec.ts › Logical TestSuite (shard 7, 1 retry)
  • Pages/UserDetails.spec.ts › Create team with domain and verify visibility of inherited domain in user profile after team removal (shard 7, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 7, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@sonika-shah
sonika-shah requested a review from a team as a code owner July 28, 2026 12:43
@sonika-shah
sonika-shah marked this pull request as draft July 28, 2026 12:43
@gitar-bot

gitar-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Cherry-pick of the fix excluding soft-deleted entities from the Elasticsearch and OpenSearch column aggregators, complete with a regression test. No issues found.

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 👍 / 👎 | Gitar | Powered by Gitar — free for open source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant