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

JodaCompatibleZonedDateTime in Elasticsearch 6.5.x breaks scripts with "The datetime zone id 'Z' is not recognised" #36306

Closed
centic9 opened this issue Dec 6, 2018 · 1 comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache

Comments

@centic9
Copy link
Contributor

centic9 commented Dec 6, 2018

Elasticsearch version 6.5.1

Plugins installed: []

JVM version JVM 1.8.0_191

OS version Windows 10

Description of the problem including expected versus actual behavior:

We use scripting with date functions in some code. When we try to upgrade to Elasticsearch 6.5.1, one of our unit tests fails with the following exception:

Caused by: java.lang.IllegalArgumentException: The datetime zone id 'Z' is not recognised
	at org.joda.time.DateTimeZone.forID(DateTimeZone.java:234)
	at org.elasticsearch.script.JodaCompatibleZonedDateTime.toString(JodaCompatibleZonedDateTime.java:405)
	at java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:627)
	at org.elasticsearch.painless.DefBootstrap$PIC.fallback(DefBootstrap.java:216)
	... 32 more

Steps to reproduce:

The following also reproduces this for me, it seems Joda does not know about the special "Z" timezone identifier, but Java Time uses it:

    @Test
    public void testJodaCompatibleZonedDateTime() {
        // check for an issue that we see with 6.5
        JodaCompatibleZonedDateTime dt = new JodaCompatibleZonedDateTime(Instant.EPOCH, ZoneOffset.ofTotalSeconds(0));
        assertNotNull(dt.toString("yyyy-MM-dd hh:mm", Locale.ROOT));
    }

It seems this was introduced with the Java-time-compatibility layer in commit 758a6f5 and 3046656, PR #33486

A more elaborate integration test that works at least in Elasticsearch 6.2.4, but fails with Elasticsearch 6.5.1:

  @Test
   public void testScriptDateTime() {
       index("idx", "_doc", "id1", "{ \"endTime\": 0}");
       refresh();

       final SearchRequestBuilder searchRequestBuilder = client().prepareSearch("*").
               setSize(100).
               addAggregation(AggregationBuilders.terms("script").
                       script(new Script(ScriptType.INLINE, "painless",
                               "doc['endTime'].date.toString('yyyy-MM-dd hh:mm', Locale.ROOT)", Collections.emptyMap())));
       final SearchResponse searchResponse = searchRequestBuilder.execute().actionGet();
       assertNotNull(searchResponse);
       final Terms terms = searchResponse.getAggregations().get("script");
       assertNotNull(terms.getBuckets());
       assertEquals("1970-01-01 12:00", terms.getBuckets().get(0).getKeyAsString());
       assertEquals(1, searchResponse.getHits().getTotalHits());
   }
@andrershov andrershov added the :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache label Dec 6, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

spinscale added a commit to spinscale/elasticsearch that referenced this issue Dec 6, 2018
The conversion of timezones in JodaCompatibleZonedDateTime from joda to
java time requires the use of the DateUtils class to cater for corner
cases.

Closes elastic#36306
spinscale added a commit that referenced this issue Dec 7, 2018
The conversion of timezones in JodaCompatibleZonedDateTime from joda to
java time requires the use of the DateUtils class to cater for corner
cases.

Closes #36306
spinscale added a commit that referenced this issue Dec 7, 2018
The conversion of timezones in JodaCompatibleZonedDateTime from joda to
java time requires the use of the DateUtils class to cater for corner
cases.

Closes #36306
spinscale added a commit that referenced this issue Dec 7, 2018
The conversion of timezones in JodaCompatibleZonedDateTime from joda to
java time requires the use of the DateUtils class to cater for corner
cases.

Closes #36306
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache
Projects
None yet
Development

No branches or pull requests

3 participants