924060929 commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4217784122
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java:
##########
@@ -63,15 +65,32 @@ private Sha2(ScalarFunctionParams functionParams) {
@Override
public void checkLegalityBeforeTypeCoercion() {
- checkLegalityAfterRewrite();
+ // validate the value FE can evaluate here, because constant folding,
e.g. of sha2(null, 1 + 2), may remove
+ // this function before checkLegalityAfterRewrite
+
checkDigestLength(ExpressionUtils.foldConstantArgument(getArgument(1)));
}
@Override
public void checkLegalityAfterRewrite() {
- if (!(child(1) instanceof IntegerLikeLiteral)) {
- throw new AnalysisException("the second parameter of sha2 must be
a literal but got: " + child(1).toSql());
+ checkDigestLength(getArgument(1));
+ }
+
+ private void checkDigestLength(Expression digestLength) {
+ if (!digestLength.isConstant()) {
+ throw new AnalysisException("the second parameter of sha2 must be
a constant but got: "
+ + digestLength.toSql());
+ }
+ // the type is checked before the type coercion casts the argument to
the INT signature, so a decimal
+ // constant is rejected like a decimal literal
+ if (!digestLength.getDataType().isIntegralType()) {
+ throw new AnalysisException("the second parameter of sha2 must be
an integer but got: "
+ + digestLength.toSql());
+ }
+ // the value of a constant FE cannot fold is validated by BE when it
is evaluated
+ if (!(digestLength instanceof Literal)) {
+ return;
}
- final int constParam = ((IntegerLikeLiteral) child(1)).getIntValue();
+ final int constParam = ((IntegerLikeLiteral)
digestLength).getIntValue();
Review Comment:
Rechecked at `2e1e23e17ce6`: the lifecycle refactor addresses the earlier
architecture recommendation, but this typed-NULL issue is still present. For
`select sha2('abc', cast(null as int))`, `checkLegalityBeforeTypeCoercion()`
evaluates the length through `foldConstantArgument()` and gets
`NullLiteral(IntegerType)`. It passes the constant, integral-type, and
`Literal` checks, then the cast to `IntegerLikeLiteral` in `Sha2.java:93`
throws `ClassCastException`. This is based on tracing the current source; I
have not run the query locally. Please handle typed NULL explicitly before the
cast, preferably reporting `AnalysisException` to preserve the previous
rejection behavior, and add a unit/regression case with NULL as the digest
length. The current NULL-input tests exercise the first argument and do not
cover this case.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]