-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Conversation
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
Pinging @elastic/es-search |
Tried to add some unit test for this, but it's quite difficult because of the mocked |
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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
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