github-actions[bot] commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4130774383
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -62,13 +64,41 @@ private DateTrunc(ScalarFunctionParams functionParams) {
super(functionParams);
}
+ @Override
+ public boolean needFoldToLiteral(int index) {
+ // a string argument may be the time unit unless the other argument is
already a literal time unit
+ return getArgument(index).getDataType().isStringLikeType() &&
!isTimeUnit(getArgument(1 - index));
+ }
+
+ @Override
+ public boolean acceptFoldedLiteral(int index, Literal folded) {
+ // Only the time unit is replaced by its folded literal. A string date
value is kept unfolded, because
+ // folding it would change the derived return type. When the other
argument is a date, the folded
+ // literal is the time unit and an illegal value is reported by
checkLegalityBeforeTypeCoercion.
+ return isTimeUnit(folded) || getArgument(1 -
index).getDataType().isDateLikeType();
+ }
+
+ private static boolean isTimeUnit(Expression expression) {
+ return expression instanceof StringLikeLiteral
+ && LEGAL_TIME_UNIT.contains(((StringLikeLiteral)
expression).getStringValue().toLowerCase());
+ }
+
+ private static boolean isConstantString(Expression expression) {
+ return expression.isConstant() &&
expression.getDataType().isStringLikeType();
+ }
+
@Override
public void checkLegalityBeforeTypeCoercion() {
boolean firstArgIsStringLiteral =
getArgument(0).isConstant() && getArgument(0) instanceof
StringLikeLiteral;
boolean secondArgIsStringLiteral =
getArgument(1).isConstant() && getArgument(1) instanceof
StringLikeLiteral;
if (!firstArgIsStringLiteral && !secondArgIsStringLiteral) {
+ // a time unit FE cannot fold is accepted beside a date argument,
and BE validates it
+ if ((getArgument(0).getDataType().isDateLikeType() &&
isConstantString(getArgument(1)))
Review Comment:
[P2] Validate the evaluated `date_trunc` unit exactly. `date_trunc(DATE
'2024-03-15', concat('month', crc32('')))` now passes FE because its unit is
constant but not an FE literal; the evaluated unit is `month0`. BE
`DateTrunc::create_state` compares only the first five bytes with `month`, so
it returns March 1, whereas the equivalent literal `month0` is rejected. Check
the entire evaluated unit in BE for both argument orders.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/SequenceFunction.java:
##########
@@ -20,31 +20,54 @@
import org.apache.doris.analysis.FunctionCallExpr;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.FoldLiteralArguments;
import org.apache.doris.nereids.trees.expressions.functions.FunctionTrait;
+import org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.expressions.literal.StringLikeLiteral;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/** SequenceFunction */
-public interface SequenceFunction extends FunctionTrait {
+public interface SequenceFunction extends FunctionTrait, FoldLiteralArguments {
Pattern EVENT_PATTERN = Pattern.compile("\\(\\?(\\d+)\\)");
+ @Override
+ default boolean needFoldToLiteral(int index) {
+ // the pattern
+ return index == 0;
+ }
+
@Override
default void checkLegalityBeforeTypeCoercion() {
String functionName = getName();
Expression firstArg = getArgument(0);
- if (!(firstArg instanceof StringLikeLiteral)) {
+ if (!firstArg.isConstant() || (firstArg instanceof Literal &&
!(firstArg instanceof StringLikeLiteral))) {
throw new AnalysisException("The pattern param `" +
firstArg.toSql() + "` of " + functionName
- + " function must be string literal, but it is "
+ + " function must be string constant, but it is "
+ firstArg.getClass().getSimpleName());
}
if (!getArgumentType(1).isDateLikeType()) {
throw new AnalysisException("The timestamp params of " +
functionName
+ " function must be DATE, DATETIME, TIMESTAMP_NS or
TIMESTAMPTZ, but it is "
+ getArgumentType(1));
}
- String pattern = ((StringLikeLiteral) firstArg).getStringValue();
+ // a pattern FE cannot fold is parsed by BE when it is evaluated
+ if (firstArg instanceof StringLikeLiteral) {
Review Comment:
[P2] Return an error for invalid evaluated sequence patterns. A BE-only
constant such as `lpad('(?9)', 4, '(')` bypasses `checkPattern`, while BE
`parse_pattern` only logs and clears its conditions for the out-of-range event
reference. `sequence_match`/`sequence_count` then return normal false/zero
results, as the new regression expects, although the identical literal fails
analysis. Propagate the parse error instead of producing a valid-looking
aggregate result.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/TopN.java:
##########
@@ -104,7 +110,9 @@ public void checkLegalityAfterRewrite() {
if (topNCount.isNullLiteral()) {
return;
}
- if (!(topNCount instanceof Literal) || ((Literal)
topNCount).getDouble() <= 0) {
+ // a constant FE cannot fold is evaluated by BE
+ if (!topNCount.isConstant()
Review Comment:
[P1] Reject nonpositive TopN counts before computing BE capacity. `topn(s,
crc32('') - 1)` is newly accepted because the positivity check runs only for
literals; `TopNArray` and `TopNWeighted` have the same relaxation. BE casts -1
to `uint64_t` before multiplying by 50, so partial-state serialization can
include every distinct candidate instead of the bounded set, while `topn`
returns `{}`. Validate the evaluated count in BE for all three variants before
setting capacity.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Tokenize.java:
##########
@@ -58,13 +60,25 @@ private Tokenize(ScalarFunctionParams functionParams) {
super(functionParams);
}
+ @Override
+ public boolean needFoldToLiteral(int index) {
+ return index == 1;
+ }
+
@Override
public void checkLegalityBeforeTypeCoercion() {
Expression rightChild = getArgument(1);
// tokenize(k7, null) could return NULL
if (rightChild instanceof NullLiteral) {
return;
}
+ if (!rightChild.isConstant()) {
+ throw new AnalysisException("tokenize second argument must be a
string constant");
+ }
+ // a constant FE cannot fold is parsed by BE when it is evaluated
+ if (!(rightChild instanceof Literal)) {
Review Comment:
[P2] Reject malformed evaluated tokenize properties. `tokenize('hello
world', lpad('x', 1, 'x'))` now bypasses FE property parsing; BE `parse`
returns OK after matching no key/value pair, selects parser `none`, and returns
the original string as one token. The identical literal is rejected. Require
the BE parser to consume and validate the evaluated properties before choosing
defaults.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/SplitByRegexp.java:
##########
@@ -75,13 +77,20 @@ public SplitByRegexp withChildren(List<Expression>
children) {
return new SplitByRegexp(getFunctionParams(children));
}
+ @Override
+ public boolean needFoldToLiteral(int index) {
+ return index == 2;
+ }
+
@Override
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)) {
+ // a constant FE cannot fold is passed to BE, which takes a
negative limit as unlimited
+ if (!thirdArgument.isConstant() || (thirdArgument instanceof
Literal
+ && (!(thirdArgument instanceof IntegerLikeLiteral)
+ || ((IntegerLikeLiteral) thirdArgument).getIntValue() <
0))) {
Review Comment:
[P2] Preserve the nonnegative limit check for BE-only constants.
`split_by_regexp('a,b,c', ',', crc32('') - 1)` is newly accepted because only
literal limits are range-checked. BE treats -1 as unlimited and returns a split
result, while the equivalent literal -1 fails analysis; the new regression
records this mismatch. Validate the evaluated limit before splitting.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java:
##########
@@ -61,17 +63,32 @@ private Sha2(ScalarFunctionParams functionParams) {
super(functionParams);
}
+ @Override
+ public boolean needFoldToLiteral(int index) {
+ return index == 1;
+ }
+
@Override
public void checkLegalityBeforeTypeCoercion() {
checkLegalityAfterRewrite();
}
@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());
+ Expression digestLength = getArgument(1);
+ 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:
[P1] Handle a literal input when the valid digest length is a full BE
column. With BE constant folding disabled, `sha2('abc', 256 +
uniform(1,10,crc32('x')) % uniform(1,10,crc32('x')))` passes this new FE branch
and always has digest 256. BE Uniform emits full columns, so the all-constant
wrapper leaves `'abc'` as `ColumnConst`; SHA2 then DCHECKs that it is not const
or throws because it accepts only raw string columns. Materialize or unpack the
literal input in the mixed-column execution path.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/SplitByRegexp.java:
##########
@@ -75,13 +77,20 @@ public SplitByRegexp withChildren(List<Expression>
children) {
return new SplitByRegexp(getFunctionParams(children));
}
+ @Override
+ public boolean needFoldToLiteral(int index) {
+ return index == 2;
+ }
+
@Override
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)) {
+ // a constant FE cannot fold is passed to BE, which takes a
negative limit as unlimited
+ if (!thirdArgument.isConstant() || (thirdArgument instanceof
Literal
Review Comment:
[P1] Keep the literal source row-aligned when the limit is a full column.
`split_by_regexp('a,b', ',', uniform(1,10,crc32('x')))` over a multirow
`numbers` input now passes FE with a positive constant limit, but BE Uniform
emits a full column. The function unwraps the literal source and pattern to
one-row nested columns, takes the `right_const` branch, and
`_execute_constant_pattern` calls `src_column_string.get_data_at(row)` for
every input row. Row 1 hits the bounds DCHECK or reads past the source offsets.
Respect `left_const` in that branch or materialize the source; add a multirow
regression.
--
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]
