slinkydeveloper commented on a change in pull request #17878:
URL: https://github.com/apache/flink/pull/17878#discussion_r755314532
##########
File path:
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/AbstractCodeGeneratorCastRule.java
##########
@@ -102,12 +105,32 @@ protected AbstractCodeGeneratorCastRule(CastRulePredicate
predicate) {
final String functionSignature =
"@Override public Object cast(Object _myInputObj) throws "
+ className(TableException.class);
- final String inputVarDecl =
- inputTypeTerm + " " + inputTerm + " = (" + inputTypeTerm + ")
_myInputObj;\n";
- final String inputIsNullVarDecl =
- "boolean " + inputIsNullTerm + " = _myInputObj == null;\n";
- final String returnStmt = "return " + codeBlock.getReturnTerm() +
";\n";
+ // Write the function body
+ final CastRuleUtils.CodeWriter bodyWriter = new
CastRuleUtils.CodeWriter();
+ bodyWriter.declStmt(inputTypeTerm, inputTerm, cast(inputTypeTerm,
"_myInputObj"));
+ bodyWriter.declStmt("boolean", inputIsNullTerm, "_myInputObj == null");
+ ctx.variableDeclarationStatements.forEach(decl ->
bodyWriter.appendBlock(decl + "\n"));
+
+ if (this.canFail()) {
+ bodyWriter.tryCatchStmt(
+ tryWriter ->
+ tryWriter.append(codeBlock).stmt("return " +
codeBlock.getReturnTerm()),
+ Throwable.class,
+ (exceptionTerm, catchWriter) ->
+ catchWriter.throwStmt(
+ constructorCall(
Review comment:
I don't know, TBH I don't like much `newException(Class<?>
exceptionClass, String msg)` because then we need the version of the method
that accepts the cause term, making the signature a bit confusing because both
are strings: `newException(Class<?> exceptionClass, String msg, String
causeTerm)`. Adding the logic to `throwStmt` is also inconsistent with the rest
of `CodeWriter`, as `throw` accepts any exception, not just a constructor
invocation expression...
What if we revisit this topic once we'll need in some rule to throw an
exception in the cast logic? This `AbstractCodeGeneratorCastRule` is a bit of a
special case for me because it just glues the code generated rules with the
`CastExecutor`, but it doesn't really have cast logic, so I'm not sure whether
we'll ever need again this try catch and throw new methods and how we want to
use them
--
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]
