mrhhsg commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4221274088
##########
be/src/exprs/function/function_regexp.cpp:
##########
@@ -404,69 +398,103 @@ class FunctionRegexpReplace : public IFunction {
for (int i = 0; i < 3; ++i) {
col_const[i] =
is_column_const(*block.get_by_position(arguments[i]).column);
}
- argument_columns[0] = col_const[0] ? static_cast<const ColumnConst&>(
-
*block.get_by_position(arguments[0]).column)
- .convert_to_full_column()
- :
block.get_by_position(arguments[0]).column;
+ const auto& [source_column, source_const] =
+ unpack_if_const(block.get_by_position(arguments[0]).column);
+ argument_columns[0] = source_column;
default_preprocess_parameter_columns(argument_columns, col_const, {1,
2}, block, arguments);
+ if constexpr (std::is_same_v<FourParamTypes, ParamTypes>) {
+ // The regex of a constant pattern was not compiled in open
because the options were
+ // not evaluated there. Compile it once with the options of the
first row.
+ if (col_const[1] && !context->is_col_constant(3) &&
input_rows_count > 0 &&
Review Comment:
Fixed in f43a0c16a87. The execute-time compilation is now gated on
`context->is_col_constant(1)`, the same query-level constant check `open()`
uses, so a pattern that a lazy join presents as a physical `ColumnConst` for
one block is no longer cached and reused for later blocks. BE UT
`regexp_replace_pattern_changes_across_blocks` executes two blocks with
`ColumnConst` patterns `a` and `b` on one `FunctionContext` without a constant
column entry for the pattern and expects both rows replaced; both replacement
variants are covered. Your follow-up review confirmed the fix on that head.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -62,13 +63,53 @@ private DateTrunc(ScalarFunctionParams functionParams) {
super(functionParams);
}
+ @Override
+ public Expression prepareBeforeTypeCoercion() {
+ // When an argument is a date, the other one is the time unit, and the
signature does not need its value.
+ if (getArgument(0).getDataType().isDateLikeType() ||
getArgument(1).getDataType().isDateLikeType()) {
+ return this;
+ }
+ // Otherwise customSignature tells the time unit from the date value
by the literal time unit, so fold a
+ // constant string that evaluates to a time unit, unless the other
argument already is one. A string date
+ // value is kept unfolded, because folding it would change the derived
return type.
+ return withChildren((argument, index) -> {
+ if (!argument.getDataType().isStringLikeType() ||
isTimeUnit(getArgument(1 - index))) {
+ return argument;
+ }
+ Expression folded = ExpressionUtils.foldConstantArgument(argument);
+ return isTimeUnit(folded) ? folded : argument;
+ });
+ }
+
+ 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) {
+ for (int i = 0; i < 2; i++) {
Review Comment:
Fixed in f43a0c16a87 and reworked in dba6a527db7. A nonconstant argument,
e.g. a VARCHAR column, is the date value and a constant string beside it is the
time unit in both argument orders; the corresponding signature is selected and
BE validates the evaluated unit.
`ConstantFunctionArgumentTest.testConstantFeCannotFoldIsLeftToBe` covers
`date_trunc(s, lpad('mo', 2, 'nth'))` and the reversed order with a VARCHAR
column from `(select '2024-03-15' s) t`. dba6a527db7 moved this role inference
into a shared `dateArgumentIndex()` used by both
`checkLegalityBeforeTypeCoercion` and `customSignature`.
--
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]
