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.

Sign up for GitHub

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

ESQL: allow DATE_PARSE to read the timezones #118603

Merged
merged 10 commits into from
Dec 19, 2024
Merged

ESQL: allow DATE_PARSE to read the timezones #118603

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b798118
Allow DATE_PARSE to read the TZ
bpintea Dec 12, 2024
22c812a
Drop TZ param
bpintea Dec 12, 2024
cd85b6a
more tests
bpintea Dec 12, 2024
6e7273b
drop redundant import
bpintea Dec 12, 2024
4d44e80
Update docs/changelog/118603.yaml
bpintea Dec 12, 2024
84d0e93
add cap guards
bpintea Dec 12, 2024
01327a1
propagate dropping of TZ to unit test
bpintea Dec 12, 2024
8bdd6c7
Merge branch 'main' into fix/date_parse_tz
bpintea Dec 12, 2024
babdb3e
Review comments
bpintea Dec 19, 2024
083857f
Merge branch 'main' into fix/date_parse_tz
bpintea Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/118603.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 118603
summary: Allow DATE_PARSE to read the timezones
area: ES|QL
type: bug
issues:
- 117680
46 changes: 46 additions & 0 deletions x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,52 @@ b:datetime
null
;

evalDateParseWithTimezone
required_capability: date_parse_tz
row s = "12/Jul/2022:10:24:10 +0900" | eval d = date_parse("dd/MMM/yyyy:HH:mm:ss Z", s);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it would be good to have at least one test that crosses midnight (we could complicate it more with months/years/leap day but IMHO it's overkill)


s:keyword | d:datetime
12/Jul/2022:10:24:10 +0900 | 2022-07-12T01:24:10.000Z
;

evalDateParseWithTimezone2
required_capability: date_parse_tz
row s1 = "12/Jul/2022:10:24:10 +0900", s2 = "2022/12/07 09:24:10 +0800"
| eval d1 = date_parse("dd/MMM/yyyy:HH:mm:ss Z", s1), d2 = date_parse("yyyy/dd/MM HH:mm:ss Z", s2)
| eval eq = d1 == d2
| keep d1, eq
;

d1:datetime | eq:boolean
2022-07-12T01:24:10.000Z | true
;

evalDateParseWithAndWithoutTimezone
required_capability: date_parse_tz
row s = "2022/12/07 09:24:10", format="yyyy/dd/MM HH:mm:ss"
| eval no_tz = date_parse(format, s)
| eval with_tz = date_parse(concat(format, " Z"), concat(s, " +0900"))
| keep s, no_tz, with_tz
;

s:keyword | no_tz:datetime | with_tz:datetime
2022/12/07 09:24:10 | 2022-07-12T09:24:10.000Z | 2022-07-12T00:24:10.000Z
;

evalDateParseWithOtherTimezoneSpecifiers
required_capability: date_parse_tz
row s = "2022/12/07 09:24:10", format="yyyy/dd/MM HH:mm:ss"
| eval with_tz1 = date_parse(concat(format, " Z"), concat(s, " +0900"))
| eval with_tz2 = date_parse(concat(format, " x"), concat(s, " +09"))
| eval with_tz3 = date_parse(concat(format, " X"), concat(s, " +0900"))
| eval with_tz4 = date_parse(concat(format, " O"), concat(s, " GMT+9"))
| keep s, with_tz*
;

s:keyword | with_tz1:datetime | with_tz2:datetime | with_tz3:datetime | with_tz4:datetime
2022/12/07 09:24:10 | 2022-07-12T00:24:10.000Z | 2022-07-12T00:24:10.000Z | 2022-07-12T00:24:10.000Z | 2022-07-12T00:24:10.000Z
;

evalDateParseDynamic
from employees | where emp_no == 10039 or emp_no == 10040 | sort emp_no
| eval birth_date_string = date_format("yyyy-MM-dd", birth_date)
Expand Down
21 changes: 7 additions & 14 deletions ...ated/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateParseEvaluator.java
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,11 @@ public enum Cap {
*/
DATE_NANOS_AGGREGATIONS(),

/**
* DATE_PARSE supports reading timezones
*/
DATE_PARSE_TZ(),

/**
* Support for datetime in least and greatest functions
*/
Expand Down
15 changes: 6 additions & 9 deletions ...src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateParse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,12 @@
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;

import java.io.IOException;
import java.time.ZoneId;
import java.util.List;

import static org.elasticsearch.common.time.DateFormatter.forPattern;
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST;
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND;
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isString;
import static org.elasticsearch.xpack.esql.core.util.DateUtils.UTC;
import static org.elasticsearch.xpack.esql.expression.EsqlTypeResolutions.isStringAndExact;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.DEFAULT_DATE_TIME_FORMATTER;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToLong;
Expand Down Expand Up @@ -130,13 +128,12 @@ public static long process(BytesRef val, @Fixed DateFormatter formatter) throws
}

@Evaluator(warnExceptions = { IllegalArgumentException.class })
static long process(BytesRef val, BytesRef formatter, @Fixed ZoneId zoneId) throws IllegalArgumentException {
return dateTimeToLong(val.utf8ToString(), toFormatter(formatter, zoneId));
static long process(BytesRef val, BytesRef formatter) throws IllegalArgumentException {
return dateTimeToLong(val.utf8ToString(), toFormatter(formatter));
}

@Override
public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
ZoneId zone = UTC; // TODO session timezone?
ExpressionEvaluator.Factory fieldEvaluator = toEvaluator.apply(field);
if (format == null) {
return new DateParseConstantEvaluator.Factory(source(), fieldEvaluator, DEFAULT_DATE_TIME_FORMATTER);
Expand All @@ -146,18 +143,18 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
}
if (format.foldable()) {
try {
DateFormatter formatter = toFormatter(format.fold(), zone);
DateFormatter formatter = toFormatter(format.fold());
return new DateParseConstantEvaluator.Factory(source(), fieldEvaluator, formatter);
} catch (IllegalArgumentException e) {
throw new InvalidArgumentException(e, "invalid date pattern for [{}]: {}", sourceText(), e.getMessage());
}
}
ExpressionEvaluator.Factory formatEvaluator = toEvaluator.apply(format);
return new DateParseEvaluator.Factory(source(), fieldEvaluator, formatEvaluator, zone);
return new DateParseEvaluator.Factory(source(), fieldEvaluator, formatEvaluator);
}

private static DateFormatter toFormatter(Object format, ZoneId zone) {
return forPattern(((BytesRef) format).utf8ToString()).withZone(zone);
private static DateFormatter toFormatter(Object format) {
return forPattern(((BytesRef) format).utf8ToString());
}

@Override
Expand Down
25 changes: 19 additions & 6 deletions ...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateParseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,24 @@ public static Iterable<Object[]> parameters() {
new TestCaseSupplier.TypedData(new BytesRef("yyyy-MM-dd"), DataType.KEYWORD, "first"),
new TestCaseSupplier.TypedData(new BytesRef("2023-05-05"), DataType.KEYWORD, "second")
),
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0], zoneId=Z]",
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0]]",
DataType.DATETIME,
equalTo(1683244800000L)
)
),
new TestCaseSupplier(
"Timezoned Case",
List.of(DataType.KEYWORD, DataType.KEYWORD),
() -> new TestCaseSupplier.TestCase(
List.of(
new TestCaseSupplier.TypedData(new BytesRef("dd/MMM/yyyy:HH:mm:ss Z"), DataType.KEYWORD, "first"),
new TestCaseSupplier.TypedData(new BytesRef("12/Jul/2022:10:24:10 +0900"), DataType.KEYWORD, "second")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we could randomize this a bit (only on the hour of course, no need to complicate it too much)

),
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0]]",
DataType.DATETIME,
equalTo(1657589050000L)
)
),
new TestCaseSupplier(
"With Text",
List.of(DataType.KEYWORD, DataType.TEXT),
Expand All @@ -59,7 +72,7 @@ public static Iterable<Object[]> parameters() {
new TestCaseSupplier.TypedData(new BytesRef("yyyy-MM-dd"), DataType.KEYWORD, "first"),
new TestCaseSupplier.TypedData(new BytesRef("2023-05-05"), DataType.TEXT, "second")
),
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0], zoneId=Z]",
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0]]",
DataType.DATETIME,
equalTo(1683244800000L)
)
Expand All @@ -72,7 +85,7 @@ public static Iterable<Object[]> parameters() {
new TestCaseSupplier.TypedData(new BytesRef("yyyy-MM-dd"), DataType.TEXT, "first"),
new TestCaseSupplier.TypedData(new BytesRef("2023-05-05"), DataType.TEXT, "second")
),
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0], zoneId=Z]",
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0]]",
DataType.DATETIME,
equalTo(1683244800000L)
)
Expand All @@ -85,7 +98,7 @@ public static Iterable<Object[]> parameters() {
new TestCaseSupplier.TypedData(new BytesRef("yyyy-MM-dd"), DataType.TEXT, "first"),
new TestCaseSupplier.TypedData(new BytesRef("2023-05-05"), DataType.KEYWORD, "second")
),
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0], zoneId=Z]",
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0]]",
DataType.DATETIME,
equalTo(1683244800000L)
)
Expand All @@ -98,7 +111,7 @@ public static Iterable<Object[]> parameters() {
new TestCaseSupplier.TypedData(new BytesRef("2023-05-05"), DataType.KEYWORD, "second")

),
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0], zoneId=Z]",
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0]]",
DataType.DATETIME,
is(nullValue())
).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.")
Expand All @@ -118,7 +131,7 @@ public static Iterable<Object[]> parameters() {
new TestCaseSupplier.TypedData(new BytesRef("not a date"), DataType.KEYWORD, "second")

),
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0], zoneId=Z]",
"DateParseEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0]]",
DataType.DATETIME,
is(nullValue())
).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.")
Expand Down