floation-cutie commented on code in PR #60892:
URL: https://github.com/apache/doris/pull/60892#discussion_r2938082534
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/SplitByString.java:
##########
@@ -60,10 +71,25 @@ private SplitByString(ScalarFunctionParams functionParams) {
*/
@Override
public SplitByString withChildren(List<Expression> children) {
- Preconditions.checkArgument(children.size() == 2);
+ Preconditions.checkArgument(children.size() == 2 || children.size() ==
3);
return new SplitByString(getFunctionParams(children));
}
+ @Override
+ public void checkLegalityBeforeTypeCoercion() {
+ checkLegalityAfterRewrite();
+ }
+
+ @Override
+ public void checkLegalityAfterRewrite() {
Review Comment:
If the expression `split_by_string('one,two,three,', ',', '-1')` need to be
supported, checkLegalityBeforeTypeCoercion should be removed as you said.
For checkLegalityAfterRewrite, I'd like to keep it because at that point
'-1' has already been folded to IntegerLiteral(-1) and passes the check. But
column references like split_by_string(v1, v2, k1) would still be correctly
rejected since the BE extracts the limit once in open() and doesn't support
per-row values. This is consistent with Sha2's pattern. Does this approach work
for you?
And I'll add more tests about this case
--
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]
