Skip to content

Commit

Permalink
SQL: Move internals from Joda to java.time (elastic#35649)
Browse files Browse the repository at this point in the history
Remove/Limit usage of Joda through-out the processors and functions
Use ZonedDateTime wherever possible instead of long/tzId

Fix elastic#35633
  • Loading branch information
costin authored Nov 17, 2018
1 parent bb51cdb commit f8e333b
Show file tree
Hide file tree
Showing 39 changed files with 451 additions and 346 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.xpack.sql.proto.ColumnInfo;
import org.elasticsearch.xpack.sql.proto.DateUtils;

import java.io.IOException;
import java.time.ZonedDateTime;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
Expand All @@ -24,7 +26,7 @@ public class CliFormatter implements Writeable {
* The minimum width for any column in the formatted results.
*/
private static final int MIN_COLUMN_WIDTH = 15;

private int[] width;

/**
Expand All @@ -45,7 +47,7 @@ public CliFormatter(List<ColumnInfo> columns, List<List<Object>> rows) {
for (int i = 0; i < width.length; i++) {
// TODO are we sure toString is correct here? What about dates that come back as longs.
// Tracked by https://github.com/elastic/x-pack-elasticsearch/issues/3081
width[i] = Math.max(width[i], Objects.toString(row.get(i)).length());
width[i] = Math.max(width[i], toString(row.get(i)).length());
}
}
}
Expand Down Expand Up @@ -116,10 +118,10 @@ private String formatWithoutHeader(StringBuilder sb, List<List<Object>> rows) {
if (i > 0) {
sb.append('|');
}

// TODO are we sure toString is correct here? What about dates that come back as longs.
// Tracked by https://github.com/elastic/x-pack-elasticsearch/issues/3081
String string = Objects.toString(row.get(i));
String string = toString(row.get(i));

if (string.length() <= width[i]) {
// Pad
sb.append(string);
Expand All @@ -138,6 +140,14 @@ private String formatWithoutHeader(StringBuilder sb, List<List<Object>> rows) {
return sb.toString();
}

private static String toString(Object object) {
if (object instanceof ZonedDateTime) {
return DateUtils.toString((ZonedDateTime) object);
} else {
return Objects.toString(object);
}
}

/**
* Pick a good estimate of the buffer size needed to contain the rows.
*/
Expand All @@ -154,8 +164,12 @@ int estimateSize(int rows) {

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
CliFormatter that = (CliFormatter) o;
return Arrays.equals(width, that.width);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.xpack.sql.proto.ColumnInfo;
import org.elasticsearch.xpack.sql.proto.DateUtils;
import org.elasticsearch.xpack.sql.proto.Mode;
import org.joda.time.ReadableDateTime;

import java.io.IOException;
import java.sql.JDBCType;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -167,9 +168,17 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
* Serializes the provided value in SQL-compatible way based on the client mode
*/
public static XContentBuilder value(XContentBuilder builder, Mode mode, Object value) throws IOException {
if (Mode.isDriver(mode) && value instanceof ReadableDateTime) {
// JDBC cannot parse dates in string format
builder.value(((ReadableDateTime) value).getMillis());
if (value instanceof ZonedDateTime) {
ZonedDateTime zdt = (ZonedDateTime) value;
if (Mode.isDriver(mode)) {
// JDBC cannot parse dates in string format and ODBC can have issues with it
// so instead, use the millis since epoch (in UTC)
builder.value(zdt.toInstant().toEpochMilli());
}
// otherwise use the ISO format
else {
builder.value(DateUtils.toString(zdt));
}
} else {
builder.value(value);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.sql.proto;

import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.util.Locale;

import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE;
import static java.time.temporal.ChronoField.HOUR_OF_DAY;
import static java.time.temporal.ChronoField.MILLI_OF_SECOND;
import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;

public class DateUtils {

private static final DateTimeFormatter ISO_WITH_MILLIS = new DateTimeFormatterBuilder()
.parseCaseInsensitive()
.append(ISO_LOCAL_DATE)
.appendLiteral('T')
.appendValue(HOUR_OF_DAY, 2)
.appendLiteral(':')
.appendValue(MINUTE_OF_HOUR, 2)
.appendLiteral(':')
.appendValue(SECOND_OF_MINUTE, 2)
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
.appendOffsetId()
.toFormatter(Locale.ROOT);

private DateUtils() {}


public static String toString(ZonedDateTime dateTime) {
return dateTime.format(ISO_WITH_MILLIS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@
*/
package org.elasticsearch.xpack.sql.execution.search.extractor;

import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation.Bucket;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.querydsl.container.GroupByRef.Property;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.elasticsearch.xpack.sql.util.DateUtils;

import java.io.IOException;
import java.time.ZoneId;
import java.util.Map;
import java.util.Objects;
import java.util.TimeZone;
Expand All @@ -29,6 +28,7 @@ public class CompositeKeyExtractor implements BucketExtractor {
private final String key;
private final Property property;
private final TimeZone timeZone;
private final ZoneId zoneId;

/**
* Constructs a new <code>CompositeKeyExtractor</code> instance.
Expand All @@ -38,40 +38,29 @@ public CompositeKeyExtractor(String key, Property property, TimeZone timeZone) {
this.key = key;
this.property = property;
this.timeZone = timeZone;
this.zoneId = timeZone != null ? timeZone.toZoneId() : null;
}

CompositeKeyExtractor(StreamInput in) throws IOException {
key = in.readString();
property = in.readEnum(Property.class);
if (in.getVersion().onOrAfter(Version.V_6_3_0)) {
if (in.readBoolean()) {
timeZone = TimeZone.getTimeZone(in.readString());
} else {
timeZone = null;
}
if (in.readBoolean()) {
timeZone = TimeZone.getTimeZone(in.readString());
} else {
DateTimeZone dtz = in.readOptionalTimeZone();
if (dtz == null) {
timeZone = null;
} else {
timeZone = dtz.toTimeZone();
}
timeZone = null;
}
this.zoneId = timeZone != null ? timeZone.toZoneId() : null;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(key);
out.writeEnum(property);
if (out.getVersion().onOrAfter(Version.V_6_3_0)) {
if (timeZone == null) {
out.writeBoolean(false);
} else {
out.writeBoolean(true);
out.writeString(timeZone.getID());
}
if (timeZone == null) {
out.writeBoolean(false);
} else {
out.writeOptionalTimeZone(timeZone == null ? null : DateTimeZone.forTimeZone(timeZone));
out.writeBoolean(true);
out.writeString(timeZone.getID());
}
}

Expand Down Expand Up @@ -110,7 +99,7 @@ public Object extract(Bucket bucket) {
if (object == null) {
return object;
} else if (object instanceof Long) {
object = new DateTime(((Long) object).longValue(), DateTimeZone.forTimeZone(timeZone));
object = DateUtils.of(((Long) object).longValue(), zoneId);
} else {
throw new SqlIllegalArgumentException("Invalid date key returned: {}", object);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.util.DateUtils;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.ReadableDateTime;

import java.io.IOException;
import java.util.List;
Expand Down Expand Up @@ -136,11 +135,16 @@ private Object unwrapMultiValue(Object values) {
if (values instanceof Map) {
throw new SqlIllegalArgumentException("Objects (returned by [{}]) are not supported", fieldName);
}
if (values instanceof String && dataType == DataType.DATE) {
return new DateTime(Long.parseLong(values.toString()), DateTimeZone.UTC);
if (dataType == DataType.DATE) {
if (values instanceof String) {
return DateUtils.of(Long.parseLong(values.toString()));
}
// returned by nested types...
if (values instanceof DateTime) {
return DateUtils.of((DateTime) values);
}
}
if (values instanceof Long || values instanceof Double || values instanceof String || values instanceof Boolean
|| values instanceof ReadableDateTime) {
if (values instanceof Long || values instanceof Double || values instanceof String || values instanceof Boolean) {
return values;
}
throw new SqlIllegalArgumentException("Type {} (returned by [{}]) is not supported", values.getClass().getSimpleName(), fieldName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,22 @@
import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.joda.time.DateTime;

import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.Objects;
import java.util.TimeZone;

abstract class BaseDateTimeFunction extends UnaryScalarFunction {

private final TimeZone timeZone;
private final ZoneId zoneId;
private final String name;

BaseDateTimeFunction(Location location, Expression field, TimeZone timeZone) {
super(location, field);
this.timeZone = timeZone;
this.zoneId = timeZone != null ? timeZone.toZoneId() : null;

StringBuilder sb = new StringBuilder(super.name());
// add timezone as last argument
Expand Down Expand Up @@ -61,15 +64,15 @@ public boolean foldable() {

@Override
public Object fold() {
DateTime folded = (DateTime) field().fold();
ZonedDateTime folded = (ZonedDateTime) field().fold();
if (folded == null) {
return null;
}

return doFold(folded.getMillis(), timeZone().getID());
return doFold(folded.withZoneSameInstant(zoneId));
}

protected abstract Object doFold(long millis, String tzId);
protected abstract Object doFold(ZonedDateTime dateTime);


@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,25 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
import org.joda.time.ReadableInstant;

import java.io.IOException;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.TimeZone;

public abstract class BaseDateTimeProcessor implements Processor {

private final TimeZone timeZone;
private final ZoneId zoneId;

BaseDateTimeProcessor(TimeZone timeZone) {
this.timeZone = timeZone;
this.zoneId = timeZone.toZoneId();
}

BaseDateTimeProcessor(StreamInput in) throws IOException {
timeZone = TimeZone.getTimeZone(in.readString());
zoneId = timeZone.toZoneId();
}

@Override
Expand All @@ -37,23 +41,17 @@ TimeZone timeZone() {
}

@Override
public Object process(Object l) {
if (l == null) {
public Object process(Object input) {
if (input == null) {
return null;
}
long millis;
if (l instanceof String) {
// 6.4+
millis = Long.parseLong(l.toString());
} else if (l instanceof ReadableInstant) {
// 6.3-
millis = ((ReadableInstant) l).getMillis();
} else {
throw new SqlIllegalArgumentException("A string or a date is required; received {}", l);

if (!(input instanceof ZonedDateTime)) {
throw new SqlIllegalArgumentException("A date is required; received {}", input);
}
return doProcess(millis);

return doProcess(((ZonedDateTime) input).withZoneSameInstant(zoneId));
}

abstract Object doProcess(long millis);
}
abstract Object doProcess(ZonedDateTime dateTime);
}
Loading

0 comments on commit f8e333b

Please sign in to comment.