honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Fix DATETIME_PARSE behaviour regarding timezones (#56158) (#56182) 

Previously, when the timezone was missing from the datetime string
and the pattern, UTC was used, instead of the session defined timezone.
Moreover, if a timezone was included in the datetime string and the
pattern then this timezone was used. To have a consistent behaviour
the resulting datetime will always be converted to the session defined
timezone, e.g.:
```
SELECT DATETIME_PARSE('2020-05-04 10:20:30.123 +02:00', 'HH:mm:ss dd/MM/uuuu VV') AS datetime;
```
with `time_zone` set to `-03:00` will result in
```
2020-05-04T05:20:40.123-03:00
```

Follows: #54960
(cherry picked from commit 8810ed03a209cc8fe1bad309a81e85b56a39da27)
This commit is contained in:
Marios Trivyzas 2020-05-05 12:08:39 +02:00 committed by GitHub
parent f717830563
commit 363e994171
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 108 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/reference/sql/endpoints/jdbc.asciidoc
View File

@ -72,7 +72,7 @@ The driver recognized the following properties:
[[jdbc-cfg]]
[float]
===== Essential
[[jdbc-cfg-timezone]]
`timezone` (default JVM timezone)::
Timezone used by the driver _per connection_ indicated by its `ID`.
*Highly* recommended to set it (to, say, `UTC`) as the JVM timezone can vary, is global for the entire JVM and can't be changed easily when running under a security manager.

2
docs/reference/sql/endpoints/rest.asciidoc
View File

@ -529,7 +529,7 @@ s|Description
|45s
|The timeout before a pagination request fails.
|time_zone
|[[sql-rest-fields-timezone]]time_zone
|`Z` (or `UTC`)
|Time-zone in ISO 8601 used for executing the query on the server.
More information available https://docs.oracle.com/javase/8/docs/api/java/time/ZoneId.html[here].

15
docs/reference/sql/functions/date-time.asciidoc
View File

@ -469,9 +469,6 @@ format pattern used is the one from
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/time/format/DateTimeFormatter.html[`java.time.format.DateTimeFormatter`].
If any of the two arguments is `null` or an empty string `null` is returned.
[NOTE]
If timezone is not specified in the datetime string expression and the parsing pattern, the resulting `datetime` will
be in `UTC` timezone.
[NOTE]
If the parsing pattern contains only date or only time units (e.g. 'dd/MM/uuuu', 'HH:mm:ss', etc.) an error is returned
@ -487,6 +484,18 @@ include-tagged::{sql-specs}/docs/docs.csv-spec[dateTimeParse1]
include-tagged::{sql-specs}/docs/docs.csv-spec[dateTimeParse2]
--------------------------------------------------
[NOTE]
====
If timezone is not specified in the datetime string expression and the parsing pattern, the resulting `datetime` will have the
time zone specified by the user through the <<sql-rest-fields-timezone,`time_zone`>>/<<jdbc-cfg-timezone,`timezone`>> REST/driver parameters
with no conversion applied.
[source, sql]
--------------------------------------------------
include-tagged::{sql-specs}/docs/docs.csv-spec[dateTimeParse3]
--------------------------------------------------
====
[[sql-functions-datetime-part]]
==== `DATE_PART/DATEPART`

14
x-pack/plugin/sql/qa/src/main/resources/docs/docs.csv-spec
View File

@ -2768,6 +2768,20 @@ SELECT DATETIME_PARSE('10:20:30 07/04/2020 Europe/Berlin', 'HH:mm:ss dd/MM/uuuu
// end::dateTimeParse2
;
dateTimeParse3-Ignore
schema::datetime:ts
// tag::dateTimeParse3
{
"query" : "SELECT DATETIME_PARSE('10:20:30 07/04/2020', 'HH:mm:ss dd/MM/uuuu') AS \"datetime\"",
"time_zone" : "Europe/Athens"
}
datetime
------------------------------------
2020-04-07T08:20:30.000Europe/Athens
// end::dateTimeParse3
;
datePartDateTimeYears
// tag::datePartDateTimeYears
SELECT DATE_PART('year', '2019-09-22T11:22:33.123Z'::datetime) AS "years";

13
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeParse.java
View File

@ -17,12 +17,11 @@ import org.elasticsearch.xpack.ql.type.DataTypes;
import java.time.ZoneId;
import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isString;
import static org.elasticsearch.xpack.ql.type.DateUtils.UTC;
public class DateTimeParse extends BinaryDateTimeFunction {
public DateTimeParse(Source source, Expression timestamp, Expression pattern) {
super(source, timestamp, pattern, UTC);
public DateTimeParse(Source source, Expression timestamp, Expression pattern, ZoneId zoneId) {
super(source, timestamp, pattern, zoneId);
}
@Override
@ -45,12 +44,12 @@ public class DateTimeParse extends BinaryDateTimeFunction {
@Override
protected BinaryScalarFunction replaceChildren(Expression timestamp, Expression pattern) {
return new DateTimeParse(source(), timestamp, pattern);
return new DateTimeParse(source(), timestamp, pattern, zoneId());
}
@Override
protected NodeInfo<? extends Expression> info() {
return NodeInfo.create(this, DateTimeParse::new, left(), right());
return NodeInfo.create(this, DateTimeParse::new, left(), right(), zoneId());
}
@Override
@ -60,11 +59,11 @@ public class DateTimeParse extends BinaryDateTimeFunction {
@Override
public Object fold() {
return DateTimeParseProcessor.process(left().fold(), right().fold());
return DateTimeParseProcessor.process(left().fold(), right().fold(), zoneId());
}
@Override
protected Pipe createPipe(Pipe timestamp, Pipe pattern, ZoneId zoneId) {
return new DateTimeParsePipe(source(), this, timestamp, pattern);
return new DateTimeParsePipe(source(), this, timestamp, pattern, zoneId);
}
}

10
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeParsePipe.java
View File

@ -15,22 +15,22 @@ import java.time.ZoneId;
public class DateTimeParsePipe extends BinaryDateTimePipe {
public DateTimeParsePipe(Source source, Expression expression, Pipe left, Pipe right) {
super(source, expression, left, right, null);
public DateTimeParsePipe(Source source, Expression expression, Pipe left, Pipe right, ZoneId zoneId) {
super(source, expression, left, right, zoneId);
}
@Override
protected NodeInfo<DateTimeParsePipe> info() {
return NodeInfo.create(this, DateTimeParsePipe::new, expression(), left(), right());
return NodeInfo.create(this, DateTimeParsePipe::new, expression(), left(), right(), zoneId());
}
@Override
protected DateTimeParsePipe replaceChildren(Pipe left, Pipe right) {
return new DateTimeParsePipe(source(), expression(), left, right);
return new DateTimeParsePipe(source(), expression(), left, right, zoneId());
}
@Override
protected Processor makeProcessor(Processor left, Processor right, ZoneId zoneId) {
return new DateTimeParseProcessor(left, right);
return new DateTimeParseProcessor(left, right, zoneId);
}
}

17
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeParseProcessor.java
View File

@ -8,25 +8,24 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.xpack.ql.expression.gen.processor.Processor;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.util.DateUtils;
import java.io.IOException;
import java.time.DateTimeException;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.temporal.TemporalAccessor;
import java.util.Locale;
import java.util.Objects;
import static org.elasticsearch.xpack.ql.util.DateUtils.UTC;
public class DateTimeParseProcessor extends BinaryDateTimeProcessor {
public static final String NAME = "dtparse";
public DateTimeParseProcessor(Processor source1, Processor source2) {
super(source1, source2, null);
public DateTimeParseProcessor(Processor source1, Processor source2, ZoneId zoneId) {
super(source1, source2, zoneId);
}
public DateTimeParseProcessor(StreamInput in) throws IOException {
@ -36,7 +35,7 @@ public class DateTimeParseProcessor extends BinaryDateTimeProcessor {
/**
* Used in Painless scripting
*/
public static Object process(Object timestampStr, Object pattern) {
public static Object process(Object timestampStr, Object pattern, ZoneId zoneId) {
if (timestampStr == null || pattern == null) {
return null;
}
@ -55,9 +54,9 @@ public class DateTimeParseProcessor extends BinaryDateTimeProcessor {
TemporalAccessor ta = DateTimeFormatter.ofPattern((String) pattern, Locale.ROOT)
.parseBest((String) timestampStr, ZonedDateTime::from, LocalDateTime::from);
if (ta instanceof LocalDateTime) {
return ZonedDateTime.ofInstant((LocalDateTime) ta, ZoneOffset.UTC, UTC);
return DateUtils.atTimeZone((LocalDateTime) ta, zoneId);
} else {
return ta;
return ((ZonedDateTime) ta).withZoneSameInstant(zoneId);
}
} catch (IllegalArgumentException | DateTimeException e) {
String msg = e.getMessage();
@ -80,7 +79,7 @@ public class DateTimeParseProcessor extends BinaryDateTimeProcessor {
@Override
protected Object doProcess(Object timestamp, Object pattern) {
return process(timestamp, pattern);
return process(timestamp, pattern, zoneId());
}
@Override

2
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/whitelist/InternalSqlScriptUtils.java
View File

@ -294,7 +294,7 @@ public class InternalSqlScriptUtils extends InternalQlScriptUtils {
}
public static Object dateTimeParse(String dateField, String pattern, String tzId) {
return DateTimeParseProcessor.process(dateField, pattern);
return DateTimeParseProcessor.process(dateField, pattern, ZoneId.of(tzId));
}
public static ZonedDateTime asDateTime(Object dateTime) {

5
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java
View File

@ -17,6 +17,7 @@ import org.elasticsearch.xpack.sql.type.SqlDataTypeConverter;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.OffsetTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
@ -204,4 +205,8 @@ public final class DateUtils {
nano = nano - nano % (int) Math.pow(10, (9 - precision));
return nano;
}
public static ZonedDateTime atTimeZone(LocalDateTime ldt, ZoneId zoneId) {
return ZonedDateTime.ofInstant(ldt, zoneId.getRules().getValidOffsets(ldt).get(0), zoneId);
}
}

34
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeParsePipeTests.java
View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.function.scalar.FunctionTestUtils;
import org.elasticsearch.xpack.ql.expression.gen.pipeline.BinaryPipe;
@ -26,7 +27,12 @@ import static org.elasticsearch.xpack.ql.tree.SourceTests.randomSource;
public class DateTimeParsePipeTests extends AbstractNodeTestCase<DateTimeParsePipe, Pipe> {
public static DateTimeParsePipe randomDateTimeParsePipe() {
return (DateTimeParsePipe) new DateTimeParse(randomSource(), randomStringLiteral(), randomStringLiteral()).makePipe();
return (DateTimeParsePipe) new DateTimeParse(
randomSource(),
randomStringLiteral(),
randomStringLiteral(),
randomZone()
).makePipe();
}
@Override
@ -45,12 +51,12 @@ public class DateTimeParsePipeTests extends AbstractNodeTestCase<DateTimeParsePi
DateTimeParsePipe b1 = randomInstance();
Expression newExpression = randomValueOtherThan(b1.expression(), this::randomDateTimeParsePipeExpression);
DateTimeParsePipe newB = new DateTimeParsePipe(b1.source(), newExpression, b1.left(), b1.right());
DateTimeParsePipe newB = new DateTimeParsePipe(b1.source(), newExpression, b1.left(), b1.right(), b1.zoneId());
assertEquals(newB, b1.transformPropertiesOnly(v -> Objects.equals(v, b1.expression()) ? newExpression : v, Expression.class));
DateTimeParsePipe b2 = randomInstance();
Source newLoc = randomValueOtherThan(b2.source(), SourceTests::randomSource);
newB = new DateTimeParsePipe(newLoc, b2.expression(), b2.left(), b2.right());
newB = new DateTimeParsePipe(newLoc, b2.expression(), b2.left(), b2.right(), b2.zoneId());
assertEquals(newB, b2.transformPropertiesOnly(v -> Objects.equals(v, b2.source()) ? newLoc : v, Source.class));
}
@ -59,7 +65,7 @@ public class DateTimeParsePipeTests extends AbstractNodeTestCase<DateTimeParsePi
DateTimeParsePipe b = randomInstance();
Pipe newLeft = pipe(((Expression) randomValueOtherThan(b.left(), FunctionTestUtils::randomDatetimeLiteral)));
Pipe newRight = pipe(((Expression) randomValueOtherThan(b.right(), FunctionTestUtils::randomStringLiteral)));
DateTimeParsePipe newB = new DateTimeParsePipe(b.source(), b.expression(), b.left(), b.right());
DateTimeParsePipe newB = new DateTimeParsePipe(b.source(), b.expression(), b.left(), b.right(), b.zoneId());
BinaryPipe transformed = newB.replaceChildren(newLeft, b.right());
assertEquals(transformed.left(), newLeft);
@ -88,7 +94,8 @@ public class DateTimeParsePipeTests extends AbstractNodeTestCase<DateTimeParsePi
f.source(),
f.expression(),
pipe(((Expression) randomValueOtherThan(f.left(), FunctionTestUtils::randomDatetimeLiteral))),
f.right()
f.right(),
f.zoneId()
)
);
randoms.add(
@ -96,7 +103,17 @@ public class DateTimeParsePipeTests extends AbstractNodeTestCase<DateTimeParsePi
f.source(),
f.expression(),
f.left(),
pipe(((Expression) randomValueOtherThan(f.right(), FunctionTestUtils::randomStringLiteral)))
pipe(((Expression) randomValueOtherThan(f.right(), FunctionTestUtils::randomStringLiteral))),
f.zoneId()
)
);
randoms.add(
f -> new DateTimeParsePipe(
f.source(),
f.expression(),
f.left(),
f.right(),
randomValueOtherThan(f.zoneId(), ESTestCase::randomZone)
)
);
randoms.add(
@ -104,7 +121,8 @@ public class DateTimeParsePipeTests extends AbstractNodeTestCase<DateTimeParsePi
f.source(),
f.expression(),
pipe(((Expression) randomValueOtherThan(f.left(), FunctionTestUtils::randomDatetimeLiteral))),
pipe(((Expression) randomValueOtherThan(f.right(), FunctionTestUtils::randomStringLiteral)))
pipe(((Expression) randomValueOtherThan(f.right(), FunctionTestUtils::randomStringLiteral))),
randomValueOtherThan(f.zoneId(), ESTestCase::randomZone)
)
);
@ -113,6 +131,6 @@ public class DateTimeParsePipeTests extends AbstractNodeTestCase<DateTimeParsePi
@Override
protected DateTimeParsePipe copy(DateTimeParsePipe instance) {
return new DateTimeParsePipe(instance.source(), instance.expression(), instance.left(), instance.right());
return new DateTimeParsePipe(instance.source(), instance.expression(), instance.left(), instance.right(), instance.zoneId());
}
}

50
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeParseProcessorTests.java
View File

@ -25,7 +25,8 @@ public class DateTimeParseProcessorTests extends AbstractSqlWireSerializingTestC
public static DateTimeParseProcessor randomDateTimeParseProcessor() {
return new DateTimeParseProcessor(
new ConstantProcessor(randomRealisticUnicodeOfLengthBetween(0, 128)),
new ConstantProcessor(randomRealisticUnicodeOfLengthBetween(0, 128))
new ConstantProcessor(randomRealisticUnicodeOfLengthBetween(0, 128)),
randomZone()
);
}
@ -43,26 +44,27 @@ public class DateTimeParseProcessorTests extends AbstractSqlWireSerializingTestC
protected DateTimeParseProcessor mutateInstance(DateTimeParseProcessor instance) {
return new DateTimeParseProcessor(
new ConstantProcessor(ESTestCase.randomRealisticUnicodeOfLength(128)),
new ConstantProcessor(ESTestCase.randomRealisticUnicodeOfLength(128))
new ConstantProcessor(ESTestCase.randomRealisticUnicodeOfLength(128)),
randomZone()
);
}
public void testInvalidInputs() {
SqlIllegalArgumentException siae = expectThrows(
SqlIllegalArgumentException.class,
() -> new DateTimeParse(Source.EMPTY, l(10), randomStringLiteral()).makePipe().asProcessor().process(null)
() -> new DateTimeParse(Source.EMPTY, l(10), randomStringLiteral(), randomZone()).makePipe().asProcessor().process(null)
);
assertEquals("A string is required; received [10]", siae.getMessage());
siae = expectThrows(
SqlIllegalArgumentException.class,
() -> new DateTimeParse(Source.EMPTY, randomStringLiteral(), l(20)).makePipe().asProcessor().process(null)
() -> new DateTimeParse(Source.EMPTY, randomStringLiteral(), l(20), randomZone()).makePipe().asProcessor().process(null)
);
assertEquals("A string is required; received [20]", siae.getMessage());
siae = expectThrows(
SqlIllegalArgumentException.class,
() -> new DateTimeParse(Source.EMPTY, l("2020-04-07"), l("invalid")).makePipe().asProcessor().process(null)
() -> new DateTimeParse(Source.EMPTY, l("2020-04-07"), l("invalid"), randomZone()).makePipe().asProcessor().process(null)
);
assertEquals(
"Invalid date/time string [2020-04-07] or pattern [invalid] is received; Unknown pattern letter: i",
@ -71,7 +73,7 @@ public class DateTimeParseProcessorTests extends AbstractSqlWireSerializingTestC
siae = expectThrows(
SqlIllegalArgumentException.class,
() -> new DateTimeParse(Source.EMPTY, l("2020-04-07"), l("MM/dd")).makePipe().asProcessor().process(null)
() -> new DateTimeParse(Source.EMPTY, l("2020-04-07"), l("MM/dd"), randomZone()).makePipe().asProcessor().process(null)
);
assertEquals(
"Invalid date/time string [2020-04-07] or pattern [MM/dd] is received; Text '2020-04-07' could not be parsed at index 2",
@ -80,7 +82,7 @@ public class DateTimeParseProcessorTests extends AbstractSqlWireSerializingTestC
siae = expectThrows(
SqlIllegalArgumentException.class,
() -> new DateTimeParse(Source.EMPTY, l("07/05/2020"), l("dd/MM/uuuu")).makePipe().asProcessor().process(null)
() -> new DateTimeParse(Source.EMPTY, l("07/05/2020"), l("dd/MM/uuuu"), randomZone()).makePipe().asProcessor().process(null)
);
assertEquals(
"Invalid date/time string [07/05/2020] or pattern [dd/MM/uuuu] is received; Unable to convert parsed text into [datetime]",
@ -88,8 +90,8 @@ public class DateTimeParseProcessorTests extends AbstractSqlWireSerializingTestC
);
siae = expectThrows(
SqlIllegalArgumentException.class,
() -> new DateTimeParse(Source.EMPTY, l("10:20:30.123456789"), l("HH:mm:ss.SSSSSSSSS")).makePipe().asProcessor().process(null)
SqlIllegalArgumentException.class, () -> new DateTimeParse(
Source.EMPTY, l("10:20:30.123456789"), l("HH:mm:ss.SSSSSSSSS"), randomZone()).makePipe().asProcessor().process(null)
);
assertEquals(
"Invalid date/time string [10:20:30.123456789] or pattern [HH:mm:ss.SSSSSSSSS] is received; "
@ -99,30 +101,32 @@ public class DateTimeParseProcessorTests extends AbstractSqlWireSerializingTestC
}
public void testWithNulls() {
assertNull(new DateTimeParse(Source.EMPTY, randomStringLiteral(), NULL).makePipe().asProcessor().process(null));
assertNull(new DateTimeParse(Source.EMPTY, randomStringLiteral(), l("")).makePipe().asProcessor().process(null));
assertNull(new DateTimeParse(Source.EMPTY, NULL, randomStringLiteral()).makePipe().asProcessor().process(null));
assertNull(new DateTimeParse(Source.EMPTY, l(""), randomStringLiteral()).makePipe().asProcessor().process(null));
assertNull(new DateTimeParse(Source.EMPTY, randomStringLiteral(), NULL, randomZone()).makePipe().asProcessor().process(null));
assertNull(new DateTimeParse(Source.EMPTY, randomStringLiteral(), l(""), randomZone()).makePipe().asProcessor().process(null));
assertNull(new DateTimeParse(Source.EMPTY, NULL, randomStringLiteral(), randomZone()).makePipe().asProcessor().process(null));
assertNull(new DateTimeParse(Source.EMPTY, l(""), randomStringLiteral(), randomZone()).makePipe().asProcessor().process(null));
}
public void testParsing() {
ZoneId zoneId = ZoneId.of("America/Sao_Paulo");
assertEquals(
dateTime(2020, 4, 7, 10, 20, 30, 123000000),
new DateTimeParse(Source.EMPTY, l("07/04/2020 10:20:30.123"), l("dd/MM/uuuu HH:mm:ss.SSS")).makePipe()
.asProcessor()
.process(null)
);
assertEquals(
dateTime(2020, 4, 7, 10, 20, 30, 123456789, zoneId),
new DateTimeParse(Source.EMPTY, l("07/04/2020 10:20:30.123456789 America/Sao_Paulo"), l("dd/MM/uuuu HH:mm:ss.SSSSSSSSS VV"))
dateTime(2020, 4, 7, 10, 20, 30, 123000000, zoneId),
new DateTimeParse(Source.EMPTY, l("07/04/2020 10:20:30.123"), l("dd/MM/uuuu HH:mm:ss.SSS"), zoneId)
.makePipe()
.asProcessor()
.process(null)
);
assertEquals(
dateTime(2020, 4, 7, 10, 20, 30, 123456789, ZoneId.of("+05:30")),
new DateTimeParse(Source.EMPTY, l("07/04/2020 10:20:30.123456789 +05:30"), l("dd/MM/uuuu HH:mm:ss.SSSSSSSSS zz")).makePipe()
dateTime(2020, 4, 7, 5, 20, 30, 123456789, zoneId),
new DateTimeParse(Source.EMPTY, l("07/04/2020 10:20:30.123456789 Europe/Berlin"), l("dd/MM/uuuu HH:mm:ss.SSSSSSSSS VV"), zoneId)
.makePipe()
.asProcessor()
.process(null)
);
assertEquals(
dateTime(2020, 4, 7, 1, 50, 30, 123456789, zoneId),
new DateTimeParse(Source.EMPTY, l("07/04/2020 10:20:30.123456789 +05:30"), l("dd/MM/uuuu HH:mm:ss.SSSSSSSSS zz"), zoneId)
.makePipe()
.asProcessor()
.process(null)
);