github-actions[bot] commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4236474622
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java:
##########
@@ -63,15 +66,35 @@ 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;
+ }
+ if (digestLength instanceof NullLiteral) {
+ throw new AnalysisException("sha2 functions only support digest
length of " + validDigest.toString());
}
- final int constParam = ((IntegerLikeLiteral) child(1)).getIntValue();
+ final int constParam = ((IntegerLikeLiteral)
digestLength).getIntValue();
Review Comment:
[P2] Validate the full SHA2 length before narrowing it. `sha2('abc',
cast(4294967552 as bigint) + cast(0 as bigint))` now passes this check because
`getIntValue()` wraps the folded BIGINT to 256. The subsequent INT signature
cast overflows and, with the default non-strict cast setting, folds the call to
NULL instead of reporting an unsupported digest length. Compare the original
integer against the supported lengths and cover a wide folded expression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/UtcTime.java:
##########
@@ -94,6 +96,17 @@ public void checkLegalityAfterRewrite() {
}
}
+ @Override
+ public Expression prepareBeforeTypeCoercion() {
+ // computeSignature derives the return type from the value of the
scale, but only from an INT literal,
+ // and a wider literal would keep a narrowing cast to INT, so fold
only a scale whose type is at most INT
+ return withChildren(scale -> {
+ DataType type = scale.getDataType();
+ return type.isTinyIntType() || type.isSmallIntType() ||
type.isIntegerType()
Review Comment:
[P2] Include folded SMALLINT and TINYINT scales in the UTC_TIME signature
check. `utc_time(cast(3 as smallint))` is newly accepted after this preparation
folds the cast to `SmallIntLiteral(3)`, but `computeSignature` only recognizes
`IntegerLiteral`, so FE declares TIMEV2(0) while BE evaluates scale 3. `cast(7
as smallint)` also skips the 0..6 error. Validate and derive the return type
from every folded integral literal (or normalize it to INT) before coercion,
and cover both cases.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/SplitByRegexp.java:
##########
@@ -79,9 +81,15 @@ public SplitByRegexp withChildren(List<Expression> children)
{
public void checkLegalityBeforeTypeCoercion() {
List<Expression> arguments = getArguments();
if (arguments.size() == 3) {
- Expression thirdArgument = getArgument(2);
- if (!thirdArgument.isConstant() || !(thirdArgument instanceof
IntegerLikeLiteral)
- || (((IntegerLikeLiteral) thirdArgument).getIntValue() <
0)) {
+ // validate the value FE can evaluate here, because constant
folding may remove this function before
+ // any later check; the type is checked before the type coercion
casts the argument to the INT
+ // signature, and the value of a constant FE cannot fold is
validated by BE when it is evaluated.
+ // A typed NULL, e.g. cast(null as int), is an integral constant
but not a positive one.
+ Expression thirdArgument =
ExpressionUtils.foldConstantArgument(getArgument(2));
+ if (!thirdArgument.isConstant() ||
!thirdArgument.getDataType().isIntegralType()
+ || thirdArgument instanceof NullLiteral
+ || (thirdArgument instanceof IntegerLikeLiteral
+ && ((IntegerLikeLiteral) thirdArgument).getIntValue() <
0)) {
Review Comment:
[P2] Validate the full split limit before narrowing it.
`split_by_regexp('a,b', ',', cast(-4294967296 as bigint) + cast(0 as bigint))`
is newly accepted because `getIntValue()` wraps that negative BIGINT to zero.
Its later INT signature cast overflows and the default non-strict rewrite
returns NULL, so this invalid limit avoids both the FE error and BE check.
Check the full folded integer range before coercion and cover a wide negative
expression.
--
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]
