morrySnow commented on code in PR #50245: URL: https://github.com/apache/doris/pull/50245#discussion_r2052191078
########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpReplace.java: ########## @@ -52,13 +59,44 @@ public RegexpReplace(Expression arg0, Expression arg1, Expression arg2) { super("regexp_replace", arg0, arg1, arg2); } + /** + * constructor with 4 arguments. + */ + public RegexpReplace(Expression arg0, Expression arg1, Expression arg2, Expression arg3) { + super("regexp_replace", arg0, arg1, arg2, arg3); + } + /** * withChildren. */ @Override public RegexpReplace withChildren(List<Expression> children) { - Preconditions.checkArgument(children.size() == 3); - return new RegexpReplace(children.get(0), children.get(1), children.get(2)); + Preconditions.checkArgument(children.size() == 3 || children.size() == 4, + "RegexpReplace should have 3 or 4 children, but got: " + children.size()); + if (children.size() == 3) { + return new RegexpReplace(children.get(0), children.get(1), children.get(2)); + } else { + return new RegexpReplace(children.get(0), children.get(1), children.get(2), children.get(3)); + } + } + + @Override + public void checkLegalityBeforeTypeCoercion() { + if (children().size() == 3) { + return; + } + if (children().size() == 4) { + Expression value = child(3); + DataType type = value.getDataType(); + if (!type.isStringLikeType()) { + throw new AnalysisException( + "The fourth param of regexp_replace must be a string type: " + this.toSql()); + } + if (!value.isConstant()) { Review Comment: in planner, `isConstant` means there is not slot in the expr tree. so a function expr could return true when eval `isConstant`. if the last parameter could be a function, why not must be constant? ########## regression-test/suites/nereids_syntax_p0/test_regexp_replace.groovy: ########## @@ -36,4 +36,24 @@ suite("test_regexp_replace") { (5, null);""" qt_replace_in_table_chinese """SELECT id, regexp_replace(name, '\\\\p{Han}', '汉') as replaced_name FROM test_table_for_regexp;""" + + qt_replace_ignore1 """select regexp_replace('{"abc":5},{"def":78}', '\\}\\,\\{', '\\}&&\\{', 'IGNORE_INVALID_ESCAPE');""" + qt_replace_ignore2 """select regexp_replace('abc', 'b', "\\}", 'IGNORE_INVALID_ESCAPE');""" + qt_replace_ignore3 """select regexp_replace_one('{"abc":5},{"def":78}', '\\}\\,\\{', '\\}&&\\{', 'IGNORE_INVALID_ESCAPE');""" + qt_replace_ignore4 """select regexp_replace_one('abc', 'b', '\\}', 'IGNORE_INVALID_ESCAPE');""" Review Comment: how this sql could exec since u not register regexp_replace_one in `fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinScalarFunctions.java` -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org