From 162ff3a5d323434f61dea89cc9dbd020a930ef06 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Thu, 23 May 2019 17:57:36 +0200 Subject: [PATCH 1/3] Fix sorting on nested field with unmapped Previously sorting on a missing nested field would fail with an Exception: `[nested_field] failed to find nested object under path [nested_path]` despite `unmapped_type` being set on the query. Fixes: #33644 --- .../search/sort/FieldSortBuilder.java | 30 +++++++++++-------- .../search/sort/FieldSortIT.java | 8 +++++ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index 2be73a0da9cb6..a8888a453c1b9 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -366,6 +366,7 @@ private static NumericType resolveNumericType(String value) { @Override public SortFieldAndFormat build(QueryShardContext context) throws IOException { + boolean isUnmapped = false; if (DOC_FIELD_NAME.equals(fieldName)) { if (order == SortOrder.DESC) { return SORT_DOC_REVERSE; @@ -375,6 +376,7 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException { } else { MappedFieldType fieldType = context.fieldMapper(fieldName); if (fieldType == null) { + isUnmapped = true; if (unmappedType != null) { fieldType = context.getMapperService().unmappedFieldType(unmappedType); } else { @@ -392,20 +394,22 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException { localSortMode = reverse ? MultiValueMode.MAX : MultiValueMode.MIN; } - final Nested nested; - if (nestedSort != null) { - if (context.indexVersionCreated().before(Version.V_6_5_0) && nestedSort.getMaxChildren() != Integer.MAX_VALUE) { - throw new QueryShardException(context, - "max_children is only supported on v6.5.0 or higher"); - } - if (nestedSort.getNestedSort() != null && nestedSort.getMaxChildren() != Integer.MAX_VALUE) { - throw new QueryShardException(context, - "max_children is only supported on last level of nested sort"); + Nested nested = null; + if (!isUnmapped) { + if (nestedSort != null) { + if (context.indexVersionCreated().before(Version.V_6_5_0) && nestedSort.getMaxChildren() != Integer.MAX_VALUE) { + throw new QueryShardException(context, + "max_children is only supported on v6.5.0 or higher"); + } + if (nestedSort.getNestedSort() != null && nestedSort.getMaxChildren() != Integer.MAX_VALUE) { + throw new QueryShardException(context, + "max_children is only supported on last level of nested sort"); + } + // new nested sorts takes priority + nested = resolveNested(context, nestedSort); + } else { + nested = resolveNested(context, nestedPath, nestedFilter); } - // new nested sorts takes priority - nested = resolveNested(context, nestedSort); - } else { - nested = resolveNested(context, nestedPath, nestedFilter); } IndexFieldData fieldData = context.getForField(fieldType); diff --git a/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java b/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java index 526fe0a48b575..604bc195e4049 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java +++ b/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java @@ -903,6 +903,14 @@ public void testIgnoreUnmapped() throws Exception { .addSort(SortBuilders.fieldSort("kkk").unmappedType("keyword")) .get(); assertNoFailures(searchResponse); + + // nested field + searchResponse = client().prepareSearch() + .setQuery(matchAllQuery()) + .addSort(SortBuilders.fieldSort("nested.foo").unmappedType("keyword") + .setNestedSort(new NestedSortBuilder("nested").setNestedSort(new NestedSortBuilder("nested.foo")))) + .get(); + assertNoFailures(searchResponse); } public void testSortMVField() throws Exception { From 421d112aa02e52907cb29a24bee84893b8cbf9d8 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Thu, 23 May 2019 18:34:59 +0200 Subject: [PATCH 2/3] fix variable scope --- .../java/org/elasticsearch/search/sort/FieldSortBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index a8888a453c1b9..3e5e37afc130c 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -366,7 +366,6 @@ private static NumericType resolveNumericType(String value) { @Override public SortFieldAndFormat build(QueryShardContext context) throws IOException { - boolean isUnmapped = false; if (DOC_FIELD_NAME.equals(fieldName)) { if (order == SortOrder.DESC) { return SORT_DOC_REVERSE; @@ -374,6 +373,7 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException { return SORT_DOC; } } else { + boolean isUnmapped = false; MappedFieldType fieldType = context.fieldMapper(fieldName); if (fieldType == null) { isUnmapped = true; From 596b71cbe63d2cad5afd9ae732576364585f9b66 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Fri, 24 May 2019 12:57:13 +0200 Subject: [PATCH 3/3] address comments --- .../org/elasticsearch/search/sort/FieldSortBuilder.java | 2 +- .../java/org/elasticsearch/search/sort/FieldSortIT.java | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index 3e5e37afc130c..8abd4b9f40d5c 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -395,7 +395,7 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException { } Nested nested = null; - if (!isUnmapped) { + if (isUnmapped == false) { if (nestedSort != null) { if (context.indexVersionCreated().before(Version.V_6_5_0) && nestedSort.getMaxChildren() != Integer.MAX_VALUE) { throw new QueryShardException(context, diff --git a/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java b/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java index 604bc195e4049..d3f21867ab1d1 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java +++ b/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java @@ -911,6 +911,14 @@ public void testIgnoreUnmapped() throws Exception { .setNestedSort(new NestedSortBuilder("nested").setNestedSort(new NestedSortBuilder("nested.foo")))) .get(); assertNoFailures(searchResponse); + + // nestedQuery + searchResponse = client().prepareSearch() + .setQuery(matchAllQuery()) + .addSort(SortBuilders.fieldSort("nested.foo").unmappedType("keyword") + .setNestedSort(new NestedSortBuilder("nested").setFilter(QueryBuilders.termQuery("nested.foo", "abc")))) + .get(); + assertNoFailures(searchResponse); } public void testSortMVField() throws Exception {