Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sorting on nested field with unmapped #42451

Merged
merged 4 commits into from
May 24, 2019
Merged

Conversation

matriv
Copy link
Contributor

@matriv matriv commented May 23, 2019

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

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: elastic#33644
@matriv matriv added :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.2.0 v7.3.0 v6.8.1 v7.1.1 labels May 23, 2019
@matriv matriv requested review from cbuescher and jimczi May 23, 2019 16:02
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@matriv
Copy link
Contributor Author

matriv commented May 23, 2019

Tried to add some unit test for this, but it's quite difficult because of the mocked QueryShardContext. We would need to mock the getMapperService() and for the returned MapperService the unmappedFieldType(), so decided to go only for an integration test.

@jpountz jpountz added v7.1.2 and removed v7.1.1 labels May 24, 2019
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @matriv, I left some minor comments but the change looks good to me.

throw new QueryShardException(context,
"max_children is only supported on last level of nested sort");
Nested nested = null;
if (!isUnmapped) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we have a convention to use isUnmapped == false for readability.

.addSort(SortBuilders.fieldSort("nested.foo").unmappedType("keyword")
.setNestedSort(new NestedSortBuilder("nested").setNestedSort(new NestedSortBuilder("nested.foo"))))
.get();
assertNoFailures(searchResponse);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test with a nested query ?

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matriv matriv merged commit 631142d into elastic:master May 24, 2019
@matriv matriv deleted the fix-33644 branch May 24, 2019 13:20
matriv added a commit that referenced this pull request May 24, 2019
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

(cherry picked from commit 631142d)
matriv added a commit that referenced this pull request May 24, 2019
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

(cherry picked from commit 631142d)
matriv added a commit that referenced this pull request May 24, 2019
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

(cherry picked from commit 631142d)
matriv added a commit that referenced this pull request May 24, 2019
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

(cherry picked from commit 631142d)
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
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: elastic#33644
@colings86 colings86 added the >bug label Jun 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v6.8.1 v7.1.2 v7.2.0 v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sorting by unmapped nested field
7 participants