[SPARK-49768][SQL] Provide error conditions for make_date/make_timestamp errors _LEGACY_ERROR_TEMP_2000
#48242
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -278,13 +278,49 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE | |
| } | ||
|
|
||
| def ansiDateTimeError(e: Exception): SparkDateTimeException = { | ||
| def extractDateTimeErrorInfo(e: Exception): (String, String, String) = { | ||
| val errorMessage = e.getMessage | ||
|
|
||
| val valuePattern = "Invalid value for ([A-Za-z]+) \\(valid values (.+)\\): (.+)".r | ||
| val datePattern = "Invalid date '[A-Z]+ ([0-9]+)'".r | ||
|
|
||
| errorMessage match { | ||
|
Contributor
There was a problem hiding this comment. @srielau If I understood @MaxGekk we want to get away from this message creation in the specific calls of the error, but actually keep all the info in error-conditions.json. This will make it really hard to sync with other APIs, especially as we are expanding these endless number of exception messages that we can get from the java library. Also, since this is external dependancy, who is to guarantee that this message format will stay the same when java updates.
Contributor
There was a problem hiding this comment. The guarantee is the tests we write against them. I'm not opposed to having catch all for the "unforeseen".
Contributor
There was a problem hiding this comment. Agree, but are we really going for this over getting as much information as we can provide to the user? For example there is a specific exception that is returning leap year error, but we will just say this should be in range. If I got a message like this, tbh I would be confused as to why it is not working when my value actually is in 1...28/31 range (29th february). We could do this changing for errors that we know for sure how they behave, but for all others, I lean to leaving the java message, as it provides sufficient info on why it is failing and end goal should be providing user with sufficient info to fix the error imo. |
||
| case valuePattern(field, range, badValue) => | ||
| val unit = field match { | ||
| case "Year" => "YEAR" | ||
| case "MonthOfYear" => "MONTH" | ||
itholic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| case "DayOfMonth" => "DAY" | ||
| case "HourOfDay" => "HOUR" | ||
| case "MinuteOfHour" => "MINUTE" | ||
| case "SecondOfMinute" => "SECOND" | ||
| } | ||
| val formattedRange = range.replace(" - ", " ... ") | ||
| (unit, formattedRange, badValue) | ||
| case datePattern(badDate) => | ||
| ("DAY", "1 ... 28/31", badDate) | ||
|
Contributor
There was a problem hiding this comment. This is really weird message now. The error we are getting here now is totally relentless of leap years. Java library returns special case errors when we have leap years, look at
Contributor
There was a problem hiding this comment. To clarify, I find it weird that we are removing extra information which java is giving us for free. |
||
| case _ => | ||
| throw new SparkDateTimeException( | ||
|
Contributor
There was a problem hiding this comment. @srielau @gengliangwang let's link issues to the proper tickets and not open duplicate tickets. The reason for not having a error condition is that |
||
| errorClass = "_LEGACY_ERROR_TEMP_2000", | ||
| messageParameters = Map( | ||
| "message" -> errorMessage, | ||
| "ansiConfig" -> toSQLConf(SQLConf.ANSI_ENABLED.key)), | ||
| context = Array.empty, | ||
| summary = "") | ||
| } | ||
|
Member
There was a problem hiding this comment. @itholic Do you expect that it should match to both the cases above. What if not? I would add a default case.
Member
There was a problem hiding this comment. For example, How does your code handles this?
Contributor
Author
There was a problem hiding this comment. Good point. I think We may need introduce several new error classes to cover the all potential exception cases along with more test cases in a separate ticket. |
||
| } | ||
| val (unit, range, badValue) = extractDateTimeErrorInfo(e) | ||
| new SparkDateTimeException( | ||
| errorClass = "DATETIME_FIELD_OUT_OF_BOUNDS", | ||
| messageParameters = Map( | ||
| "ansiConfig" -> toSQLConf(SQLConf.ANSI_ENABLED.key), | ||
| "unit" -> unit, | ||
| "range" -> range, | ||
| "badValue" -> toSQLValue(badValue) | ||
| ), | ||
| context = Array.empty, | ||
| summary = "", | ||
| cause = Some(e) | ||
| ) | ||
| } | ||
|
|
||
| def ansiIllegalArgumentError(message: String): SparkIllegalArgumentException = { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please follow the same way of providing ANSI turn off suggestion, as it makes it easier to track what error messages are related to ANSI.
If necessary set <ansiConfig> to "false" to bypass this error.Removal of this message is related to https://issues.apache.org/jira/browse/SPARK-49642