fix(column-bulk): make column grid sort null-safe (follow-up to #29699)#29752
Open
sonika-shah wants to merge 2 commits into
Open
fix(column-bulk): make column grid sort null-safe (follow-up to #29699)#29752sonika-shah wants to merge 2 commits into
sonika-shah wants to merge 2 commits into
Conversation
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.
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
Contributor
There was a problem hiding this comment.
Pull request overview
Makes the column bulk-operations grid sorting in ColumnMetadataGrouper.groupColumns null-safe so malformed data (e.g., a null column-name key) doesn’t crash the whole request, while preserving deterministic alphabetical ordering.
Changes:
- Wraps the primary (case-insensitive) and tie-breaker (natural) comparators with
Comparator.nullsLast(...)to preventNullPointerExceptionduring sorting. - Adds a unit test ensuring
nullcolumn names sort last and do not throw.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnMetadataGrouper.java | Updates the grid item comparator chain to be null-safe (nullsLast) while keeping existing ordering semantics. |
| openmetadata-service/src/test/java/org/openmetadata/service/search/ColumnMetadataGrouperTest.java | Adds coverage for the null column-name scenario to prevent regressions. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
🟡 Playwright Results — all passed (23 flaky)✅ 4497 passed · ❌ 0 failed · 🟡 23 flaky · ⏭️ 37 skipped
🟡 23 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.
Code Review ✅ ApprovedWraps column grid sort comparators in nullsLast to prevent NullPointerExceptions and skips null-named columns. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
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.
Follow-up to #29699 — make the column grid sort null-safe
The alphabetical sort added in #29699 (
ColumnMetadataGrouper.groupColumns) usesComparator.comparing(ColumnGridItem::getColumnName, String.CASE_INSENSITIVE_ORDER), which throws aNullPointerExceptionif a column name isnull. Column names are realistically non-null (callers guard the map key), but the sort introduced a new failure mode that didn't exist before — a single malformed bucket with a null key would fail the whole column-grid request instead of returning the remaining results.Fix
Wrap both key comparators in
Comparator.nullsLast(...)so a null column name sorts last rather than crashing:Adds
testGroupColumns_nullColumnNameSortsLastWithoutErrorcovering the null case.Verification
openmetadata-service+ deps) — BUILD SUCCESS.ColumnMetadataGrouperTest— 11/11 pass.Also applied to the 1.13 backport (#29751) so the two lines stay aligned.
Greptile Summary
This PR makes column-grid grouping handle malformed null column names safely.
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(column-bulk): skip null-named column..." | Re-trigger Greptile
Context used (3)