Skip to content

Esql additional date format testing #120000

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

Merged
merged 5 commits into from
Jan 13, 2025
Merged

Esql additional date format testing #120000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
70413e7
randomized testing for date format
not-napoleon Jan 10, 2025
c2cbced
slightly cleaner regex
not-napoleon Jan 10, 2025
26fe685
handle single parameter case, such as it is
not-napoleon Jan 10, 2025
74dcf98
[CI] Auto commit changes from spotless
Jan 10, 2025
e8a6d96
Merge branch 'main' into esql-date-format-testing
not-napoleon Jan 13, 2025
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
12 changes: 12 additions & 0 deletions docs/reference/esql/functions/kibana/definition/date_format.json
View file
Open in desktop

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

1 change: 1 addition & 0 deletions docs/reference/esql/functions/types/date_format.asciidoc
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion ...rc/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateFormat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public class DateFormat extends EsqlConfigurationFunction implements OptionalArg
)
public DateFormat(
Source source,
@Param(optional = true, name = "dateFormat", type = { "keyword", "text" }, description = """
Copy link
Member Author

Choose a reason for hiding this comment

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

The first positional argument can be a date, in the one parameter version of the function.

Copy link
Member

Choose a reason for hiding this comment

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

It's quite a weird way to do it, but that's how the tests work.

We could, optionally, hack around this in the tests somehow. Or teach them about first position optional arguments. It's just that we don't have that many!

@Param(optional = true, name = "dateFormat", type = { "keyword", "text", "date" }, description = """
Date format (optional). If no format is specified, the `yyyy-MM-dd'T'HH:mm:ss.SSSZ` format is used.
If `null`, the function returns `null`.""") Expression format,
@Param(name = "date", type = { "date" }, description = "Date expression. If `null`, the function returns `null`.") Expression date,
Expand Down
24 changes: 24 additions & 0 deletions ...esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,17 @@ public static List<TypedDataSupplier> dateCases() {
return dateCases(Long.MIN_VALUE, Long.MAX_VALUE);
}

/**
* Generate cases for {@link DataType#DATETIME}.
* <p>
* For multi-row parameters, see {@link MultiRowTestCaseSupplier#dateCases}.
* </p>
* Helper function for if you want to specify your min and max range as dates instead of longs.
*/
public static List<TypedDataSupplier> dateCases(Instant min, Instant max) {
return dateCases(min.toEpochMilli(), max.toEpochMilli());
}

/**
* Generate cases for {@link DataType#DATETIME}.
* <p>
Expand Down Expand Up @@ -1045,6 +1056,19 @@ public static List<TypedDataSupplier> dateCases(long min, long max) {
return cases;
}

/**
*
* @return randomized valid date formats
*/
public static List<TypedDataSupplier> dateFormatCases() {
return List.of(
new TypedDataSupplier("<format as KEYWORD>", () -> new BytesRef(ESTestCase.randomDateFormatterPattern()), DataType.KEYWORD),
Copy link
Member Author

Choose a reason for hiding this comment

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

This only generates the named formats, as far as I can tell. Would be good to write something that generates arbitrary format strings too, but I didn't see such a thing and this is still more coverage than we had.

new TypedDataSupplier("<format as TEXT>", () -> new BytesRef(ESTestCase.randomDateFormatterPattern()), DataType.TEXT),
new TypedDataSupplier("<format as KEYWORD>", () -> new BytesRef("yyyy"), DataType.KEYWORD),
new TypedDataSupplier("<format as TEXT>", () -> new BytesRef("yyyy"), DataType.TEXT)
);
}

/**
* Generate cases for {@link DataType#DATE_NANOS}.
*
Expand Down
13 changes: 12 additions & 1 deletion ...va/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateFormatErrorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,22 @@ protected List<TestCaseSupplier> cases() {

@Override
protected Expression build(Source source, List<Expression> args) {
return new DateFormat(source, args.get(0), args.get(1), EsqlTestUtils.TEST_CFG);
return new DateFormat(source, args.get(0), args.size() == 2 ? args.get(1) : null, EsqlTestUtils.TEST_CFG);
}

@Override
protected Matcher<String> expectedTypeErrorMatcher(List<Set<DataType>> validPerPosition, List<DataType> signature) {
// Single argument version
String source = sourceForSignature(signature);
String name = signature.get(0).typeName();
if (signature.size() == 1) {
return equalTo("first argument of [" + source + "] must be [datetime], found value [] type [" + name + "]");
}
// Two argument version
// Handle the weird case where we're calling the two argument version with the date first instead of the format.
if (signature.get(0).isDate()) {
return equalTo("first argument of [" + source + "] must be [string], found value [] type [" + name + "]");
}
return equalTo(typeErrorMessage(true, validPerPosition, signature, (v, p) -> switch (p) {
case 0 -> "string";
case 1 -> "datetime";
Expand Down
57 changes: 28 additions & 29 deletions ...st/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateFormatTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,21 @@
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;

import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.lucene.BytesRefs;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.xpack.esql.core.expression.Expression;
import org.elasticsearch.xpack.esql.core.tree.Source;
import org.elasticsearch.xpack.esql.core.type.DataType;
import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
import org.elasticsearch.xpack.esql.expression.function.scalar.AbstractConfigurationFunctionTestCase;
import org.elasticsearch.xpack.esql.session.Configuration;
import org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter;

import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Supplier;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.matchesPattern;

public class DateFormatTests extends AbstractConfigurationFunctionTestCase {
public DateFormatTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
Expand All @@ -31,39 +34,35 @@ public DateFormatTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> tes

@ParametersFactory
public static Iterable<Object[]> parameters() {
return parameterSuppliersFromTypedDataWithDefaultChecksNoErrors(
true,
List.of(
new TestCaseSupplier(
List.of(DataType.KEYWORD, DataType.DATETIME),
() -> new TestCaseSupplier.TestCase(
List.of(
new TestCaseSupplier.TypedData(new BytesRef("yyyy"), DataType.KEYWORD, "formatter"),
new TestCaseSupplier.TypedData(1687944333000L, DataType.DATETIME, "val")
),
"DateFormatEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0], locale=en_US]",
DataType.KEYWORD,
equalTo(BytesRefs.toBytesRef("2023"))
)
List<TestCaseSupplier> suppliers = new ArrayList<>();
// Formatter supplied cases
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
(format, value) -> new BytesRef(
DateFormatter.forPattern(((BytesRef) format).utf8ToString()).formatMillis(((Instant) value).toEpochMilli())
),
new TestCaseSupplier(
List.of(DataType.TEXT, DataType.DATETIME),
() -> new TestCaseSupplier.TestCase(
List.of(
new TestCaseSupplier.TypedData(new BytesRef("yyyy"), DataType.TEXT, "formatter"),
new TestCaseSupplier.TypedData(1687944333000L, DataType.DATETIME, "val")
),
"DateFormatEvaluator[val=Attribute[channel=1], formatter=Attribute[channel=0], locale=en_US]",
DataType.KEYWORD,
equalTo(BytesRefs.toBytesRef("2023"))
)
)
DataType.KEYWORD,
TestCaseSupplier.dateFormatCases(),
TestCaseSupplier.dateCases(Instant.parse("1900-01-01T00:00:00.00Z"), Instant.parse("9999-12-31T00:00:00.00Z")),
matchesPattern("DateFormatEvaluator\\[val=Attribute\\[channel=1], formatter=Attribute\\[(channel=0|\\w+)], locale=en_US]"),
(lhs, rhs) -> List.of(),
false
)
);
// Default formatter cases
TestCaseSupplier.unary(
suppliers,
"DateFormatConstantEvaluator[val=Attribute[channel=0], formatter=format[strict_date_optional_time] locale[]]",
TestCaseSupplier.dateCases(Instant.parse("1900-01-01T00:00:00.00Z"), Instant.parse("9999-12-31T00:00:00.00Z")),
DataType.KEYWORD,
(value) -> new BytesRef(EsqlDataTypeConverter.DEFAULT_DATE_TIME_FORMATTER.formatMillis(((Instant) value).toEpochMilli())),
List.of()
);
return parameterSuppliersFromTypedDataWithDefaultChecksNoErrors(true, suppliers);
}

@Override
protected Expression buildWithConfiguration(Source source, List<Expression> args, Configuration configuration) {
return new DateFormat(source, args.get(0), args.get(1), configuration);
return new DateFormat(source, args.get(0), args.size() == 2 ? args.get(1) : null, configuration);
}
}