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.

Sign up for GitHub

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

Jump to bottom

CompositeKeyExtractorTest.testExtractDate fails #30156

Closed
mayya-sharipova opened this issue Apr 25, 2018 · 6 comments
Closed

CompositeKeyExtractorTest.testExtractDate fails #30156

mayya-sharipova opened this issue Apr 25, 2018 · 6 comments
Assignees
@cbuescher
Labels
:Analytics/SQL SQL querying >test Issues or PRs that are addressing/adding tests v6.4.0

Comments

@mayya-sharipova
Copy link
Contributor

mayya-sharipova commented Apr 25, 2018 •
edited
Loading

Test reproducibly fails on elasticsearch 6.3, 6.x, and master on my local Mac laptop:

Last CLI failure on

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+6.3+multijob-unix-compatibility/os=fedora/3/console

Reproduce:
./gradlew :x-pack:plugin:sql:test -Dtests.seed=56E62F320F1CC3E3 -Dtests.class=org.elasticsearch.xpack.sql.execution.search.extractor.CompositeKeyExtractorTests -Dtests.method="testExtractDate" -Dtests.security.manager=true -Dtests.locale=he -Dtests.timezone=Atlantic/Faeroe

with an error message:

17:16:06 ERROR   0.06s J2 | CompositeKeyExtractorTests.testExtractDate <<< FAILURES!
17:16:06    > Throwable #1: java.lang.IllegalArgumentException: The datetime zone id 'SystemV/YST9' is not recognised
...
17:16:06    > 	at org.joda.time.DateTimeZone.forTimeZone(DateTimeZone.java:380)
17:16:06    > 	at org.elasticsearch.xpack.sql.execution.search.extractor.CompositeKeyExtractorTests.testExtractDate(CompositeKeyExtractorTests.java:65)

Or another time zone reproduction line:

/gradlew :x-pack:plugin:sql:test -Dtests.seed=A303E5CA896B87E9 -Dtests.class=org.elasticsearch.xpack.sql.execution.search.extractor.CompositeKeyExtractorTests -Dtests.method="testExtractDate" -Dtests.security.manager=true -Dtests.locale=id-ID -Dtests.timezone=Asia/Dhaka

with an error:
Throwable #1: java.lang.IllegalArgumentException: The datetime zone id 'SystemV/AST4' is not recognised
@mayya-sharipova mayya-sharipova added the :Analytics/SQL SQL querying label Apr 25, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@mayya-sharipova
Copy link
Contributor Author

AwaitsFix was applied for this test on 6.3

@colings86 colings86 added the >test Issues or PRs that are addressing/adding tests label Apr 26, 2018
@cbuescher
Copy link
Member

I'm looking into this. there seems to be some uncovered corner cases when converting from java TimeZone back to joda DateTimeZone.

@cbuescher cbuescher self-assigned this Apr 26, 2018
@cbuescher
Copy link
Member

We randomly test with all java.util.TimeZone ids, some of which jodas DateTimeZone.forTimeZone does not recognize. Fortunately those are very odd ones so I guess we can just exclude them from the set of allowed random timezones in the test. I ran the conversion for all java.util.TimeZone ids and those are the ones that don't convert:

The datetime zone id 'SystemV/AST4' is not recognised
The datetime zone id 'SystemV/AST4ADT' is not recognised
The datetime zone id 'SystemV/CST6' is not recognised
The datetime zone id 'SystemV/CST6CDT' is not recognised
The datetime zone id 'SystemV/EST5' is not recognised
The datetime zone id 'SystemV/EST5EDT' is not recognised
The datetime zone id 'SystemV/HST10' is not recognised
The datetime zone id 'SystemV/MST7' is not recognised
The datetime zone id 'SystemV/MST7MDT' is not recognised
The datetime zone id 'SystemV/PST8' is not recognised
The datetime zone id 'SystemV/PST8PDT' is not recognised
The datetime zone id 'SystemV/YST9' is not recognised
The datetime zone id 'SystemV/YST9YDT' is not recognised

I will open a PR that excluded those.

@cbuescher
Copy link
Member

For reference: https://github.com/JodaOrg/joda-time/blob/master/src/main/java/org/joda/time/tz/src/systemv

@costin
Copy link
Member

costin commented Apr 26, 2018 •
edited
Loading

This is an old issue that @imotov (currently on holidays) looked at; as @cbuescher is spot on with his assessment.

cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Apr 26, 2018
Fix edge cases in CompositeKeyExtractorTests
3c112fd 
Currently the test picks random java.util.TimeZone ids in some places.
Internally we still need to convert back to joda DateTimeZone by id
occassionally (e.g. when serializing to pre 6.3 versions). There are
some deprecated "SystemV/*" time zones that Jodas DateTimeZone refuses
to convert. This change excludes those rare cases from the set of
allowed random time zones. It would be quiet odd for them to appear in
practice.

Closes elastic#30156
@cbuescher cbuescher mentioned this issue Apr 26, 2018
Merged
dakrone pushed a commit to dakrone/elasticsearch that referenced this issue Apr 26, 2018
Fix edge cases in CompositeKeyExtractorTests (elastic#30175)
d0416c7 
Currently the test picks random java.util.TimeZone ids in some places.
Internally we still need to convert back to joda DateTimeZone by id
occassionally (e.g. when serializing to pre 6.3 versions). There are
some deprecated "SystemV/*" time zones that Jodas DateTimeZone refuses
to convert. This change excludes those rare cases from the set of
allowed random time zones. It would be quiet odd for them to appear in
practice.

Closes elastic#30156
cbuescher pushed a commit that referenced this issue Apr 26, 2018
Fix edge cases in CompositeKeyExtractorTests (#30175)
eb1b9df 
Currently the test picks random java.util.TimeZone ids in some places.
Internally we still need to convert back to joda DateTimeZone by id
occassionally (e.g. when serializing to pre 6.3 versions). There are
some deprecated "SystemV/*" time zones that Jodas DateTimeZone refuses
to convert. This change excludes those rare cases from the set of
allowed random time zones. It would be quiet odd for them to appear in
practice.

Closes #30156
costin pushed a commit that referenced this issue Apr 26, 2018
@costin
Fix edge cases in CompositeKeyExtractorTests (#30175)
982f7d7 
Currently the test picks random java.util.TimeZone ids in some places.
Internally we still need to convert back to joda DateTimeZone by id
occassionally (e.g. when serializing to pre 6.3 versions). There are
some deprecated "SystemV/*" time zones that Jodas DateTimeZone refuses
to convert. This change excludes those rare cases from the set of
allowed random time zones. It would be quiet odd for them to appear in
practice.

Closes #30156
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cbuescher cbuescher

Labels
:Analytics/SQL SQL querying >test Issues or PRs that are addressing/adding tests v6.4.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@costin @colings86 @bleskes @mayya-sharipova @cbuescher @elasticmachine