fix(column-bulk): sort column grid results alphabetically by column name#29699
Merged
Conversation
The column grid rendered rows in HashMap iteration order because ColumnMetadataGrouper.groupColumns emitted them in entrySet order without sorting, discarding the ascending order the aggregators request from the search engine. Sort gridItems by columnName (case-insensitive) so the grid is deterministic and alphabetical across both engines and all query paths. Fixes #29698
Contributor
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
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
This PR fixes non-deterministic ordering in Column Bulk Operations by enforcing a consistent alphabetical sort of column grid results (case-insensitive) at the final grouping stage, so UI lists are predictable regardless of map/aggregation iteration order.
Changes:
- Sort
ColumnMetadataGrouper.groupColumnsoutput bycolumnName(case-insensitive) before returning results. - Add unit tests asserting ascending, case-insensitive ordering of grouped column grid items.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnMetadataGrouper.java | Sorts grouped ColumnGridItem results by column name to make grid ordering deterministic. |
| openmetadata-service/src/test/java/org/openmetadata/service/search/ColumnMetadataGrouperTest.java | Adds tests validating sorted output from groupColumns, including case-insensitive ordering. |
Address review feedback on the column grid sort: - Add a case-sensitive tie-breaker (.thenComparing) so column names differing only in case have a stable order instead of falling back to map iteration. - Use LinkedHashMap with deliberately non-sorted insertion in the sort tests so the unsorted input is deterministic (not reliant on HashMap iteration order). - Add a test asserting case-only collisions sort deterministically.
Contributor
🟡 Playwright Results — all passed (27 flaky)✅ 4033 passed · ❌ 0 failed · 🟡 27 flaky · ⏭️ 22 skipped
🟡 27 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 |
sonika-shah
force-pushed
the
sonika/fix-column-grid-sort
branch
from
828fb2f to
22c669b
Compare
yan-3005
approved these changes
Jul 3, 2026
|
Code Review ✅ Approved 1 resolved / 1 findingsImplements case-insensitive alphabetical sorting in ✅ 1 resolved✅ Edge Case: Case-only name collisions have non-deterministic order
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
This was referenced Jul 5, 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.
Fixes #29698
Problem
In Column Bulk Operations, the Column Name list is not in a consistent order — e.g.
c_abbrevation/c_abnormappear belowc_adm*entries instead of at the top. The order is neither reliably alphabetical nor otherwise predictable, which makes the list hard to scan.Root cause
The aggregators request ascending order from the search engine (composite aggregation
order(SortOrder.Asc)oncolumns.name.keyword; the pattern-search path builds a sortedTreeSet), but that order is lost before the grid is built: the parse step collects buckets into aHashMap, andColumnMetadataGrouper.groupColumnsiteratesentrySet()and emits rows in map (hash-bucket) order without re-sorting. So the grid renders in effectively arbitrary order. This affects both the default browse and the pattern-search paths, on both OpenSearch and Elasticsearch.Fix
Sort the output
gridItemsbycolumnName(ascending, case-insensitive) before returning fromColumnMetadataGrouper.groupColumns. Both engines and all three query paths (browse / pattern / tag) funnel through this method, so the single change makes ordering deterministic everywhere. Case-insensitive ordering matches the pattern path's existingTreeSet(String.CASE_INSENSITIVE_ORDER)dedup and is friendlier for scanning.Pagination is unaffected: browse pages arrive in global ascending order and pattern pages are sliced from a globally-sorted name list, so sorting each returned page alphabetically matches the global order.
Tests
ColumnMetadataGrouperTest:testGroupColumns_resultsAreSortedAscendingByColumnName— feeds a scrambledHashMap(mirroring the reportedc_ab*belowc_adm*case) and asserts ascending output.testGroupColumns_sortIsCaseInsensitive—Zebra,apple,Mango→apple,Mango,Zebra.Verified: all query paths and pagination stay sorted
1. Every return path funnels through
groupColumns— filters only narrow the query, they don't add a path. The three paths and their sinks:groupColumns(OSOpenSearchColumnAggregator.java:219, ESElasticSearchColumnAggregator.java:298)groupColumns(OS:175, ES:261)groupColumns(OS:117, ES:179)Dispatch is in
aggregateColumns(OS:101/:106, ES:118/:123); everything else falls through to browse.2. All three paths paginate with the same (case-insensitive) collation as the sort, so page boundaries don't drift:
TreeSet(String.CASE_INSENSITIVE_ORDER)before slicing (OS:156, ES:202).TreeMap(String.CASE_INSENSITIVE_ORDER)(OS:235, ES:314).*.name.keyword, and that keyword sub-field carrieslowercase_normalizeron all five grid asset indexes, so the composite orders case-insensitively:elasticsearch/en/table_index_mapping.json→columns.name.fields.keyworddashboard_data_model_index_mapping.json→columns.name.fields.keywordtopic_index_mapping.json→messageSchema.schemaFields.name.fields.keywordsearch_entity_index_mapping.json→fields.name.fields.keywordcontainer_index_mapping.json→dataModel.columns.name.fields.keywordElasticSearchColumnAggregator.java:70-79)Displayed column names keep their original case (reconstructed from
_source); only ordering uses the normalized term. Result: the grid is deterministically ascending-alphabetical for the default view, any single or combined filter, search, and tags — consistently across pagination on both OpenSearch and Elasticsearch.Greptile Summary
Sorts the column bulk operations grid alphabetically (case-insensitive) by fixing the single shared output point —
ColumnMetadataGrouper.groupColumns— rather than patching each of the three query paths individually.ColumnMetadataGrouper.java: Adds agridItems.sort(...)call after the list is built, usingString.CASE_INSENSITIVE_ORDERas the primary key and naturalStringordering as a tie-breaker for names that differ only in case (e.g.\"Name\"/\"name\"). The comment explains why the input order is unreliable (HashMap iteration).ColumnMetadataGrouperTest.java: Adds three focused tests covering the reported scrambled-c_ab*/c_adm*scenario, case-insensitive ordering, and deterministic tie-breaking for same-case-fold names; also extracts a sharedsingleOccurrencehelper.Confidence Score: 5/5
Safe to merge — the change is a single in-place sort at the shared output point, with no side-effects on upstream aggregation or pagination logic.
The fix is minimal and well-contained: one sort call is inserted at the only place all query paths converge, so there is no risk of missed paths or double-sorting. The comparator correctly handles case-insensitive ordering with a stable tie-breaker, and three targeted tests directly exercise the reported bug scenario, the case-folding behavior, and the determinism of same-fold names. No data-model or API contract changes are involved.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[aggregateColumns] -->|tag filter present| B[Tag path] A -->|search pattern only| C[Pattern path] A -->|default / scope filter| D[Browse path] B -->|HashMap of columnName → occurrences| E[groupColumns] C -->|TreeSet-deduped names, HashMap results| E D -->|composite-agg buckets, HashMap| E E --> F["Build gridItems list (HashMap iteration order)"] F --> G["gridItems.sort(CASE_INSENSITIVE_ORDER + tie-breaker)"] G --> H[Return sorted ColumnGridItem list]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[aggregateColumns] -->|tag filter present| B[Tag path] A -->|search pattern only| C[Pattern path] A -->|default / scope filter| D[Browse path] B -->|HashMap of columnName → occurrences| E[groupColumns] C -->|TreeSet-deduped names, HashMap results| E D -->|composite-agg buckets, HashMap| E E --> F["Build gridItems list (HashMap iteration order)"] F --> G["gridItems.sort(CASE_INSENSITIVE_ORDER + tie-breaker)"] G --> H[Return sorted ColumnGridItem list]Reviews (7): Last reviewed commit: "Merge branch 'main' into sonika/fix-colu..." | Re-trigger Greptile