Skip to content

[ES|QL] String literal implicit casting #106932

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
fang-xing-esql merged 24 commits into from
Apr 11, 2024
Merged

[ES|QL] String literal implicit casting #106932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ae28791
string literal casting
fang-xing-esql Mar 29, 2024
c1ca596
string literal casting
fang-xing-esql Mar 29, 2024
31d6654
fix tests failures
fang-xing-esql Apr 1, 2024
66e4202
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 1, 2024
d33a18b
add new tests for string casting
fang-xing-esql Apr 1, 2024
e8ce087
update according to review comments
fang-xing-esql Apr 2, 2024
b96be99
fix tests failures
fang-xing-esql Apr 2, 2024
55a8f30
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 2, 2024
21819de
fix tests failures
fang-xing-esql Apr 2, 2024
d61ef8d
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 2, 2024
f99d040
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 3, 2024
1314c32
update according to review comments
fang-xing-esql Apr 3, 2024
b81d62a
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 4, 2024
37ee143
auto casting for EsqlArithmeticOperation
fang-xing-esql Apr 5, 2024
1bd84bb
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 5, 2024
3473df2
update according to review comments
fang-xing-esql Apr 8, 2024
a5af576
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 8, 2024
14f6061
update according to review comments
fang-xing-esql Apr 10, 2024
d3d1e8c
Merge branch 'test' into auto-casting-functions
fang-xing-esql Apr 10, 2024
84e2412
merge recent main
fang-xing-esql Apr 10, 2024
aed668c
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 10, 2024
6b6ac05
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 10, 2024
b6eae98
update according to review comments
fang-xing-esql Apr 10, 2024
905102e
Merge branch 'main' into auto-casting-functions
fang-xing-esql Apr 10, 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
240 changes: 0 additions & 240 deletions docs/reference/esql/functions/kibana/definition/bucket.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,246 +34,6 @@
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
Copy link
Copy Markdown
Member

@nik9000 nik9000 Apr 11, 2024

Choose a reason for hiding this comment

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

@drewdaemon I've just realized you'll need this too. Here's the deal - we have automatic promotion of string literals. For this particular function these always had to be literals anyway so we no longer think of these as valid signatures.

This is an interesting wrinkle. We don't send anything telling you this has to be a literal. We probably should.

Copy link
Copy Markdown
Contributor

@drewdaemon drewdaemon Apr 11, 2024

Choose a reason for hiding this comment

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

@nik9000 thanks for thinking of us! Yes, we currently have this information hard-coded on our end. If there's a way to surface this in the definitions y'all provide us... all the better!

Copy link
Copy Markdown
Member

@nik9000 nik9000 Apr 11, 2024

Choose a reason for hiding this comment

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

We could generate something like:

       {
          "name" : "from",
          "type" : "keyword",
          "optional" : false,
          "description" : "",
          "only_literals": "date"
        },

Or something like that. But in general if something says datetime now it can take a literal string that is formatted like a date. Same for numbers. But that's quite a lot to add to your editor, especially right now.

There are other cases where we send you "only_literals": "any" for bits that take any literal. The buckets parameter here only takes literal integers. No fields allowed.

Copy link
Copy Markdown
Contributor

@drewdaemon drewdaemon Apr 11, 2024

Choose a reason for hiding this comment

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

Yeah, this kind of thing would be great. I have put elastic/kibana#179634 a bit on hold while I squish editor bugs for the 8.14 release. Once that settles down, I'm going to work on pulling these generated definitions into Kibana "for real."

At that point, I think I'll have a clearer understanding of exactly what structure would work best.

But in general if something says datetime now it can take a literal string that is formatted like a date. Same for numbers.

Noted, thanks!

{
"name" : "field",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "buckets",
"type" : "integer",
"optional" : false,
"description" : ""
},
{
"name" : "from",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "to",
"type" : "keyword",
"optional" : false,
"description" : ""
}
],
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
{
"name" : "field",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "buckets",
"type" : "integer",
"optional" : false,
"description" : ""
},
{
"name" : "from",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "to",
"type" : "text",
"optional" : false,
"description" : ""
}
],
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
{
"name" : "field",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "buckets",
"type" : "integer",
"optional" : false,
"description" : ""
},
{
"name" : "from",
"type" : "keyword",
"optional" : false,
"description" : ""
},
{
"name" : "to",
"type" : "datetime",
"optional" : false,
"description" : ""
}
],
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
{
"name" : "field",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "buckets",
"type" : "integer",
"optional" : false,
"description" : ""
},
{
"name" : "from",
"type" : "keyword",
"optional" : false,
"description" : ""
},
{
"name" : "to",
"type" : "keyword",
"optional" : false,
"description" : ""
}
],
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
{
"name" : "field",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "buckets",
"type" : "integer",
"optional" : false,
"description" : ""
},
{
"name" : "from",
"type" : "keyword",
"optional" : false,
"description" : ""
},
{
"name" : "to",
"type" : "text",
"optional" : false,
"description" : ""
}
],
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
{
"name" : "field",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "buckets",
"type" : "integer",
"optional" : false,
"description" : ""
},
{
"name" : "from",
"type" : "text",
"optional" : false,
"description" : ""
},
{
"name" : "to",
"type" : "datetime",
"optional" : false,
"description" : ""
}
],
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
{
"name" : "field",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "buckets",
"type" : "integer",
"optional" : false,
"description" : ""
},
{
"name" : "from",
"type" : "text",
"optional" : false,
"description" : ""
},
{
"name" : "to",
"type" : "keyword",
"optional" : false,
"description" : ""
}
],
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
{
"name" : "field",
"type" : "datetime",
"optional" : false,
"description" : ""
},
{
"name" : "buckets",
"type" : "integer",
"optional" : false,
"description" : ""
},
{
"name" : "from",
"type" : "text",
"optional" : false,
"description" : ""
},
{
"name" : "to",
"type" : "text",
"optional" : false,
"description" : ""
}
],
"variadic" : false,
"returnType" : "datetime"
},
{
"params" : [
{
Expand Down
8 changes: 0 additions & 8 deletions docs/reference/esql/functions/types/bucket.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@
|===
field | buckets | from | to | result
datetime | integer | datetime | datetime | datetime
datetime | integer | datetime | keyword | datetime
datetime | integer | datetime | text | datetime
datetime | integer | keyword | datetime | datetime
datetime | integer | keyword | keyword | datetime
datetime | integer | keyword | text | datetime
datetime | integer | text | datetime | datetime
datetime | integer | text | keyword | datetime
datetime | integer | text | text | datetime
Copy link
Copy Markdown
Member

@nik9000 nik9000 Apr 11, 2024

Choose a reason for hiding this comment

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

This is interesting. We no longer tell folks they can use strings here, only dates. But maybe a human is ok because the examples will contain string literals.

Copy link
Copy Markdown
Contributor

@bpintea bpintea Apr 11, 2024

Choose a reason for hiding this comment

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

Is this correct, though? The function does accept the string types (the foldToLong() logic remained), but generally, wouldn't we have to rather extend the function infos of scalar/arithmetic functions to include these types, now that we autocast?

Copy link
Copy Markdown
Member Author

@fang-xing-esql fang-xing-esql Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Currently the implicit casting is done at resolution phase. We can do auto casting if the string literals are provided as input to the bucket function directly, e.g.
BUCKET(date, 20, "1985-01-01T00:00:00Z", "1986-01-01T00:00:00Z"). --> auto casting convert from and to to datetime

However, if the string literals are provided as input to the bucket function through a variable, auto casting does not apply, e.g.
| EVAL bucket_start = "1999-01-01T00:00:00Z". --> auto casting does not apply
| EVAL bucket_end = NOW()
| EVAL bucket = AUTO_BUCKET(hire_date, 5, bucket_start, bucket_end)

This is the main reason that there are still code inside to function to do the conversion, as the foldable eval propagation happens at logical planner. We have some options to make auto casting work better, like:

  • apply the implicit casting rule in logical planner after foldable eval propagation
  • apply foldable eval propagation in resolution phase (I brought up this idea before, probably not a good one)

There are some follow up work related to the auto casting. A lot(perhaps most) of the rules in analyzer and logical planner are related to query transformation, there are chances that we can rearrange the rules to make resolution and query transformation work more effectively.

Copy link
Copy Markdown
Member

@nik9000 nik9000 Apr 11, 2024

Choose a reason for hiding this comment

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

As to whether this is right or not, I dunno. You certainly can only put date-like-strings in it. And now that we support implicit conversion for these literals everywhere where this list says "date" it means "date or date-like-string". I dunno.

Copy link
Copy Markdown
Contributor

@bpintea bpintea Apr 11, 2024

Choose a reason for hiding this comment

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

This is the main reason that there are still code inside to function to do the conversion

Sure, that's fine. I was mostly commenting on adjusting the function info to remove the keyword and text types as accepted by the function (and the docs changes that ensued).

double | integer | double | double | double
double | integer | double | integer | double
double | integer | double | long | double
Expand Down
44 changes: 44 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 @@ -510,6 +510,17 @@ date1:date | date2:date | dd_ms:integer
// end::docsDateDiff-result[]
;

evalDateDiffString
required_feature: esql.string_literal_auto_casting

ROW date1 = TO_DATETIME("2023-12-02T11:00:00.000Z")
| EVAL dd_ms = DATE_DIFF("microseconds", date1, "2023-12-02T11:00:00.001Z")
;

date1:date | dd_ms:integer
2023-12-02T11:00:00.000Z | 1000
;

evalDateParseWithSimpleDate
row a = "2023-02-01" | eval b = date_parse("yyyy-MM-dd", a) | keep b;

Expand Down Expand Up @@ -1085,6 +1096,17 @@ date:date | year:long
// end::dateExtract-result[]
;

dateExtractString
required_feature: esql.string_literal_auto_casting

ROW date = DATE_PARSE("yyyy-MM-dd", "2022-05-06")
| EVAL year = DATE_EXTRACT("year", "2022-05-06")
;

date:date | year:long
2022-05-06T00:00:00.000Z | 2022
;

docsDateExtractBusinessHours
// tag::docsDateExtractBusinessHours[]
FROM sample_data
Expand Down Expand Up @@ -1115,6 +1137,17 @@ Anneke |Preusig |1989-06-02T00:00:00.000Z|1989-06-02
// end::docsDateFormat-result[]
;

evalDateFormatString
required_feature: esql.string_literal_auto_casting

ROW a = 1
| EVAL df = DATE_FORMAT("YYYY-MM-dd", "1989-06-02T00:00:00.000Z")
;

a:integer | df:keyword
1 | 1989-06-02
;

docsDateTrunc
// tag::docsDateTrunc[]
FROM employees
Expand All @@ -1133,6 +1166,17 @@ Anneke |Preusig |1989-06-02T00:00:00.000Z|1989-01-01T00:00:00.000
// end::docsDateTrunc-result[]
;

evalDateTruncString
required_feature: esql.string_literal_auto_casting

ROW a = 1
| EVAL year_hired = DATE_TRUNC(1 year, "1991-06-26T00:00:00.000Z")
;

a:integer | year_hired:date
1 | 1991-01-01T00:00:00.000Z
;

docsDateTruncHistogram
// tag::docsDateTruncHistogram[]
FROM employees
Expand Down
Loading