Skip to content

Commit 35fbefa

Browse files
mohityadav766claude
mohityadav766
and
claude
authored
Fix: Resolve text fields to .keyword for ES/OS sorting and aggregation (#27103)
* Fix: Resolve text fields to .keyword for ES/OS sorting and aggregation Text fields like `name` and `displayName` are mapped as `text` type in ES/OS indexes (for full-text search) with `.keyword` sub-fields for sorting and aggregation. When these bare field names were passed to sort or terms aggregation builders, ES/OS rejected them with `illegal_argument_exception` because text fields cannot be used for per-document field data operations. Add centralized `resolveFieldForSortOrAggregation()` in SearchSourceBuilderFactory that converts known text fields to their `.keyword` sub-fields, and apply it across both search managers and aggregation builders (terms, top_hits) for both ES and OpenSearch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix: Address code review comments on resolveFieldForSortOrAggregation - Support nested text fields (e.g. columns.name → columns.name.keyword) by extracting the leaf segment for whitelist lookup instead of skipping all dotted paths - Fix unmappedType="integer" for remapped owner fields (ownerDisplayName, ownerName) by introducing KEYWORD_SORT_FIELDS set used alongside the .keyword suffix check in both search managers - Remove dead remapAggregationField() method (no callers remained) - Add 2 new test cases: nested text field resolution and flat keyword sort field passthrough Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix: Address round 2 review comments on resolveFieldForSortOrAggregation - Expand AGGREGATION_FIELD_REMAPS to cover bare owner paths (owners.name, owners.displayName) so they remap to ownerName/ownerDisplayName instead of incorrectly gaining a .keyword suffix - Replace leaf-based extraction with exact-path matching so only root-level name/displayName fields get .keyword appended; dotted paths like service.name or columns.name now pass through unchanged - Remove convertsNestedTextFieldsToKeyword test (no longer valid behavior) - Add bare owner remap tests and doesNotAppendKeywordToNestedTextPaths test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 967e5cd commit 35fbefa

12 files changed

Lines changed: 191 additions & 20 deletions

‎openmetadata-service/src/main/java/org/openmetadata/service/search/SearchSourceBuilderFactory.java‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -225,16 +225,60 @@ default boolean isNonFuzzyField(String key) {
225225
}
226226

227227
/**
228-
* Remap nested owner field paths to their flat top-level equivalents.
229-
* The flat fields {@code ownerDisplayName} and {@code ownerName} are
230-
* denormalized copies maintained in every search index.
228+
* Remap nested owner field paths to their flat top-level equivalents. The flat fields {@code
229+
* ownerDisplayName} and {@code ownerName} are denormalized copies maintained in every search
230+
* index.
231231
*/
232232
Map<String, String> AGGREGATION_FIELD_REMAPS =
233233
Map.of(
234234
"owners.displayName.keyword", "ownerDisplayName",
235-
"owners.name.keyword", "ownerName");
235+
"owners.displayName", "ownerDisplayName",
236+
"owners.name.keyword", "ownerName",
237+
"owners.name", "ownerName");
236238

237-
static String remapAggregationField(String field) {
238-
return AGGREGATION_FIELD_REMAPS.getOrDefault(field, field);
239+
/**
240+
* Root-level text fields that carry a {@code .keyword} sub-field in all index mappings. Only
241+
* exact root-level names match; dotted paths (e.g. {@code columns.name}) are not resolved here.
242+
*/
243+
Set<String> TEXT_FIELDS_WITH_KEYWORD = Set.of("name", "displayName");
244+
245+
/**
246+
* Flat keyword fields produced by owner-field remapping. These do not carry a {@code .keyword}
247+
* suffix but must be treated as keyword sort fields for correct {@code unmappedType} resolution.
248+
*/
249+
Set<String> KEYWORD_SORT_FIELDS = Set.of("ownerDisplayName", "ownerName");
250+
251+
/**
252+
* Resolve a field name for use in sorting or aggregation contexts.
253+
*
254+
* <ul>
255+
* <li>Applies owner-field remapping (e.g. {@code owners.displayName.keyword} &rarr; {@code
256+
* ownerDisplayName})
257+
* <li>Passes through ES/OS internal fields that start with {@code _}
258+
* <li>Passes through fields already ending with {@code .keyword}
259+
* <li>Appends {@code .keyword} to root-level fields that exactly match {@code
260+
* TEXT_FIELDS_WITH_KEYWORD} ({@code name}, {@code displayName}); dotted paths pass through
261+
* unchanged
262+
* <li>Passes through all other fields (numeric, date, keyword-typed) unchanged
263+
* </ul>
264+
*/
265+
static String resolveFieldForSortOrAggregation(String field) {
266+
if (field == null || field.isEmpty()) {
267+
return field;
268+
}
269+
String remapped = AGGREGATION_FIELD_REMAPS.getOrDefault(field, field);
270+
if (!remapped.equals(field)) {
271+
return remapped;
272+
}
273+
if (field.startsWith("_")) {
274+
return field;
275+
}
276+
if (field.endsWith(".keyword")) {
277+
return field;
278+
}
279+
if (TEXT_FIELDS_WITH_KEYWORD.contains(field)) {
280+
return field + ".keyword";
281+
}
282+
return field;
239283
}
240284
}

‎openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchAggregationManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ public Response aggregate(AggregationRequest request) throws IOException {
143143
}
144144

145145
String aggregationField =
146-
SearchSourceBuilderFactory.remapAggregationField(request.getFieldName());
146+
SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(request.getFieldName());
147147
if (aggregationField == null || aggregationField.isBlank()) {
148148
throw new IllegalArgumentException("Aggregation field (fieldName) cannot be null or empty");
149149
}

‎openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchSearchManager.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import org.openmetadata.service.search.SearchManagementClient;
7575
import org.openmetadata.service.search.SearchResultListMapper;
7676
import org.openmetadata.service.search.SearchSortFilter;
77+
import org.openmetadata.service.search.SearchSourceBuilderFactory;
7778
import org.openmetadata.service.search.SearchUtils;
7879
import org.openmetadata.service.search.elasticsearch.queries.ElasticQueryBuilder;
7980
import org.openmetadata.service.search.nlq.NLQService;
@@ -1183,14 +1184,18 @@ private ElasticSearchRequestBuilder buildSearchRequestBuilder(
11831184

11841185
// Handle sorting
11851186
if (!nullOrEmpty(request.getSortFieldParam()) && !request.getIsHierarchy()) {
1186-
String sortField = request.getSortFieldParam();
1187+
String sortField =
1188+
SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(request.getSortFieldParam());
11871189
String sortTypeCapitalized =
11881190
request.getSortOrder().substring(0, 1).toUpperCase()
11891191
+ request.getSortOrder().substring(1).toLowerCase();
11901192
SortOrder sortOrder = SortOrder.valueOf(sortTypeCapitalized);
11911193

11921194
if (!sortField.equalsIgnoreCase("_score")) {
1193-
requestBuilder.sort(sortField, sortOrder, "integer");
1195+
boolean isKeywordField =
1196+
sortField.endsWith(".keyword")
1197+
|| SearchSourceBuilderFactory.KEYWORD_SORT_FIELDS.contains(sortField);
1198+
requestBuilder.sort(sortField, sortOrder, isKeywordField ? "keyword" : "integer");
11941199
} else {
11951200
requestBuilder.sort(sortField, sortOrder, null);
11961201
}

‎openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchSourceBuilderFactory.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,8 @@ private ElasticSearchRequestBuilder addAggregationV2(
275275
int maxSize = searchSettings.getGlobalSettings().getMaxAggregateSize();
276276

277277
if (!nullOrEmpty(agg.getField())) {
278-
String field = SearchSourceBuilderFactory.remapAggregationField(agg.getField());
278+
String field =
279+
SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(agg.getField());
279280
termsAgg = ElasticAggregationBuilder.termsAggregation(field, maxSize);
280281
} else if (!nullOrEmpty(agg.getScript())) {
281282
termsAgg =
@@ -932,7 +933,7 @@ protected void addConfiguredAggregationsV2(
932933
int maxSize = searchSettings.getGlobalSettings().getMaxAggregateSize();
933934

934935
if (!nullOrEmpty(agg.getField())) {
935-
String field = SearchSourceBuilderFactory.remapAggregationField(agg.getField());
936+
String field = SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(agg.getField());
936937
termsAgg = ElasticAggregationBuilder.termsAggregation(field, maxSize);
937938
} else if (!nullOrEmpty(agg.getScript())) {
938939
termsAgg = ElasticAggregationBuilder.termsAggregationWithScript(agg.getScript(), maxSize);

‎openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/aggregations/ElasticTermsAggregations.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import lombok.Getter;
1010
import lombok.Setter;
1111
import org.openmetadata.service.search.SearchAggregationNode;
12+
import org.openmetadata.service.search.SearchSourceBuilderFactory;
1213

1314
@Setter
1415
@Getter
@@ -26,7 +27,7 @@ public void createAggregation(SearchAggregationNode node) {
2627
Map<String, String> params = node.getValue();
2728
this.aggregationName = node.getName();
2829

29-
this.field = params.get("field");
30+
this.field = SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(params.get("field"));
3031
this.includesStr = params.get("include");
3132
String sizeStr = params.get("size");
3233
this.missing = params.get("missing");

‎openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/aggregations/ElasticTopHitsAggregations.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import lombok.Getter;
99
import lombok.Setter;
1010
import org.openmetadata.service.search.SearchAggregationNode;
11+
import org.openmetadata.service.search.SearchSourceBuilderFactory;
1112

1213
@Setter
1314
@Getter
@@ -23,7 +24,8 @@ public void createAggregation(SearchAggregationNode node) {
2324
this.aggregationName = node.getName();
2425

2526
int size = Integer.parseInt(params.get("size"));
26-
String sortField = params.get("sort_field");
27+
String sortField =
28+
SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(params.get("sort_field"));
2729
String sortOrderParam = params.get("sort_order");
2830
SortOrder sortOrder = sortOrderParam.equalsIgnoreCase("desc") ? SortOrder.Desc : SortOrder.Asc;
2931

‎openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchAggregationManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ public Response aggregate(AggregationRequest request) throws IOException {
143143
}
144144

145145
String aggregationField =
146-
SearchSourceBuilderFactory.remapAggregationField(request.getFieldName());
146+
SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(request.getFieldName());
147147
if (aggregationField == null || aggregationField.isBlank()) {
148148
throw new IllegalArgumentException("Aggregation field (fieldName) cannot be null or empty");
149149
}

‎openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchSearchManager.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import org.openmetadata.service.search.SearchManagementClient;
6060
import org.openmetadata.service.search.SearchResultListMapper;
6161
import org.openmetadata.service.search.SearchSortFilter;
62+
import org.openmetadata.service.search.SearchSourceBuilderFactory;
6263
import org.openmetadata.service.search.SearchUtils;
6364
import org.openmetadata.service.search.nlq.NLQService;
6465
import org.openmetadata.service.search.opensearch.queries.OpenSearchQueryBuilder;
@@ -1222,14 +1223,18 @@ private OpenSearchRequestBuilder buildSearchRequestBuilder(
12221223
// Handle sorting
12231224
if (!nullOrEmpty(request.getSortFieldParam())
12241225
&& !Boolean.TRUE.equals(request.getIsHierarchy())) {
1225-
String sortField = request.getSortFieldParam();
1226+
String sortField =
1227+
SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(request.getSortFieldParam());
12261228
String sortTypeCapitalized =
12271229
request.getSortOrder().substring(0, 1).toUpperCase()
12281230
+ request.getSortOrder().substring(1).toLowerCase();
12291231
SortOrder sortOrder = SortOrder.valueOf(sortTypeCapitalized);
12301232

12311233
if (!sortField.equalsIgnoreCase("_score")) {
1232-
requestBuilder.sort(sortField, sortOrder, "integer");
1234+
boolean isKeywordField =
1235+
sortField.endsWith(".keyword")
1236+
|| SearchSourceBuilderFactory.KEYWORD_SORT_FIELDS.contains(sortField);
1237+
requestBuilder.sort(sortField, sortOrder, isKeywordField ? "keyword" : "integer");
12331238
} else {
12341239
requestBuilder.sort(sortField, sortOrder, null);
12351240
}

‎openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchSourceBuilderFactory.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,7 @@ protected void addConfiguredAggregationsV2(
722722
int maxSize = searchSettings.getGlobalSettings().getMaxAggregateSize();
723723

724724
if (!nullOrEmpty(agg.getField())) {
725-
String field = SearchSourceBuilderFactory.remapAggregationField(agg.getField());
725+
String field = SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(agg.getField());
726726
termsAgg = OpenSearchAggregationBuilder.termsAggregation(field, maxSize);
727727
} else if (!nullOrEmpty(agg.getScript())) {
728728
termsAgg =
@@ -743,7 +743,8 @@ private OpenSearchRequestBuilder addAggregationV2(OpenSearchRequestBuilder searc
743743
int maxSize = searchSettings.getGlobalSettings().getMaxAggregateSize();
744744

745745
if (!nullOrEmpty(agg.getField())) {
746-
String field = SearchSourceBuilderFactory.remapAggregationField(agg.getField());
746+
String field =
747+
SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(agg.getField());
747748
termsAgg = OpenSearchAggregationBuilder.termsAggregation(field, maxSize);
748749
} else if (!nullOrEmpty(agg.getScript())) {
749750
termsAgg =

‎openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/aggregations/OpenTermsAggregations.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import lombok.Getter;
99
import lombok.Setter;
1010
import org.openmetadata.service.search.SearchAggregationNode;
11+
import org.openmetadata.service.search.SearchSourceBuilderFactory;
1112
import os.org.opensearch.client.opensearch._types.aggregations.Aggregation;
1213

1314
@Setter
@@ -26,7 +27,7 @@ public void createAggregation(SearchAggregationNode node) {
2627
Map<String, String> params = node.getValue();
2728
this.aggregationName = node.getName();
2829

29-
this.field = params.get("field");
30+
this.field = SearchSourceBuilderFactory.resolveFieldForSortOrAggregation(params.get("field"));
3031
this.includesStr = params.get("include");
3132
String sizeStr = params.get("size");
3233
this.missing = params.get("missing");

0 commit comments

Comments
 (0)