Skip to content

Commit fea332c

Browse files
committed
SQL: Move internals from Joda to java.time (#35649)
Remove/Limit usage of Joda through-out the processors and functions Use ZonedDateTime wherever possible instead of long/tzId Fix #35633 (cherry picked from commit f8e333b)
1 parent c4d61f2 commit fea332c

39 files changed

+451
-346
lines changed

x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/CliFormatter.java

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99
import org.elasticsearch.common.io.stream.StreamOutput;
1010
import org.elasticsearch.common.io.stream.Writeable;
1111
import org.elasticsearch.xpack.sql.proto.ColumnInfo;
12+
import org.elasticsearch.xpack.sql.proto.DateUtils;
1213

1314
import java.io.IOException;
15+
import java.time.ZonedDateTime;
1416
import java.util.Arrays;
1517
import java.util.List;
1618
import java.util.Objects;
@@ -24,7 +26,7 @@ public class CliFormatter implements Writeable {
2426
* The minimum width for any column in the formatted results.
2527
*/
2628
private static final int MIN_COLUMN_WIDTH = 15;
27-
29+
2830
private int[] width;
2931

3032
/**
@@ -45,7 +47,7 @@ public CliFormatter(List<ColumnInfo> columns, List<List<Object>> rows) {
4547
for (int i = 0; i < width.length; i++) {
4648
// TODO are we sure toString is correct here? What about dates that come back as longs.
4749
// Tracked by https://github.com/elastic/x-pack-elasticsearch/issues/3081
48-
width[i] = Math.max(width[i], Objects.toString(row.get(i)).length());
50+
width[i] = Math.max(width[i], toString(row.get(i)).length());
4951
}
5052
}
5153
}
@@ -116,10 +118,10 @@ private String formatWithoutHeader(StringBuilder sb, List<List<Object>> rows) {
116118
if (i > 0) {
117119
sb.append('|');
118120
}
119-
120121
// TODO are we sure toString is correct here? What about dates that come back as longs.
121122
// Tracked by https://github.com/elastic/x-pack-elasticsearch/issues/3081
122-
String string = Objects.toString(row.get(i));
123+
String string = toString(row.get(i));
124+
123125
if (string.length() <= width[i]) {
124126
// Pad
125127
sb.append(string);
@@ -138,6 +140,14 @@ private String formatWithoutHeader(StringBuilder sb, List<List<Object>> rows) {
138140
return sb.toString();
139141
}
140142

143+
private static String toString(Object object) {
144+
if (object instanceof ZonedDateTime) {
145+
return DateUtils.toString((ZonedDateTime) object);
146+
} else {
147+
return Objects.toString(object);
148+
}
149+
}
150+
141151
/**
142152
* Pick a good estimate of the buffer size needed to contain the rows.
143153
*/
@@ -154,8 +164,12 @@ int estimateSize(int rows) {
154164

155165
@Override
156166
public boolean equals(Object o) {
157-
if (this == o) return true;
158-
if (o == null || getClass() != o.getClass()) return false;
167+
if (this == o) {
168+
return true;
169+
}
170+
if (o == null || getClass() != o.getClass()) {
171+
return false;
172+
}
159173
CliFormatter that = (CliFormatter) o;
160174
return Arrays.equals(width, that.width);
161175
}

x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
import org.elasticsearch.common.xcontent.ToXContentObject;
1414
import org.elasticsearch.common.xcontent.XContentBuilder;
1515
import org.elasticsearch.xpack.sql.proto.ColumnInfo;
16+
import org.elasticsearch.xpack.sql.proto.DateUtils;
1617
import org.elasticsearch.xpack.sql.proto.Mode;
17-
import org.joda.time.ReadableDateTime;
1818

1919
import java.io.IOException;
2020
import java.sql.JDBCType;
21+
import java.time.ZonedDateTime;
2122
import java.util.ArrayList;
2223
import java.util.List;
2324
import java.util.Objects;
@@ -167,9 +168,17 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
167168
* Serializes the provided value in SQL-compatible way based on the client mode
168169
*/
169170
public static XContentBuilder value(XContentBuilder builder, Mode mode, Object value) throws IOException {
170-
if (Mode.isDriver(mode) && value instanceof ReadableDateTime) {
171-
// JDBC cannot parse dates in string format
172-
builder.value(((ReadableDateTime) value).getMillis());
171+
if (value instanceof ZonedDateTime) {
172+
ZonedDateTime zdt = (ZonedDateTime) value;
173+
if (Mode.isDriver(mode)) {
174+
// JDBC cannot parse dates in string format and ODBC can have issues with it
175+
// so instead, use the millis since epoch (in UTC)
176+
builder.value(zdt.toInstant().toEpochMilli());
177+
}
178+
// otherwise use the ISO format
179+
else {
180+
builder.value(DateUtils.toString(zdt));
181+
}
173182
} else {
174183
builder.value(value);
175184
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.sql.proto;
8+
9+
import java.time.ZonedDateTime;
10+
import java.time.format.DateTimeFormatter;
11+
import java.time.format.DateTimeFormatterBuilder;
12+
import java.util.Locale;
13+
14+
import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE;
15+
import static java.time.temporal.ChronoField.HOUR_OF_DAY;
16+
import static java.time.temporal.ChronoField.MILLI_OF_SECOND;
17+
import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
18+
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
19+
20+
public class DateUtils {
21+
22+
private static final DateTimeFormatter ISO_WITH_MILLIS = new DateTimeFormatterBuilder()
23+
.parseCaseInsensitive()
24+
.append(ISO_LOCAL_DATE)
25+
.appendLiteral('T')
26+
.appendValue(HOUR_OF_DAY, 2)
27+
.appendLiteral(':')
28+
.appendValue(MINUTE_OF_HOUR, 2)
29+
.appendLiteral(':')
30+
.appendValue(SECOND_OF_MINUTE, 2)
31+
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
32+
.appendOffsetId()
33+
.toFormatter(Locale.ROOT);
34+
35+
private DateUtils() {}
36+
37+
38+
public static String toString(ZonedDateTime dateTime) {
39+
return dateTime.format(ISO_WITH_MILLIS);
40+
}
41+
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/CompositeKeyExtractor.java

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,15 @@
55
*/
66
package org.elasticsearch.xpack.sql.execution.search.extractor;
77

8-
import org.elasticsearch.Version;
98
import org.elasticsearch.common.io.stream.StreamInput;
109
import org.elasticsearch.common.io.stream.StreamOutput;
1110
import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation.Bucket;
1211
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
1312
import org.elasticsearch.xpack.sql.querydsl.container.GroupByRef.Property;
14-
import org.joda.time.DateTime;
15-
import org.joda.time.DateTimeZone;
13+
import org.elasticsearch.xpack.sql.util.DateUtils;
1614

1715
import java.io.IOException;
16+
import java.time.ZoneId;
1817
import java.util.Map;
1918
import java.util.Objects;
2019
import java.util.TimeZone;
@@ -29,6 +28,7 @@ public class CompositeKeyExtractor implements BucketExtractor {
2928
private final String key;
3029
private final Property property;
3130
private final TimeZone timeZone;
31+
private final ZoneId zoneId;
3232

3333
/**
3434
* Constructs a new <code>CompositeKeyExtractor</code> instance.
@@ -38,40 +38,29 @@ public CompositeKeyExtractor(String key, Property property, TimeZone timeZone) {
3838
this.key = key;
3939
this.property = property;
4040
this.timeZone = timeZone;
41+
this.zoneId = timeZone != null ? timeZone.toZoneId() : null;
4142
}
4243

4344
CompositeKeyExtractor(StreamInput in) throws IOException {
4445
key = in.readString();
4546
property = in.readEnum(Property.class);
46-
if (in.getVersion().onOrAfter(Version.V_6_3_0)) {
47-
if (in.readBoolean()) {
48-
timeZone = TimeZone.getTimeZone(in.readString());
49-
} else {
50-
timeZone = null;
51-
}
47+
if (in.readBoolean()) {
48+
timeZone = TimeZone.getTimeZone(in.readString());
5249
} else {
53-
DateTimeZone dtz = in.readOptionalTimeZone();
54-
if (dtz == null) {
55-
timeZone = null;
56-
} else {
57-
timeZone = dtz.toTimeZone();
58-
}
50+
timeZone = null;
5951
}
52+
this.zoneId = timeZone != null ? timeZone.toZoneId() : null;
6053
}
6154

6255
@Override
6356
public void writeTo(StreamOutput out) throws IOException {
6457
out.writeString(key);
6558
out.writeEnum(property);
66-
if (out.getVersion().onOrAfter(Version.V_6_3_0)) {
67-
if (timeZone == null) {
68-
out.writeBoolean(false);
69-
} else {
70-
out.writeBoolean(true);
71-
out.writeString(timeZone.getID());
72-
}
59+
if (timeZone == null) {
60+
out.writeBoolean(false);
7361
} else {
74-
out.writeOptionalTimeZone(timeZone == null ? null : DateTimeZone.forTimeZone(timeZone));
62+
out.writeBoolean(true);
63+
out.writeString(timeZone.getID());
7564
}
7665
}
7766

@@ -110,7 +99,7 @@ public Object extract(Bucket bucket) {
11099
if (object == null) {
111100
return object;
112101
} else if (object instanceof Long) {
113-
object = new DateTime(((Long) object).longValue(), DateTimeZone.forTimeZone(timeZone));
102+
object = DateUtils.of(((Long) object).longValue(), zoneId);
114103
} else {
115104
throw new SqlIllegalArgumentException("Invalid date key returned: {}", object);
116105
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,8 @@
1313
import org.elasticsearch.search.SearchHit;
1414
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
1515
import org.elasticsearch.xpack.sql.type.DataType;
16+
import org.elasticsearch.xpack.sql.util.DateUtils;
1617
import org.joda.time.DateTime;
17-
import org.joda.time.DateTimeZone;
18-
import org.joda.time.ReadableDateTime;
1918

2019
import java.io.IOException;
2120
import java.util.List;
@@ -136,11 +135,16 @@ private Object unwrapMultiValue(Object values) {
136135
if (values instanceof Map) {
137136
throw new SqlIllegalArgumentException("Objects (returned by [{}]) are not supported", fieldName);
138137
}
139-
if (values instanceof String && dataType == DataType.DATE) {
140-
return new DateTime(Long.parseLong(values.toString()), DateTimeZone.UTC);
138+
if (dataType == DataType.DATE) {
139+
if (values instanceof String) {
140+
return DateUtils.of(Long.parseLong(values.toString()));
141+
}
142+
// returned by nested types...
143+
if (values instanceof DateTime) {
144+
return DateUtils.of((DateTime) values);
145+
}
141146
}
142-
if (values instanceof Long || values instanceof Double || values instanceof String || values instanceof Boolean
143-
|| values instanceof ReadableDateTime) {
147+
if (values instanceof Long || values instanceof Double || values instanceof String || values instanceof Boolean) {
144148
return values;
145149
}
146150
throw new SqlIllegalArgumentException("Type {} (returned by [{}]) is not supported", values.getClass().getSimpleName(), fieldName);

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/BaseDateTimeFunction.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,22 @@
1212
import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction;
1313
import org.elasticsearch.xpack.sql.tree.Location;
1414
import org.elasticsearch.xpack.sql.tree.NodeInfo;
15-
import org.joda.time.DateTime;
1615

16+
import java.time.ZoneId;
17+
import java.time.ZonedDateTime;
1718
import java.util.Objects;
1819
import java.util.TimeZone;
1920

2021
abstract class BaseDateTimeFunction extends UnaryScalarFunction {
2122

2223
private final TimeZone timeZone;
24+
private final ZoneId zoneId;
2325
private final String name;
2426

2527
BaseDateTimeFunction(Location location, Expression field, TimeZone timeZone) {
2628
super(location, field);
2729
this.timeZone = timeZone;
30+
this.zoneId = timeZone != null ? timeZone.toZoneId() : null;
2831

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

6265
@Override
6366
public Object fold() {
64-
DateTime folded = (DateTime) field().fold();
67+
ZonedDateTime folded = (ZonedDateTime) field().fold();
6568
if (folded == null) {
6669
return null;
6770
}
6871

69-
return doFold(folded.getMillis(), timeZone().getID());
72+
return doFold(folded.withZoneSameInstant(zoneId));
7073
}
7174

72-
protected abstract Object doFold(long millis, String tzId);
75+
protected abstract Object doFold(ZonedDateTime dateTime);
7376

7477

7578
@Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/BaseDateTimeProcessor.java

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,25 @@
1010
import org.elasticsearch.common.io.stream.StreamOutput;
1111
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
1212
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
13-
import org.joda.time.ReadableInstant;
1413

1514
import java.io.IOException;
15+
import java.time.ZoneId;
16+
import java.time.ZonedDateTime;
1617
import java.util.TimeZone;
1718

1819
public abstract class BaseDateTimeProcessor implements Processor {
1920

2021
private final TimeZone timeZone;
22+
private final ZoneId zoneId;
2123

2224
BaseDateTimeProcessor(TimeZone timeZone) {
2325
this.timeZone = timeZone;
26+
this.zoneId = timeZone.toZoneId();
2427
}
2528

2629
BaseDateTimeProcessor(StreamInput in) throws IOException {
2730
timeZone = TimeZone.getTimeZone(in.readString());
31+
zoneId = timeZone.toZoneId();
2832
}
2933

3034
@Override
@@ -37,23 +41,17 @@ TimeZone timeZone() {
3741
}
3842

3943
@Override
40-
public Object process(Object l) {
41-
if (l == null) {
44+
public Object process(Object input) {
45+
if (input == null) {
4246
return null;
4347
}
44-
long millis;
45-
if (l instanceof String) {
46-
// 6.4+
47-
millis = Long.parseLong(l.toString());
48-
} else if (l instanceof ReadableInstant) {
49-
// 6.3-
50-
millis = ((ReadableInstant) l).getMillis();
51-
} else {
52-
throw new SqlIllegalArgumentException("A string or a date is required; received {}", l);
48+
49+
if (!(input instanceof ZonedDateTime)) {
50+
throw new SqlIllegalArgumentException("A date is required; received {}", input);
5351
}
54-
55-
return doProcess(millis);
52+
53+
return doProcess(((ZonedDateTime) input).withZoneSameInstant(zoneId));
5654
}
5755

58-
abstract Object doProcess(long millis);
59-
}
56+
abstract Object doProcess(ZonedDateTime dateTime);
57+
}

0 commit comments

Comments
 (0)