chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549662644
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1195,27 +1208,46 @@ case class NextDay(startDate: Expression, dayOfWeek:
Expression)
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
+ val failOnErrorTerm = ctx.freshName("failOnError")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) ==
-1) {
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
+ } else if (DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ val errMsg = input.replace(
+ UTF8String.fromString("""""""), UTF8String.fromString("""\""""))
s"""
- |${ev.isNull} = true;
- """.stripMargin
+ |boolean $failOnErrorTerm = $failOnError;
+ |if ($failOnErrorTerm) {
Review comment:
Hi @cloud-fan ,
I thought about it but didn't find a solution. Here is what I tried:
Instead of doing:
```
s"""
|boolean $failOnErrorTerm = $failOnError;
|if ($failOnErrorTerm) {
| throw new IllegalArgumentException("Illegal input for
dayOfWeek: $errMsg");
|} else {
| ${ev.isNull} = true;
|}
|""".stripMargin
```
I was doing this in the beginning:
```
if (failOnError) {
val errMsg = input.replace(
UTF8String.fromString("""""""),
UTF8String.fromString("""\""""))
s"""throw new IllegalArgumentException("Illegal input for
dayOfWeek: $errMsg");"""
} else {
s"""${ev.isNull} = true;"""
}
```
however the generated code would be:
```
/* 033 */ public java.lang.Object apply(java.lang.Object _i) {
/* 034 */ InternalRow i = (InternalRow) _i;
/* 035 */
/* 036 */
/* 037 */ boolean isNull_0 = true;
/* 038 */ int value_0 = -1;
/* 039 */
/* 040 */
/* 041 */
/* 042 */ isNull_0 = false; // resultCode could change nullability.
/* 043 */ throw new IllegalArgumentException("Illegal input for
dayOfWeek: xx");
/* 044 */ isNull_3 = isNull_0;
/* 045 */ value_3 = value_0;
/* 046 */
/* 047 */ // copy all the results into MutableRow
/* 048 */
/* 049 */ if (!isNull_3) {
/* 050 */ mutableRow.setInt(0, value_3);
/* 051 */ } else {
/* 052 */ mutableRow.setNullAt(0);
/* 053 */ }
/* 054 */
/* 055 */ return mutableRow;
/* 056 */ }
```
Which can not be compiled because:
`org.codehaus.commons.compiler.CompileException: File 'generated.java', Line
44, Column 1: failed to compile:
org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 44,
Column 1: Statement is unreachable`
What's why I put this if-else here. And the generated code becomes:
```
/* 043 */
/* 044 */ boolean failOnError_0 = false;
/* 045 */ if (failOnError_0) {
/* 046 */ throw new IllegalArgumentException("Illegal input for
dayOfWeek: xx");
/* 047 */ } else {
/* 048 */ isNull_0 = true;
/* 049 */ }
/* 050 */ isNull_3 = isNull_0;
/* 051 */ value_3 = value_0;
/* 052 */
```
I looked at other expressions where the `throw exception` is always in a
try-cache block. However the `DateTimeUtils.getDayOfWeekFromString` function
here returns -1 instead of exception for non-valid inputs.
Do you have an idea how to implement without putting `failOnError` into the
generated code?
Thanks in advance
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
