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

SQL: Make sure now() always uses milliseconds precision #36877

Merged
merged 3 commits into from
Dec 20, 2018

Conversation

astefan
Copy link
Contributor

@astefan astefan commented Dec 20, 2018

This is fix for a CI failure reproducible for Java 9+ versions.
What is specific to these versions (as opposed to Java 8) is that the Clock implementation uses system's best clock implementation, whereas in Java 8 System.currentTimeMillis() is always used. To account for these differences, this solution defines a new Clock which will offer a value for now() set to always have milliseconds precision.

Fixes #35683.

different system clock implementations or different Java versions (for
Java 8, milliseconds precision is used; for Java 9+ a system specific
clock implementation is used which can have greater precision than
what we need here).
@astefan astefan added >test-failure Triaged test failures from CI v7.0.0 :Analytics/SQL SQL querying v6.7.0 labels Dec 20, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@costin
Copy link
Member

costin commented Dec 20, 2018

Nice find. As this issue is fairly obscure and we might bump into it again, it might make sense to move the clock or now function into the TestUtils along with a comment on why it's there.

Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

LGTM. Nice!

@astefan
Copy link
Contributor Author

astefan commented Dec 20, 2018

@elasticmachine run default distro tests

@astefan astefan merged commit 1236461 into elastic:master Dec 20, 2018
@astefan astefan deleted the 35683_fix branch December 20, 2018 11:51
astefan added a commit that referenced this pull request Dec 20, 2018
* This change is to account for different system clock implementations
or different Java versions (for Java 8, milliseconds precision is used;
for Java 9+ a system specific clock implementation is used which can
have greater precision than what we need here).

(cherry picked from commit 1236461)
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/SQL SQL querying >test-failure Triaged test failures from CI v6.7.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] DataTypeConversionTests testConversionToDate
5 participants