zhougit86 commented on PR #22624:
URL: https://github.com/apache/flink/pull/22624#issuecomment-1614295522
> @zhougit86 sorry for the late response
>
> I have a question: could you please clarify it it seems your change is
inside condition
>
> ```scala
> if (codeGeneratorCastRule.canFail(inputType, targetType) && nullOnFailure)
{
> ...
> val adjustedTargetType = if (nullOnFailure) targetType.copy(nullOnFailure)
else targetType
> return GeneratedExpression(
> resultTerm,
> nullTerm,
> operand.code + "\n" + castCode,
> adjustedTargetType
> )
> ...
> } else {
> ...
> return GeneratedExpression(
> castCodeBlock.getReturnTerm,
> castCodeBlock.getIsNullTerm,
> operand.code + castCode,
> targetType
> )
> }
> ```
>
> shouldn't we extract `val adjustedTargetType = if (nullOnFailure)
targetType.copy(nullOnFailure) else targetType` from `if/else` and apply it to
both `if` and `else` branches?
Hate to admit my negligence being caught again :(
I think maybe we shouldn't extract ```val adjustedTargetType = if
(nullOnFailure) targetType.copy(nullOnFailure) else targetType``` out from the
```if/else``` section, as in the else section, we will not catch the Exception
thrown by the cast, so the nullTerm is actually useless in the generated code.
Anyway, I will remove the redundant ```if (nullOnFailure)```, let me know
your think please...
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
