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

[ML] Add support for date_nanos fields in find_file_structure #62048

Merged

Conversation

droberts195
Copy link
Contributor

Now that #61324 is merged it is possible for the find_file_structure
endpoint to suggest using date_nanos fields for timestamps where
the timestamp format provides greater than millisecond accuracy.

Now that elastic#61324 is merged it is possible for the find_file_structure
endpoint to suggest using date_nanos fields for timestamps where
the timestamp format provides greater than millisecond accuracy.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

Copy link
Contributor

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

LGTM

Just one minor style nit and one question.

int consecutiveSs = 0;
for (int pos = 0; pos < format.length(); ++pos) {
char curChar = format.charAt(pos);
if (curChar == '\'') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just double-checking: can there be a quotation inside another quotation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A literal ' is specified by two consecutive single quotes - see https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html. This means that no special logic is needed in this method. If there is a literal single quote then this method will treat it as an empty literal section, which is technically wrong but achieves the desired effect.

But since this is quite subtle I will add a comment to say what is going on.

@droberts195 droberts195 merged commit 4b4dab1 into elastic:master Sep 8, 2020
@droberts195 droberts195 deleted the date_nanos_in_structure_finder branch September 8, 2020 11:48
droberts195 added a commit that referenced this pull request Sep 8, 2020
Now that #61324 is merged it is possible for the find_file_structure
endpoint to suggest using date_nanos fields for timestamps where
the timestamp format provides greater than millisecond accuracy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants