github-actions[bot] commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4148793052
##########
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:
[P2] Handle a typed NULL digest length before casting to IntegerLikeLiteral.
`sha2('abc', CAST(NULL AS INT))` reaches this check with
`NullLiteral(IntegerType)`: the new pre-coercion fold produces a literal with
an integral type, then this cast throws `ClassCastException` during analysis.
Reject it with an analysis error or preserve the function's NULL semantics, and
cover this typed-NULL case.
##########
be/src/exprs/function/function_other_types_to_date.cpp:
##########
@@ -481,41 +481,46 @@ struct DateTrunc {
if (scope != FunctionContext::THREAD_LOCAL) {
return Status::OK();
}
+ // The time unit is a constant, but a constant expression such as an
arithmetic one is not
+ // evaluated in open. Then the state is created from the first row in
execute.
if (!context->is_col_constant(DateArgIsFirst ? 1 : 0)) {
- return Status::InvalidArgument(
- "date_trunc function of time unit argument must be
constant.");
+ return Status::OK();
}
- const auto& data_str =
- context->get_constant_col(DateArgIsFirst ? 1 :
0)->column_ptr->get_data_at(0);
+ return create_state(
+ context,
+ context->get_constant_col(DateArgIsFirst ? 1 :
0)->column_ptr->get_data_at(0));
Review Comment:
[P2] Defer date_trunc state creation for a NULL constant unit. In a two-row
`INSERT ... VALUES`, `date_trunc(DATE '2024-03-15', CAST(NULL AS VARCHAR))`
passes the new FE constant-string check, and the union's value expressions
bypass FE folding. `VExpr::get_const_col` evaluates the unit as NULL, then
`open` calls `create_state` on its NULL data before the scalar NULL wrapper
runs, so the insert fails instead of storing NULL. Handle a NULL constant unit
before parsing and cover both argument orders in a multirow VALUES test.
--
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]