github-actions[bot] commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4147003023
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/AIAgg.java:
##########
@@ -83,24 +83,40 @@ public void checkLegalityAfterRewrite() {
if (!child(0).isLiteral() || !child(2).isLiteral()) {
throw new AnalysisException("AI_AGG must accept literal for
the resource name.");
}
+ checkResource();
+ }
+ }
- //Check if the resource is valid
- String resourceName =
getArgument(0).toString().replaceAll("^['\"]|['\"]$", "");
- Resource resource =
Env.getCurrentEnv().getResourceMgr().getResource(resourceName);
- if (!(resource instanceof AIResource)) {
- throw new AnalysisException("AI resource '" + resourceName +
"' does not exist");
+ @Override
+ public void checkLegalityBeforeTypeCoercion() {
+ // An aggregate function is not removed by constant folding, so a
constant task or resource name only has
+ // to be a literal in checkLegalityAfterRewrite, once the rewrite has
folded it. A literal resource name
+ // is already validated here.
+ if (!child(arity() - 1).isConstant()) {
Review Comment:
[P2] Resolve AI_AGG task constants before the DISTINCT fold skip. With a
configured AI resource and default FE folding, `ai_agg(DISTINCT 'resource', s,
concat('ta','sk'))` passes this `isConstant()` gate, but FE folding leaves
DISTINCT aggregates unchanged. `DistinctAggregateRewriter` runs later, and the
final legality check still requires a literal task, so this form fails while
the literal-task form passes that gate. Fold or resolve the task before the
final check for DISTINCT aggregates.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java:
##########
@@ -63,15 +65,30 @@ 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());
+ }
+ // a constant FE cannot fold is validated by BE when it is evaluated
+ if (!(digestLength instanceof Literal)) {
+ return;
Review Comment:
[P2] Keep deferred parameter validation reachable when another argument is
NULL. `sha2(NULL, 300 + crc32(''))` passes this branch, then FE's
`FoldConstantRuleOnFE.preProcess` replaces the `PropagateNullable` call with
NULL before BE can reject length 300; a literal 300 is rejected. Invalid
BE-only tokenize properties and split limits have the same gap, and BE's
all-NULL shortcut can also skip their runtime checks. Validate mandatory
constant parameters even on NULL-result paths.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java:
##########
@@ -63,15 +65,30 @@ 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());
+ }
+ // a constant FE cannot fold is validated by BE when it is evaluated
+ if (!(digestLength instanceof Literal)) {
Review Comment:
[P2] Preserve the integer-type check for deferred SHA2 lengths. `sha2('abc',
256.0 + crc32(''))` is constant but FE cannot fold `crc32`, so this nonliteral
return skips `IntegerLikeLiteral`. The INT signature then casts the decimal
expression and BE accepts 256, while the equivalent decimal literal fails
analysis. Check the original argument type before coercion even when BE must
evaluate its value.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/SplitByRegexp.java:
##########
@@ -79,9 +81,12 @@ 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; a constant FE cannot fold is validated by BE
when it is evaluated
+ Expression thirdArgument =
ExpressionUtils.foldConstantArgument(getArgument(2));
+ if (!thirdArgument.isConstant() || (thirdArgument instanceof
Literal
Review Comment:
[P2] Preserve the integer-type check for a deferred split limit.
`split_by_regexp('a,b,c', ',', 2.0 + crc32(''))` passes this nonliteral branch,
then the INT signature casts the decimal expression and BE accepts 2. The
equivalent decimal literal is rejected here. Validate the original constant
argument type before signature coercion.
--
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]
