-
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
Allows nanosecond resolution in search_after #60328
Allows nanosecond resolution in search_after #60328
Conversation
This fixes `search_after` to properly parse string formatted dates that have nanosecond resolution. Closes elastic#52424
Pinging @elastic/es-search (:Search/Search) |
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.
I left one question, LGTM otherwise
- length: {hits.hits: 1 } | ||
- match: {hits.hits.0._index: test } | ||
- match: {hits.hits.0._source.timestamp: "2019-10-21 08:30:04.828733" } | ||
- match: {hits.hits.0.sort: [1571646604828733000] } |
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.
I wonder if we should use the formatted version in the sort
output ? We don't allow this format (nanoseconds since epoch) when ingesting or querying a date_nanos field so that's an exception here.
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.
I'm honestly not sure! I kind of figured the contact of search_after
was "take the last sort value and stick it into search_after
" but I see that we support formatted.
I also wonder what counts as breaking here. If we were to change the result to be formatted I imagine we'll break someone.
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.
We do support formatted output so my point was that the sort
section should return the formatted version in order to avoid returning long that represents nanoseconds since epoch. I agree that this is not in the scope of this change so let's merge this one and we can discuss the formatting of sort values in a follow up (I can open the issue) ?
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.
I agree that this is not in the scope of this change so let's merge this one and we can discuss the formatting of sort values in a follow up (I can open the issue) ?
👍
server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java
Outdated
Show resolved
Hide resolved
This fixes `search_after` to properly parse string formatted dates that have nanosecond resolution. Closes elastic#52424
This fixes
search_after
to properly parse string formatted dates thathave nanosecond resolution.
Closes #52424