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

Fix milliseconds handling in intervals (#51675) #52156

Merged
merged 1 commit into from
Feb 10, 2020

Conversation

bpintea
Copy link
Contributor

@bpintea bpintea commented Feb 10, 2020

This fixes:

  • the parsing of milliseconds in intervals: everything past the . used to be converted as-is to milliseconds, with no normalisation of the unit; thus, a value of .23 ended up as 23 millis in the interval, instead of 230.
  • the printing of a trailing .0, in case the interval lacks the fractional part;
  • tests generating a random millisecond value used to simply print it in the string about to be evaluated without a necessary front-filling of 0[s], where the amount was below 100/10.

(The combination of first and last issues above, plus statistical "luck" made the incorrect handling pass the tests.)

(cherry picked from commit 4de8c64)

This fixes:

- the parsing of milliseconds in intervals: everything past the . used to be converted as-is to milliseconds, with no normalisation of the unit; thus, a value of .23 ended up as 23 millis in the interval, instead of 230.
- the printing of a trailing .0, in case the interval lacks the fractional part;
- tests generating a random millisecond value used to simply print it in the string about to be evaluated without a necessary front-filling of 0[s], where the amount was below 100/10.

(The combination of first and last issues above, plus statistical "luck" made the incorrect handling pass the tests.)

(cherry picked from commit 4de8c64)
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/SQL)

@bpintea bpintea merged commit 7b58ed0 into elastic:7.x Feb 10, 2020
@bpintea bpintea deleted the port/7.x_51675 branch February 10, 2020 18:28
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.

2 participants