lincoln-lil commented on code in PR #25076:
URL: https://github.com/apache/flink/pull/25076#discussion_r1674153859
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java:
##########
@@ -433,7 +433,7 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL)
public static final BuiltInFunctionDefinition URL_DECODE =
BuiltInFunctionDefinition.newBuilder()
- .name("url_decode")
+ .name("URL_DECODE")
.kind(SCALAR)
.inputTypeStrategy(sequence(logical(LogicalTypeFamily.CHARACTER_STRING)))
.outputTypeStrategy(nullableIfArgs(explicit(DataTypes.STRING().nullable())))
Review Comment:
@snuyanzin As I saw this pr and realized I missed your message on the
previous one(#24773).
For the output type strategy, I think it should be
'outputTypeStrategy(explicit(DataTypes.STRING().nullable()))' because the
return value may be null even if the input is non-null (e.g., decoding
failure), otherwise the downstream operator may trust the non-null property and
encounter a runtime exception (e.g., NPE) if skip the null check for field
accessing.
Does this make sense to you?
--
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]
