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

Patch up fetching dates from source (backport of #70040) #70117

Merged
merged 1 commit into from
Mar 9, 2021

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Mar 8, 2021

This fixes an issue that fields has with dates sent in the
###.###### format.

If you send a date in the format #####.###### we'll parse the bit
before the decimal place as the number of milliseconds since epoch and
we'll parse the bit after the decimal as the number of nanoseconds since
the start of that millisecond. This works and is convenient for some
folks. Sadly, the code that back the fields API for dates doesn't work
with the string format in this case - it works with a double. double
is bad for two reasons:

  1. It's default string representation is scientific notation and our
    parsers don't know how to deal with that.
  2. It loses precision relative to the string representation. double
    only has 52 bits of mantissa which can precisely store the number of
    nanoseconds until about 6am on April 15th, 1970. After that it starts
    to lose precision.

This fixed the first issue, getting us the correct string
representation is a "quick and dirty" way. It just converts the double
back to a string. But we still lose precision. Fixing that would require
a larger change.....

This fixes an issue that `fields` has with dates sent in the
`###.######` format.

If you send a date in the format `#####.######` we'll parse the bit
before the decimal place as the number of milliseconds since epoch and
we'll parse the bit after the decimal as the number of nanoseconds since
the start of that millisecond. This works and is convenient for some
folks. Sadly, the code that back the `fields` API for dates doesn't work
with the string format in this case - it works with a `double`. `double`
is bad for two reasons:
1. It's default string representation is scientific notation and our
   parsers don't know how to deal with that.
2. It loses precision relative to the string representation. `double`
   only has 52 bits of mantissa which can precisely store the number of
   nanoseconds until about 6am on April 15th, 1970. After that it starts
   to lose precision.

This fixed the first issue, getting us the correct string
representation is a "quick and dirty" way. It just converts the `double`
back to a string. But we still lose precision. Fixing that would require
a larger change.....
@nik9000 nik9000 merged commit 3db9dcf into elastic:7.x Mar 9, 2021
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Mar 9, 2021
…tic#70117)

This fixes an issue that `fields` has with dates sent in the
`###.######` format.

If you send a date in the format `#####.######` we'll parse the bit
before the decimal place as the number of milliseconds since epoch and
we'll parse the bit after the decimal as the number of nanoseconds since
the start of that millisecond. This works and is convenient for some
folks. Sadly, the code that back the `fields` API for dates doesn't work
with the string format in this case - it works with a `double`. `double`
is bad for two reasons:
1. It's default string representation is scientific notation and our
   parsers don't know how to deal with that.
2. It loses precision relative to the string representation. `double`
   only has 52 bits of mantissa which can precisely store the number of
   nanoseconds until about 6am on April 15th, 1970. After that it starts
   to lose precision.

This fixed the first issue, getting us the correct string
representation is a "quick and dirty" way. It just converts the `double`
back to a string. But we still lose precision. Fixing that would require
a larger change.....
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.

1 participant