mrhhsg commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4131582251
##########
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:
Fixed in b4c6f6d. The three-argument `split_by_regexp` now returns "the
third parameter of split_by_regexp function must be a positive constant" for a
negative evaluated limit (0 stays valid, as on FE; the two-argument form still
uses -1 internally for no limit). `split_by_regexp('a,b,c', ',', crc32('') -
1)` is now rejected, and the regression block that recorded the unlimited
result was replaced by a `test{}` case. Also covered by BE UT
`FunctionConstantArgumentTest.split_by_regexp_limit`.
##########
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:
Fixed in b4c6f6d. SHA2 now calls `convert_to_full_column_if_const()` on its
input instead of asserting it is not const, so a literal input next to a digest
length that BE evaluates to a full column works; it is a no-op in the existing
shapes. `sha2('abc', if(crc32('') = 0, 256, 224))` over `numbers('number' =
'3')` returns the SHA-256 of `abc` on every row (regression
`full_column_constant_be`, BE UT
`FunctionConstantArgumentTest.sha2_constant_input`).
##########
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:
Fixed in b4c6f6d. The constant pattern branch now receives `left_const` and
reads the source with `index_check_const(row, src_const)`, so a literal source
stays row-aligned when the limit is a full column. `split_by_regexp('a,b,c',
',', if(crc32('') = 0, 2, 3))` and `split_by_regexp(..., uniform(1, 10,
crc32('x')))` over `numbers('number' = '3')` are covered by the multirow
regression `full_column_constant_be` and BE UT
`FunctionConstantArgumentTest.split_by_regexp_limit` (3-row ColumnConst source
and pattern with a full-column limit).
--
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]
