Fixes #26824: filter column bulk operations metadata status on the aggregate row - #32905
Merged
Conversation
…, not per document The "Has / Missing Metadata" filter in Column Bulk Operations did not hide rows of other statuses, and page counts drifted per page. metadataStatus (MISSING/INCOMPLETE/COMPLETE) was pushed down as a per-document search query, while INCONSISTENT/hasConflicts/hasMissingMetadata were applied after the aggregator had already paginated and computed totals. But a row's status is an aggregate over all of a column's occurrences (INCONSISTENT when they disagree), so a per-document filter re-grouped into rows whose status differed from the request, and the post-pagination filter shrank the page while the total stayed unfiltered. Move every row-level filter (metadataStatus, hasConflicts, hasMissingMetadata) onto the fully-grouped items, before pagination, with totals derived from the filtered set. Shared pure helpers (hasRowLevelFilter, matchesRowFilters, paginateFilteredItems) live on the ColumnAggregator interface; both aggregators route through a materialized name-enumeration path when such a filter is active, and the untouched composite/pattern paths still serve unfiltered browsing. Removing the per-document status query also removes the wildcard/exists query on flat-object columns.description/columns.tags that crashed ES/OS with search_phase_execution_exception and had ColumnGridResourceIT disabled; the IT is re-enabled and its status assertions strengthened to check every returned row carries the requested status, plus a status+pagination consistency test.
Contributor
❌ 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
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Contributor
✅ Playwright Results — workflow succeededValidated commit ✅ 4476 passed · ❌ 0 failed · 🟡 7 flaky · ⏭️ 1 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 56m 12s ⏱️ Max setup 4m 56s · max shard execution 22m 15s · max shard-job elapsed before upload 25m 11s · reporting 20s 🌐 216.72 requests/attempt · 2.31 app boots/UI scenario · 34.46% common-shard skew Optimization targets still in progress:
🟡 7 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 |
…ource scan Replace the row-filter path's names-agg + per-name top_hits fan-out (~1+N queries/page) with a single _source scan per field-path group — the same mechanism the tag/glossary filter already uses — restricting _source to the column tree and entity-identity fields. This cuts queries per page from ~1+N to ~1 per field-path group, reuses the already-verified in-memory pagination, and reads every occurrence of a column instead of a 100-doc top_hits sample, so the aggregate status can no longer be misclassified (e.g. COMPLETE vs INCONSISTENT) by under-sampling. Restricting _source to columns + identity fields keeps the per-entity payload small, dropping the heavy entity-level derived fields (columnNames/columnNamesFuzzy). extractMatchingColumnsFromHit gains an includeAllColumns flag (the tag path passes false; the status scan passes true); the now-unused name-enumeration constants are removed.
Collaborator
Author
End-to-end filter test at scale — PR #32905 (Fixes #26824)Ran a fresh local stack built from this branch (verified the server jar contains our methods: Seed (fresh instance, both entity types)
Elasticsearch (MySQL + ES) — 80 / 80 checks pass
OpenSearch (Postgres + OS) — 33 / 33 correctness checks pass
Two scale findings (neither a regression from this PR)
VerdictThe #26824 fix behaves correctly at ~13k-column scale on both engines, across every filter and combination, with correct pagination and no crashes attributable to this change. The only stress point is a pre-existing/environmental memory bound on a pathological single 12k-column table under sustained deep paging — not a correctness defect and not introduced by this PR; the caching follow-up would remove it. |
|
yan-3005
approved these changes
Sep 8, 2026
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 #26824
In Column Bulk Operations, the "Has / Missing Metadata" filter (
metadataStatus) did not actually hide rows of other statuses, and the page counts were wrong. Picking "Complete" or "Incomplete" still showed Missing/Inconsistent rows, and the number of items per page drifted from page to page.Root cause was that the filter ran in the wrong place.
metadataStatus(MISSING/INCOMPLETE/COMPLETE) was pushed down as a per-document search query, while INCONSISTENT/hasConflicts/hasMissingMetadatawere applied after the aggregator had already paginated and computed the totals. But the status shown for a row is an aggregate over all of a column's occurrences (INCONSISTENT when they disagree), so a per-document filter re-grouped into rows whose status differed from the request, and the post-pagination filter shrank the page while the total stayed unfiltered.This moves every status/row-level filter to run on the fully-grouped items, before pagination, and derives the totals from the filtered set — so a status filter returns only rows of that status and the page count and per-page size stay correct.
As a bonus, the removed per-document query is the wildcard/exists query on flat-object
columns.description/columns.tagsthat crashed ES/OS withsearch_phase_execution_exception ... all shards failedand hadColumnGridResourceITdisabled. With it gone, the IT is re-enabled.Type of change:
High-level design:
The column-grid aggregator already runs three paths (composite browse, terms-agg name pattern, tag/glossary source read) introduced in #27216, each ending in
ColumnMetadataGrouper.groupColumns.A filter that depends on a row's aggregate status can't be a per-document push-down or a per-page post-filter — it has to run on the grouped items. So:
ColumnAggregatorinterface:hasRowLevelFilter,matchesRowFilters, andpaginateFilteredItems(filter grouped items by aggregate status /hasConflicts/hasMissingMetadata, then paginate in memory with totals computed from the filtered set). These are unit-tested with no ES/OS needed._sourcefor the scoped entities in one scan per field-path group — the same mechanism the tag/glossary filter already uses — group every column, then filter + paginate the items. The_sourceis restricted to the column tree and entity-identity fields, so the per-entity payload stays small (it drops the heavy entity-level derived fields likecolumnNames/columnNamesFuzzy). When no such filter is active, the existing composite/pattern paths are untouched.metadataStatusquery and its now-dead helpers from both aggregators, and removed the post-pagination filter block fromColumnRepository.ColumnResourcenow documents INCONSISTENT as a supportedmetadataStatusvalue (it is now handled uniformly with the others).Scale note: the scan is bounded to 10K entities per field-path group (the same bound the tag path uses, logged at WARN when exceeded) and only runs when a status/row-level filter is active. Because it reads every occurrence (not a
top_hitssample), the aggregate status can't be misclassified by under-sampling.Tests:
Use cases covered
metadataStatus=COMPLETEreturns only rows whose aggregate status is COMPLETE (no Missing/Incomplete/Inconsistent leak) — the reported bug.size=2over 3 matching + 1 non-matching column,totalUniqueColumnsis 3 (not 4), page 1 has 2 rows with a cursor, page 2 has the remaining 1 — the pagination half of the bug.Unit tests
ColumnAggregatorTest:matchesRowFilters(status match, case-insensitivity, INCONSISTENT as first-class, null/blank passes all,hasConflicts) andpaginateFilteredItems(filtered totals; multi-page cursor consistency).Backend integration tests
ColumnGridResourceIT(removed the@Disabledthat the crashing query forced) and strengthened its status tests to assert every returned row carries the requested status; addedtest_getColumnGrid_metadataStatusPaginationCountsAreConsistent. FixedcreateTableWithFullMetadatato add a tag so the "Complete" fixture is genuinely COMPLETE (it previously had a description but no tags → INCOMPLETE, which the old assert-not-null test never caught).Playwright (UI) tests
Manual testing performed
Backend-only change. Validated the pure filter/pagination logic with the new
ColumnAggregatorTestunit tests locally; the re-enabled ES/OSColumnGridResourceITruns against testcontainers in CI.Additionally ran a full end-to-end filter test on a fresh local stack built from this branch, on both search engines (MySQL+Elasticsearch and Postgres+OpenSearch), seeded with ~13k unique columns across tables and dashboard data models (including a 12k-column wide table) with mixed statuses/tags. Every filter and combination (metadataStatus × columnNamePattern × scope, hasConflicts, hasMissingMetadata, entityTypes, pagination) returned correct results with correct totals and no crashes attributable to this change — 80/80 checks on ES, 33/33 on OS. See the test-summary comment on this PR for details.
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.