mrhhsg commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4221277443
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -62,13 +63,57 @@ 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++) {
+ // The other argument is the time unit when this one is a
date-typed value, or when this one
+ // is simply nonconstant (e.g. a VARCHAR date column) and the
other side can only be the unit.
+ boolean thisArgIsDateRole =
getArgument(i).getDataType().isDateLikeType()
+ || !getArgument(i).isConstant();
Review Comment:
Fixed in dba6a527db7. Both `checkLegalityBeforeTypeCoercion` and
`customSignature` now use a shared `dateArgumentIndex()`: a date-typed or
nonconstant argument is the date value; otherwise a literal time unit (folded
by the preparation step) marks the other argument as the date; otherwise the
constant FE evaluates to a string literal is the date value and the other one
is the time unit BE validates. So `date_trunc('2024-03-15', lpad('nth', 5,
'mo'))`, `date_trunc(concat('2024-03-15', ''), lpad('nth', 5, 'mo'))` and the
reversed orders are accepted, and a literal date string with timezone
information keeps deriving TIMESTAMPTZ beside such a unit, as beside a literal
one. Tests: `testConstantFeCannotFoldIsLeftToBe` checks both orders for a
literal date, a zoned literal date (`TimeStampTzType`, same type as with
`'month'`) and a `concat(...)` date (`DATETIMEV2(6)`), and that `concat('mon',
'x')` is still rejected beside a literal date; the regression adds
`qt_date_trunc_string_date_b
e`, a CTAS whose `desc` shows `timestamptz` for the BE-evaluated unit in both
orders and `datetime(6)` for the `concat` date, and the BE error for
`date_trunc('2024-03-15 10:00:00', lpad('x', 3, 'mo'))`.
##########
be/src/exprs/function/function_regexp.cpp:
##########
@@ -404,69 +398,108 @@ 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.
+ // Gate this on the query-level constant-ness of the pattern (as
open() does), not on
+ // col_const[1]: a lazy join can broadcast a non-constant probe
pattern as a physical
+ // ColumnConst for one block, which would otherwise cache that
block's pattern and
+ // wrongly reuse it for later blocks with a different pattern
value.
+ if (context->is_col_constant(1) && !context->is_col_constant(3) &&
+ input_rows_count > 0 &&
+ context->get_function_state(FunctionContext::THREAD_LOCAL) ==
nullptr) {
+ RETURN_IF_ERROR(compile_constant_pattern(
+ context, argument_columns[1]->get_data_at(0),
+
block.get_by_position(arguments[3]).column->get_data_at(0)));
+ }
+ }
+
StringRef options_value;
if (col_const[1] && col_const[2]) {
- Impl::execute_impl_const_args(context, argument_columns,
options_value,
+ Impl::execute_impl_const_args(context, argument_columns,
source_const, options_value,
Review Comment:
Fixed in dba6a527db7. `execute_impl` reads the options of the first row once
before the execute-time compilation and the two branches, and passes that value
to the physical-constant path, the mixed path and `compile_constant_pattern`,
so an empty constant pattern compiled per row gets the requested option in
every path. BE UT `regexp_replace_empty_pattern_options` (empty `ColumnConst`
pattern, `ColumnConst` replacement `\x`, full options column
`ignore_invalid_escape`, no constant column entry for the options) expects
`xax`/`xbx` for `regexp_replace` and `xa`/`xb` for `regexp_replace_one`; the
regression `order_qt_regexp_replace_empty_pattern_be` runs your statement over
`numbers('number' = '3')` and yields `xax` / `xa` on every row.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/SplitByRegexp.java:
##########
@@ -79,9 +80,13 @@ public SplitByRegexp withChildren(List<Expression> children)
{
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)) {
+ // validate the value FE can evaluate here, because constant
folding may remove this function before
+ // any later check; the type is checked before the type coercion
casts the argument to the INT
+ // signature, and the value of a constant FE cannot fold is
validated by BE when it is evaluated
+ Expression thirdArgument =
ExpressionUtils.foldConstantArgument(getArgument(2));
+ if (!thirdArgument.isConstant() ||
!thirdArgument.getDataType().isIntegralType()
+ || (thirdArgument instanceof IntegerLikeLiteral
Review Comment:
Fixed in dba6a527db7. The check rejects a `NullLiteral` explicitly, like the
sha2 digest length check, so `split_by_regexp('a,b', ',', cast(null as int))`
reports "must be a positive constant" again. Covered by
`ConstantFunctionArgumentTest.testScalarFunctionValueIsCheckedBeforeTypeCoercion`
and a `test {}` in `fold_literal_arguments`.
--
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]
