SQL: Handle the bwc Joda ZonedDateTime scripting class in Painless #37024
Conversation
|
Pinging @elastic/es-search |
| @@ -468,6 +468,10 @@ public static String ucase(String s) { | |||
| // Casting | |||
| // | |||
| public static Object cast(Object value, String typeName) { | |||
| return DataTypeConversion.convert(value, DataType.fromTypeName(typeName)); | |||
| Object safeDateTimeValue = value; | |||
There was a problem hiding this comment.
It might make sense to rework this as part of asDateTime which handles the various types of dates that can be encountered.
There was a problem hiding this comment.
Yeah, I wasn't 100% happy about the duplicated code, either. I've pushed another approach. Let me know what you think @costin.
|
run the gradle build tests 2 |
2 similar comments
|
run the gradle build tests 2 |
|
run the gradle build tests 2 |
| @@ -355,11 +359,14 @@ public static ZonedDateTime asDateTime(Object dateTime) { | |||
| if (dateTime instanceof ZonedDateTime) { | |||
| return (ZonedDateTime) dateTime; | |||
| } | |||
| if (dateTime instanceof Number) { | |||
| return DateUtils.of(((Number) dateTime).longValue()); | |||
| if (false == lenient) { | |||
There was a problem hiding this comment.
Keep the instanceof check clean and move the if on the throw:
if (dateTime instanceof ...)
if (dateTime instanceof ...)
if (dateTime instanceof ...)
if (lenient) { return dateTime; } else { throw exception }
There was a problem hiding this comment.
@costin if I take the instanceof Number condition out of the if (lenient) part, there will be failures for queries that order by or group by a casted numeric function. This is because the asDateTime is specifically being used for date/time values whereas, in case of CONVERT or CAST the instanceof Number part will be applied for NON-date/time numerics, as well.
The casting method used in Painless scripting by ES SQL didn't account for
JodaCompatibleZonedDateTimeclass introduced as part of this PR. This PR fixes this and addresses #37023.