Skip to content

Adding support for the date_nanos field type #1803

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

Merged
merged 4 commits into from
Dec 1, 2021

Conversation

masseyke
Copy link
Member

@masseyke masseyke commented Nov 9, 2021

Adding support for the date_nanos field type. This field type is now treated in the same way as the date field type.
Closes #1653

@masseyke masseyke added the v8.1.0 label Nov 9, 2021
@masseyke masseyke requested a review from jbaiera November 9, 2021 22:26
Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

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

Does the code suffer from any loss of precision issues from using nano time on older time libraries? Can we add a test to make sure the serialization for date nanos works?

@masseyke
Copy link
Member Author

Does the code suffer from any loss of precision issues from using nano time on older time libraries? Can we add a test to make sure the serialization for date nanos works?

You're right -- the parser only produces milliseconds, and we're only grabbing milliseconds from it at https://github.com/masseyke/elasticsearch-hadoop/blob/feature/date-nanos/spark/core/src/main/scala/org/elasticsearch/spark/serialization/ScalaValueReader.scala#L181.

@masseyke
Copy link
Member Author

masseyke commented Dec 1, 2021

OK, I've changed it so that for date_nanos fields we keep track of the nanoseconds the whole way through. In addition to the unit tests, I manually pulled data out of an index in spark shell and wrote it out to a new index, and the nanoseconds were still correctly there.
I originally converted regular date fields to use java time instead of the mix of java 6 and joda time (uncommitted), but decided to revert that as out of scope for this PR. This should only impact the path taken for date_nanos. We probably ought to switch everything over at some point, now that we only support java 8 and newer.

@masseyke masseyke requested a review from jbaiera December 1, 2021 22:13
Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

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

LGTM

@masseyke masseyke merged commit c651e79 into elastic:master Dec 1, 2021
@masseyke masseyke deleted the feature/date-nanos branch December 1, 2021 22:23
masseyke added a commit to masseyke/elasticsearch-hadoop that referenced this pull request Dec 20, 2021
Adding support for the date_nanos field type. This field type is now treated in the same way as the date
field type, except that we keep track of nanoseconds.
Closes elastic#1653
masseyke added a commit that referenced this pull request Dec 20, 2021
Adding support for the date_nanos field type. This field type is now treated in the same way as the date
field type, except that we keep track of nanoseconds.
Closes #1653
masseyke added a commit to masseyke/elasticsearch-hadoop that referenced this pull request Dec 20, 2021
Adding support for the date_nanos field type. This field type is now treated in the same way as the date
field type, except that we keep track of nanoseconds.
Closes elastic#1653
masseyke added a commit that referenced this pull request Dec 20, 2021
Adding support for the date_nanos field type. This field type is now treated in the same way as the date
field type, except that we keep track of nanoseconds.
Closes #1653
masseyke added a commit that referenced this pull request Dec 21, 2021
This fixes some missing import statements in tests for the date_nanos fix (#1803)
Closes #1835
masseyke added a commit that referenced this pull request Dec 21, 2021
This fixes some missing import statements in tests for the date_nanos fix (#1803)
Closes #1835
masseyke added a commit that referenced this pull request Apr 7, 2022
In #1803 we added support for the data_nanos Elasticsearch field type. However there is a bug in that sets the zone offset to the offset right now, as opposed to what the zone offset was at the time of the date. So for example if the date was 2015-01-01T06:10:30.123456789-06:00, then if you read that date after daylight saving had begun you would see it as 2015-01-01T06:10:30.123456789-05:00.
masseyke added a commit to masseyke/elasticsearch-hadoop that referenced this pull request Apr 7, 2022
In elastic#1803 we added support for the data_nanos Elasticsearch field type. However there is a bug in that sets the zone offset to the offset right now, as opposed to what the zone offset was at the time of the date. So for example if the date was 2015-01-01T06:10:30.123456789-06:00, then if you read that date after daylight saving had begun you would see it as 2015-01-01T06:10:30.123456789-05:00.
masseyke added a commit to masseyke/elasticsearch-hadoop that referenced this pull request Apr 7, 2022
In elastic#1803 we added support for the data_nanos Elasticsearch field type. However there is a bug in that sets the zone offset to the offset right now, as opposed to what the zone offset was at the time of the date. So for example if the date was 2015-01-01T06:10:30.123456789-06:00, then if you read that date after daylight saving had begun you would see it as 2015-01-01T06:10:30.123456789-05:00.
masseyke added a commit that referenced this pull request Apr 8, 2022
In #1803 we added support for the data_nanos Elasticsearch field type. However there is a bug in that sets the zone offset to the offset right now, as opposed to what the zone offset was at the time of the date. So for example if the date was 2015-01-01T06:10:30.123456789-06:00, then if you read that date after daylight saving had begun you would see it as 2015-01-01T06:10:30.123456789-05:00.
masseyke added a commit that referenced this pull request Apr 8, 2022
In #1803 we added support for the data_nanos Elasticsearch field type. However there is a bug in that sets the zone offset to the offset right now, as opposed to what the zone offset was at the time of the date. So for example if the date was 2015-01-01T06:10:30.123456789-06:00, then if you read that date after daylight saving had begun you would see it as 2015-01-01T06:10:30.123456789-05:00.
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.

Unable to read date_nanos
3 participants