[ES|QL] String literal implicit casting

[fang-xing-esql]

Cast string literals to expected data types of ESQL scalar functions and arithmetic operations automatically.

[costin]

Left a small round of comments. This is shaping up nicely. Please check its impact on the rest of the code and look at removing to string conversion from various functions and see if casting works.

[elasticsearchmachine]

Pinging @elastic/es-analytical-engine (Team:Analytics)

[fang-xing-esql]

Left a small round of comments. This is shaping up nicely. Please check its impact on the rest of the code and look at removing to string conversion from various functions and see if casting works.

Thanks for reviewing! One place that we do string conversation in an EsqlScalarFunction is AutoBucket, the 3rd and 4th inputs are converted from string to datetime inside the function, only if its 1st input is a datetime. If its 1st input is numeric, we don't convert. Since the conversion depends on another argument, we might just leave it inside the AutoBucket function.

[costin]

My main comment is around still using string/keyword for AutoBucket - otherwise LGTM.

[astefan]

Left some comments but, most importantly, this PR changes at least one current behavior into a not-ideal manner.
The following query from employees | EVAL k = hire_date > DATE_FORMAT(\"YYYY-MM-dd\", \"1989-06-02T00:00:00.000Z\") | keep languages, hire_date (I didn't spend time in refining it) generates a user friendly error message in main:

                "type": "verification_exception",
                "reason": "Found 1 problem\nline 1:39: second argument of [DATE_FORMAT(\"YYYY-MM-dd\", \"1989-06-02T00:00:00.000Z\")] must be [datetime], found value [\"1989-06-02T00:00:00.000Z\"] type [keyword]",
                "stack_trace": "org.elasticsearch.xpack.esql.VerificationException: Found 1 problem\nline 1:39: second argument of [DATE_FORMAT(\"YYYY-MM-dd\", \"1989-06-02T00:00:00.000Z\")] must be [datetime], found value [\"1989-06-02T00:00:00.000Z\"] type [keyword]\r\n\tat org.elasticsearch.xpack.esql.analysis.Analyzer.verify(Analyzer.java:133)

while with this PR the error is not pretty

path: /_query, params: {format=txt, error_trace=true}, status: 500 java.lang.ClassCastException: class java.lang.String cannot be cast to class org.apache.lucene.util.BytesRef (java.lang.String is in module java.base of loader 'bootstrap'; org.apache.lucene.util.BytesRef is in module [email protected] of loader 'app')
        at org.elasticsearch.xpack.esql.expression.function.scalar.date.DateFormat.toFormatter(DateFormat.java:114)
        at org.elasticsearch.xpack.esql.expression.function.scalar.date.DateFormat.toEvaluator(DateFormat.java:106)
        at org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper.fold(EvaluatorMapper.java:48)
        at org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction.fold(EsqlScalarFunction.java:30)
        at org.elasticsearch.xpack.esql.analysis.Analyzer$PromoteStringsInDateComparisons.stringToDate(Analyzer.java:817)
        at org.elasticsearch.xpack.esql.analysis.Analyzer$PromoteStringsInDateComparisons.promote(Analyzer.java:802)
        at org.elasticsearch.xpack.ql.tree.Node.lambda$transformUp$11(Node.java:198)
        at org.elasticsearch.xpack.ql.tree.Node.transformUp(Node.java:192)

[fang-xing-esql]

Left some comments but, most importantly, this PR changes at least one current behavior into a not-ideal manner. The following query from employees | EVAL k = hire_date > DATE_FORMAT(\"YYYY-MM-dd\", \"1989-06-02T00:00:00.000Z\") | keep languages, hire_date (I didn't spend time in refining it) generates a user friendly error message in main:

                "type": "verification_exception",
                "reason": "Found 1 problem\nline 1:39: second argument of [DATE_FORMAT(\"YYYY-MM-dd\", \"1989-06-02T00:00:00.000Z\")] must be [datetime], found value [\"1989-06-02T00:00:00.000Z\"] type [keyword]",
                "stack_trace": "org.elasticsearch.xpack.esql.VerificationException: Found 1 problem\nline 1:39: second argument of [DATE_FORMAT(\"YYYY-MM-dd\", \"1989-06-02T00:00:00.000Z\")] must be [datetime], found value [\"1989-06-02T00:00:00.000Z\"] type [keyword]\r\n\tat org.elasticsearch.xpack.esql.analysis.Analyzer.verify(Analyzer.java:133)

while with this PR the error is not pretty

                "type": "verification_exception",
                "reason": "Found 1 problem\nline 1:39: second argument of [DATE_FORMAT(\"YYYY-MM-dd\", \"1989-06-02T00:00:00.000Z\")] must be [datetime], found value [\"1989-06-02T00:00:00.000Z\"] type [keyword]",
                "stack_trace": "org.elasticsearch.xpack.esql.VerificationException: Found 1 problem\nline 1:39: second argument of [DATE_FORMAT(\"YYYY-MM-dd\", \"1989-06-02T00:00:00.000Z\")] must be [datetime], found value [\"1989-06-02T00:00:00.000Z\"] type [keyword]\r\n\tat org.elasticsearch.xpack.esql.analysis.Analyzer.verify(Analyzer.java:133)

Thank you for catching this issue!

After casting the second inputs to date_format to date with the ImplicitCasting rule, type resolution passed for date_format, and it is foldable now, however a new error is thrown from the PromoteStringsInDateComparisons rule. The left hand side hire_date is a date type, the output from date_format is a string type, this is why it errors out.

Currently the auto-casting in ImplicitCasting and PromoteStringsInDateComparisons support string literal only, it does not cast the string output of scalar functions. The scalar functions will be evaluated/fold during logical plan optimizer, so it is too early to cast the output of date_format to date type during resolution phase.

The query below errors out on main with the same error at the problem query above on the branch:
from employees | EVAL k = hire_date > DATE_FORMAT("YYYY-MM-dd", now()) | keep languages, hire_date;

java.lang.ClassCastException: class java.lang.String cannot be cast to class org.apache.lucene.util.BytesRef (java.lang.String is in module java.base of loader 'bootstrap'; org.apache.lucene.util.BytesRef is in unnamed module of loader 'app')

I could add a check in PromoteStringsInDateComparisons to cast only literals, and skip functions. Perhaps I should add the same check in ImplicitCasting for nested functions.

[astefan]

@fang-xing-esql Now I realized I didn't copy-pasted the right exception for what happens in this PR. Sorry about that and thank you for testing and confirming in your reply the behavior. I've updated my comment above.

[nik9000]

This probably shouldn't be NULL - if you leave off orElse then you'll get an Optional type.

[fang-xing-esql]

Let's try if UNSUPPORTED makes sense.

[nik9000]

@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.

[drewdaemon]

@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!

[nik9000]

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.

[drewdaemon]

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!

[nik9000]

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.

[bpintea]

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?

[fang-xing-esql]

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.

[nik9000]

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.

[bpintea]

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).