fix(column-bulk): backport column grid alphabetical sort to 1.13 (#29699)#29751
fix(column-bulk): backport column grid alphabetical sort to 1.13 (#29699)#29751sonika-shah wants to merge 3 commits into
Conversation
…ame (#29699) * fix(column-bulk): sort column grid results alphabetically by column name 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 * test/fix: deterministic tie-breaker + robust ordering tests 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. * docs: clarify sort comment does not assert caller map type * docs: tighten sort comment and make test comments generic * docs: reword sort comment to say iteration order is not guaranteed
❌ 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}
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Guard the column-name comparator with Comparator.nullsLast so a null column name (e.g. a null map key) sorts last instead of throwing an NPE. Adds a test covering the null case.
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
🟡 Playwright Results — all passed (19 flaky)✅ 3664 passed · ❌ 0 failed · 🟡 19 flaky · ⏭️ 26 skipped
🟡 19 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 |
…required field A null column name (columnName is required in the column-grid response) is now skipped when building rows, so a malformed bucket is dropped and the remaining valid columns are returned. This also avoids the sort NPE at the source; the comparator keeps nullsLast as a backstop. Test updated to assert the null-named row is dropped.
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
Code Review ✅ Approved 1 resolved / 1 findingsBackports deterministic alphabetical sorting to the column bulk grid by implementing case-insensitive ordering with a null-safe comparator. No issues found. ✅ 1 resolved✅ Edge Case: Sort comparator will NPE if a column name key is null
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
1 participant
Backport of #29699 to
1.13Backports #29699 (
fix(column-bulk): sort column grid results alphabetically by column name) into the1.13release line. Refs #29698.Problem
Column Bulk Operations rendered the column list in an arbitrary order — the aggregators request ascending order from the search engine, but the results are collected into a
HashMapandColumnMetadataGrouper.groupColumnsemits rows in map (hash-bucket) iteration order without re-sorting. So the grid appeared unsorted.Fix
Sort the output
gridItemsbycolumnName(ascending, case-insensitive) before returning fromColumnMetadataGrouper.groupColumns, with a case-sensitive tie-breaker so names differing only in case stay stable. Both engines and all query paths (browse / pattern / tag) route through this method, so the single change makes ordering deterministic everywhere.Cherry-picked from
main(bd1cef723b); applied cleanly to1.13.Verification
mvn clean install -DskipTests(backend) — BUILD SUCCESS.ColumnMetadataGrouperTest— 10/10 pass (7 existing + 3 new: ascending order, case-insensitive, case-only-collision determinism).2.0.0-SNAPSHOTbuild across browse, search, filter combinations, and pagination — all return deterministically alphabetical.Greptile Summary
This PR makes column bulk grid results deterministic. The main changes are:
Confidence Score: 4/5
This is close, but the case-only paging behavior should be fixed before merging.
Nameandnamecan return an incomplete column list on pattern or tag queries.Column grid sorting and upstream name pagination.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(column-bulk): skip null-named column..." | Re-trigger Greptile
Context used: