Skip to content

Commit

Permalink
Scripting: Conditionally use java time api in scripting (#31441)
Browse files Browse the repository at this point in the history
This commit adds a boolean system property, `es.scripting.use_java_time`,
which controls the concrete return type used by doc values within
scripts. The return type of accessing doc values for a date field is
changed to Object, essentially duck typing the type to allow
co-existence during the transition from joda time to java time.
  • Loading branch information
rjernst authored Aug 1, 2018
1 parent 9fb790d commit 478f6d6
Show file tree
Hide file tree
Showing 14 changed files with 175 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -777,11 +777,16 @@ class BuildPlugin implements Plugin<Project> {
systemProperty property.getKey(), property.getValue()
}
}

// TODO: remove this once joda time is removed from scriptin in 7.0
systemProperty 'es.scripting.use_java_time', 'true'

// Set the system keystore/truststore password if we're running tests in a FIPS-140 JVM
if (project.inFipsJvm) {
systemProperty 'javax.net.ssl.trustStorePassword', 'password'
systemProperty 'javax.net.ssl.keyStorePassword', 'password'
}

boolean assertionsEnabled = Boolean.parseBoolean(System.getProperty('tests.asserts', 'true'))
enableSystemAssertions assertionsEnabled
enableAssertions assertionsEnabled
Expand Down
3 changes: 3 additions & 0 deletions docs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ integTestCluster {
extraConfigFile 'hunspell/en_US/en_US.dic', '../server/src/test/resources/indices/analyze/conf_dir/hunspell/en_US/en_US.dic'
// Whitelist reindexing from the local node so we can test it.
setting 'reindex.remote.whitelist', '127.0.0.1:*'

// TODO: remove this for 7.0, this exists to allow the doc examples in 6.x to continue using the defaults
systemProperty 'es.scripting.use_java_time', 'false'
}

// remove when https://github.com/elastic/elasticsearch/issues/31305 is fixed
Expand Down
7 changes: 6 additions & 1 deletion docs/painless/painless-getting-started.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ POST hockey/player/1/_update
==== Dates

Date fields are exposed as
`ReadableDateTime`
`ReadableDateTime` or
so they support methods like
`getYear`,
and `getDayOfWeek`.
Expand All @@ -220,6 +220,11 @@ GET hockey/_search
}
----------------------------------------------------------------
// CONSOLE
// TEST[warning:The joda time api for doc values is deprecated. Use -Des.scripting.use_java_time=true to use the java time api for date field doc values]

NOTE: Date fields are changing in 7.0 to be exposed as `ZonedDateTime`
from Java 8's time API. To switch to this functionality early,
add `-Des.scripting.use_java_time=true` to `jvm.options`.

[float]
[[modules-scripting-painless-regex]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ POST /sales/_search?size=0
--------------------------------------------------
// CONSOLE
// TEST[setup:sales]
// TEST[warning:The joda time api for doc values is deprecated. Use -Des.scripting.use_java_time=true to use the java time api for date field doc values]

Response:

Expand Down
3 changes: 1 addition & 2 deletions modules/lang-painless/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@
* under the License.
*/



esplugin {
description 'An easy, safe and fast scripting language for Elasticsearch'
classname 'org.elasticsearch.painless.PainlessPlugin'
}

integTestCluster {
module project.project(':modules:mapper-extras')
systemProperty 'es.scripting.use_java_time', 'true'
}

dependencies {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ class org.elasticsearch.index.fielddata.ScriptDocValues$Longs {
}

class org.elasticsearch.index.fielddata.ScriptDocValues$Dates {
org.joda.time.ReadableDateTime get(int)
org.joda.time.ReadableDateTime getValue()
Object get(int)
Object getValue()
List getValues()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ setup:
script_fields:
bar:
script:
source: "doc.date.value.dayOfWeek"
source: "doc.date.value.dayOfWeek.value"

- match: { hits.hits.0.fields.bar.0: 7}

Expand All @@ -123,7 +123,7 @@ setup:
source: >
StringBuilder b = new StringBuilder();
for (def date : doc.dates) {
b.append(" ").append(date.getDayOfWeek());
b.append(" ").append(date.getDayOfWeek().value);
}
return b.toString().trim()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ setup:
field:
script:
source: "doc.date.get(0)"
- match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12.000Z' }
- match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12Z' }

- do:
search:
Expand All @@ -104,7 +104,7 @@ setup:
field:
script:
source: "doc.date.value"
- match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12.000Z' }
- match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12Z' }

---
"geo_point":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.index.fielddata;


import org.apache.lucene.index.SortedNumericDocValues;
import org.apache.lucene.util.ArrayUtil;
import org.apache.lucene.util.BytesRef;
Expand All @@ -29,18 +28,23 @@
import org.elasticsearch.common.geo.GeoUtils;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.ESLoggerFactory;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.MutableDateTime;
import org.joda.time.ReadableDateTime;

import java.io.IOException;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.AbstractList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.function.Consumer;
import java.util.function.UnaryOperator;

import static org.elasticsearch.common.Booleans.parseBoolean;

/**
* Script level doc values, the assumption is that any implementation will
Expand All @@ -52,6 +56,7 @@
* values form multiple documents.
*/
public abstract class ScriptDocValues<T> extends AbstractList<T> {

/**
* Set the current doc ID.
*/
Expand Down Expand Up @@ -142,31 +147,55 @@ public int size() {
}
}

public static final class Dates extends ScriptDocValues<ReadableDateTime> {
protected static final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger(Dates.class));
public static final class Dates extends ScriptDocValues<Object> {

private static final ReadableDateTime EPOCH = new DateTime(0, DateTimeZone.UTC);
/** Whether scripts should expose dates as java time objects instead of joda time. */
private static final boolean USE_JAVA_TIME = parseBoolean(System.getProperty("es.scripting.use_java_time"), false);

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger(Dates.class));

private final SortedNumericDocValues in;

/**
* Method call to add deprecation message. Normally this is
* {@link #deprecationLogger} but tests override.
*/
private final Consumer<String> deprecationCallback;

/**
* Values wrapped in {@link MutableDateTime}. Null by default an allocated on first usage so we allocate a reasonably size. We keep
* this array so we don't have allocate new {@link MutableDateTime}s on every usage. Instead we reuse them for every document.
* Whether java time or joda time should be used. This is normally {@link #USE_JAVA_TIME} but tests override it.
*/
private MutableDateTime[] dates;
private final boolean useJavaTime;

/**
* Values wrapped in a date time object. The concrete type depends on the system property {@code es.scripting.use_java_time}.
* When that system property is {@code false}, the date time objects are of type {@link MutableDateTime}. When the system
* property is {@code true}, the date time objects are of type {@link java.time.ZonedDateTime}.
*/
private Object[] dates;
private int count;

/**
* Standard constructor.
*/
public Dates(SortedNumericDocValues in) {
this(in, message -> deprecationLogger.deprecatedAndMaybeLog("scripting_joda_time_deprecation", message), USE_JAVA_TIME);
}

/**
* Constructor for testing with a deprecation callback.
*/
Dates(SortedNumericDocValues in, Consumer<String> deprecationCallback, boolean useJavaTime) {
this.in = in;
this.deprecationCallback = deprecationCallback;
this.useJavaTime = useJavaTime;
}

/**
* Fetch the first field value or 0 millis after epoch if there are no
* in.
*/
public ReadableDateTime getValue() {
public Object getValue() {
if (count == 0) {
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
Expand All @@ -175,7 +204,7 @@ public ReadableDateTime getValue() {
}

@Override
public ReadableDateTime get(int index) {
public Object get(int index) {
if (index >= count) {
throw new IndexOutOfBoundsException(
"attempted to fetch the [" + index + "] date when there are only ["
Expand Down Expand Up @@ -206,31 +235,42 @@ void refreshArray() throws IOException {
if (count == 0) {
return;
}
if (dates == null) {
// Happens for the document. We delay allocating dates so we can allocate it with a reasonable size.
dates = new MutableDateTime[count];
for (int i = 0; i < dates.length; i++) {
dates[i] = new MutableDateTime(in.nextValue(), DateTimeZone.UTC);
if (useJavaTime) {
if (dates == null || count > dates.length) {
// Happens for the document. We delay allocating dates so we can allocate it with a reasonable size.
dates = new ZonedDateTime[count];
}
return;
}
if (count > dates.length) {
// Happens when we move to a new document and it has more dates than any documents before it.
MutableDateTime[] backup = dates;
dates = new MutableDateTime[count];
System.arraycopy(backup, 0, dates, 0, backup.length);
for (int i = 0; i < backup.length; i++) {
dates[i].setMillis(in.nextValue());
for (int i = 0; i < count; ++i) {
dates[i] = ZonedDateTime.ofInstant(Instant.ofEpochMilli(in.nextValue()), ZoneOffset.UTC);
}
for (int i = backup.length; i < dates.length; i++) {
} else {
deprecated("The joda time api for doc values is deprecated. Use -Des.scripting.use_java_time=true" +
" to use the java time api for date field doc values");
if (dates == null || count > dates.length) {
// Happens for the document. We delay allocating dates so we can allocate it with a reasonable size.
dates = new MutableDateTime[count];
}
for (int i = 0; i < count; i++) {
dates[i] = new MutableDateTime(in.nextValue(), DateTimeZone.UTC);
}
return;
}
for (int i = 0; i < count; i++) {
dates[i] = new MutableDateTime(in.nextValue(), DateTimeZone.UTC);
}
}

/**
* Log a deprecation log, with the server's permissions, not the permissions of the
* script calling this method. We need to do this to prevent errors when rolling
* the log file.
*/
private void deprecated(String message) {
// Intentionally not calling SpecialPermission.check because this is supposed to be called by scripts
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
deprecationCallback.accept(message);
return null;
}
});
}
}

public static final class Doubles extends ScriptDocValues<Double> {
Expand Down
11 changes: 5 additions & 6 deletions server/src/main/java/org/elasticsearch/script/ScriptModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

package org.elasticsearch.script;

import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.ScriptPlugin;
import org.elasticsearch.search.aggregations.pipeline.movfn.MovingFunctionScript;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -27,12 +32,6 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.ScriptPlugin;
import org.elasticsearch.search.aggregations.pipeline.movfn.MovingFunctionScript;


/**
* Manages building {@link ScriptService}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import java.io.IOException;
import java.lang.reflect.Array;
import java.time.ZonedDateTime;
import java.util.Collection;

/**
Expand Down Expand Up @@ -54,6 +55,9 @@ public boolean advanceExact(int target) throws IOException {
} else if (value instanceof ReadableInstant) {
resize(1);
values[0] = ((ReadableInstant) value).getMillis();
} else if (value instanceof ZonedDateTime) {
resize(1);
values[0] = ((ZonedDateTime) value).toInstant().toEpochMilli();
} else if (value.getClass().isArray()) {
int length = Array.getLength(value);
if (length == 0) {
Expand Down Expand Up @@ -89,6 +93,8 @@ private static double toDoubleValue(Object o) {
} else if (o instanceof ReadableInstant) {
// Dates are exposed in scripts as ReadableDateTimes but aggregations want them to be numeric
return ((ReadableInstant) o).getMillis();
} else if (o instanceof ZonedDateTime) {
return ((ZonedDateTime) o).toInstant().toEpochMilli();
} else if (o instanceof Boolean) {
// We do expose boolean fields as boolean in scripts, however aggregations still expect
// that scripts return the same internal representation as regular fields, so boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import java.io.IOException;
import java.lang.reflect.Array;
import java.time.ZonedDateTime;
import java.util.Collection;
import java.util.Iterator;

Expand Down Expand Up @@ -91,6 +92,8 @@ private static long toLongValue(Object o) {
} else if (o instanceof ReadableInstant) {
// Dates are exposed in scripts as ReadableDateTimes but aggregations want them to be numeric
return ((ReadableInstant) o).getMillis();
} else if (o instanceof ZonedDateTime) {
return ((ZonedDateTime) o).toInstant().toEpochMilli();
} else if (o instanceof Boolean) {
// We do expose boolean fields as boolean in scripts, however aggregations still expect
// that scripts return the same internal representation as regular fields, so boolean
Expand Down
Loading

0 comments on commit 478f6d6

Please sign in to comment.