-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Search Source] Specify strict_date_optional_time_nanos as a format #90415
Conversation
…r for date_nanos field
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/discover/_date_nanos·ts.discover app date_nanos should show a timestamp with nanoseconds in the first result rowStandard Out
Stack Trace
Kibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/context/_date_nanos·js.context app context view for date_nanos displays predessors - anchor - successors in right orderStandard Out
Stack Trace
Kibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/context/_date_nanos·js.context app context view for date_nanos displays predessors - anchor - successors in right orderStandard Out
Stack Trace
Metrics [docs]Page load bundle
To update your PR or re-run it, just comment with: |
Is this needed if elastic/elasticsearch#67893 is merged? |
I don't believe we plan to merge that because its a backwards breaking change. Even if we did, I think you'd be in a fairly brittle state because if folks changed the formatter you might end up with something you couldn't parse. I kind of thought kibana would always want to stick its own formatter on dates so it could be sure it could parse them back, similar to when it was requesting dates using |
@nik9000 That PR seems to be correcting unwanted behavior. Might it be merged into 8.0 and not backported? I just want to make sure I'm correctly understanding the problem.
And that would be true of regular dates as well, correct? Requested @ppisljar to review this as well. At this point I think the PR makes sense but want to ensure there's no unwanted side effects. |
The ES folk think that the unwanted behavior is that you made a
Absolutely. I kind of figured you'd specify a formatter for everything that supports it for the safety. It feels like a good level of paranoia. |
@nik9000 what about |
Can that happen for 7.12? This PR might still have value but it would be solving a different problem - setting the format when its been changed from default. That would be overriding a user choice so I'd want us to be certain of that its the right choice. |
I don't imagine it will, no.
I haven't read the PR but didn't kibana used to ask for dates as |
After syncing with @nik9000 and @timroes offline, the consensus was reached that we'd explicitly request a list of date fields (ALL date fields) in the fields request. So the request would look something like:
I think this will also alleviate some of the concerns @mattkime had around this. |
Probably, I'm getting up to speed on this. |
is the plan for this logic to live in searchsource flatten method or in index patterns (part of getComputedFields? ) or being left to actual consumer (as discover) ? seems that at least in this PR the plan is to do it in searchsource which is the place to do it in my opinion |
@ppisljar I agree that |
Closing this as it was used mostly as a reference for discussions. |
Summary
I am opening this PR to move the discussion from various Slack messages I've had over the past few days to a central place.
The problem is related to:
elastic/elasticsearch#67063
This breaks Discover: When ES returns a timestamp without proper precision, we cannot sort documents properly, so they appear out of order.
@nik9000 suggested the following workaround: we could specify
strict_date_optional_time_nanos
as the format for the timestamp field if it isdate_nanos
and this will return the timestamp in the proper format.This works well and makes sense and should be a given if the request is something like:
But the problem is that Discover in most cases requests '*' as a list of fields. In that case, we'd need to check if at least one of the fields in the index is
date_nanos
and append that formatter, so the request would look something like:The problem with this is:
But for another index I'm getting "request doesn't support [format]" exception. I guess this depends on the mapping itself?
search_source
Checklist
Delete any items that are not applicable to this PR.
For maintainers