diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnMetadataGrouper.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnMetadataGrouper.java index 0ebc13ef4756..213717b32935 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnMetadataGrouper.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnMetadataGrouper.java @@ -16,6 +16,7 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -39,6 +40,11 @@ public static List groupColumns( for (Map.Entry> entry : columnsByName.entrySet()) { String columnName = entry.getKey(); + if (columnName == null) { + // A column with no name isn't a valid grid row (columnName is required in the response); + // skip the malformed bucket so we return the remaining valid columns. + continue; + } List occurrences = entry.getValue(); Map groups = new HashMap<>(); @@ -101,6 +107,17 @@ public static List groupColumns( gridItems.add(gridItem); } + // groupColumns is the single sink for every query path. The input map's iteration order is not + // guaranteed (current callers build it as a HashMap), so sort here for a stable alphabetical + // order. Case-insensitive for natural listing, with a case-sensitive tie-breaker so names that + // differ only in case (e.g. "Name"/"name") stay stable instead of relying on map order. + // nullsLast guards against a null column name (e.g. a null map key) rather than throwing. + gridItems.sort( + Comparator.comparing( + ColumnGridItem::getColumnName, Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER)) + .thenComparing( + ColumnGridItem::getColumnName, Comparator.nullsLast(Comparator.naturalOrder()))); + return gridItems; } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/ColumnMetadataGrouperTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/ColumnMetadataGrouperTest.java index fefc8bcb64d9..f495413a09ab 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/ColumnMetadataGrouperTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/ColumnMetadataGrouperTest.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; @@ -337,4 +338,103 @@ void testGroupColumns_emptyMap() { assertNotNull(result); assertEquals(0, result.size()); } + + @Test + void testGroupColumns_resultsAreSortedAscendingByColumnName() { + // Feed columns in a non-alphabetical order (LinkedHashMap keeps the input order deterministic) + // and verify the grid is returned sorted ascending by column name. + Map> columnsByName = + new LinkedHashMap<>(); + List scrambled = + List.of( + "c_admpkg", + "c_addin", + "c_active_agent_id", + "c_admincare", + "c_address", + "c_admission_date", + "c_acasclop", + "c_abnorm", + "c_abbrevation"); + for (String name : scrambled) { + columnsByName.put(name, singleOccurrence(name)); + } + + List result = ColumnMetadataGrouper.groupColumns(columnsByName); + + List actual = result.stream().map(ColumnGridItem::getColumnName).toList(); + List expected = new ArrayList<>(scrambled); + expected.sort(String.CASE_INSENSITIVE_ORDER); + assertEquals(expected, actual); + assertEquals("c_abbrevation", actual.get(0)); + assertEquals("c_abnorm", actual.get(1)); + } + + @Test + void testGroupColumns_sortIsCaseInsensitive() { + // Insertion order (Zebra, apple, Mango) is non-alphabetical and preserved by LinkedHashMap. + Map> columnsByName = + new LinkedHashMap<>(); + for (String name : List.of("Zebra", "apple", "Mango")) { + columnsByName.put(name, singleOccurrence(name)); + } + + List result = ColumnMetadataGrouper.groupColumns(columnsByName); + + List actual = result.stream().map(ColumnGridItem::getColumnName).toList(); + assertEquals(List.of("apple", "Mango", "Zebra"), actual); + } + + @Test + void testGroupColumns_caseOnlyCollisionsAreDeterministic() { + // Names differing only in case are equal under CASE_INSENSITIVE_ORDER, so the case-sensitive + // tie-breaker decides their order. Insert in reverse of the expected result to prove the sort, + // not the map, determines it. + Map> columnsByName = + new LinkedHashMap<>(); + for (String name : List.of("name", "Name")) { + columnsByName.put(name, singleOccurrence(name)); + } + + List result = ColumnMetadataGrouper.groupColumns(columnsByName); + + List actual = result.stream().map(ColumnGridItem::getColumnName).toList(); + // Natural String order: 'N' (0x4E) precedes 'n' (0x6E). + assertEquals(List.of("Name", "name"), actual); + } + + @Test + void testGroupColumns_nullColumnNameIsSkipped() { + // A null column name (e.g. a null map key) is dropped, not returned with a null required field, + // and must not throw. The remaining valid columns still come back sorted. + Map> columnsByName = + new LinkedHashMap<>(); + columnsByName.put("beta", singleOccurrence("beta")); + columnsByName.put(null, singleOccurrence(null)); + columnsByName.put("alpha", singleOccurrence("alpha")); + + List result = ColumnMetadataGrouper.groupColumns(columnsByName); + + List actual = result.stream().map(ColumnGridItem::getColumnName).toList(); + assertEquals(List.of("alpha", "beta"), actual); + } + + private static List singleOccurrence(String columnName) { + Column column = new Column(); + column.setName(columnName); + column.setDataType(ColumnDataType.STRING); + column.setFullyQualifiedName("sample_data.db.schema.table1." + columnName); + + List occurrences = new ArrayList<>(); + occurrences.add( + new ColumnMetadataGrouper.ColumnWithContext( + column, + "table", + "sample_data.db.schema.table1", + "Table 1", + "sample_data", + "db", + "schema")); + return occurrences; + } }