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]

Reply via email to