mrhhsg commented on code in PR #68579:
URL: https://github.com/apache/doris/pull/68579#discussion_r4236402708


##########
be/src/exprs/aggregate/aggregate_function_null.h:
##########
@@ -711,6 +711,11 @@ class AggregateFunctionNullVariadicInline final
 
     void check_input_columns_type(const IColumn** columns) const override {
         IAggregateFunction::check_input_columns_type(columns);
+        if constexpr (requires(NestFuction& function, const IColumn** 
input_columns) {
+                          function.check_nullable_input_columns(input_columns);

Review Comment:
   Fixed in 4ff53fca1aa. `AggregateFunctionSequenceBase` gets a 
`check_nullable_input_columns` hook (the same mechanism 
`OrthogonalBitmapExprCalculate` uses), called unconditionally by the generic 
nullable wrapper before any row can be filtered. It rejects a NULL pattern 
explicitly and runs `parse_pattern()` on it, so a NULL pattern and an 
invalid-syntax pattern are both caught up front, regardless of whether `dt` or 
any other argument is NULL for every row. BE unit tests cover both 
`sequence_match` and `sequence_count` for a NULL pattern and for an invalid 
pattern beside an all-NULL timestamp; the regression suite adds matching cases.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayApply.java:
##########
@@ -56,22 +58,25 @@ public class ArrayApply extends ScalarFunction
      */
     public ArrayApply(Expression arg0, Expression arg1, Expression arg2) {
         super("array_apply", arg0, arg1, arg2);
-        checkArguments(arg0, arg1, arg2);
     }
 
     /** constructor for withChildren and reuse signature */
     private ArrayApply(ScalarFunctionParams functionParams) {
         super(functionParams);
-        checkArguments(
-                functionParams.arguments.get(0), 
functionParams.arguments.get(1), functionParams.arguments.get(2)
-        );
     }
 
-    private void checkArguments(Expression arg0, Expression arg1, Expression 
arg2) {
-        if (!(arg1 instanceof StringLikeLiteral)) {
+    @Override
+    public void checkLegalityBeforeTypeCoercion() {
+        // validate the op FE can evaluate here, because constant folding may 
remove this function before any
+        // later check
+        Expression arg1 = ExpressionUtils.foldConstantArgument(getArgument(1));
+        Expression arg2 = getArgument(2);
+        if (!arg1.isConstant() || (arg1 instanceof Literal && !(arg1 
instanceof StringLikeLiteral))) {

Review Comment:
   Fixed in 4ff53fca1aa for all three. `use_default_implementation_for_nulls()` 
is now disabled for `array_apply`, `sha2`, and the 3-argument 
`split_by_regexp`, so the function always runs instead of the generic 
nullable-argument shortcut returning NULL before op/the digest length/the limit 
is checked. Each function now rejects an actual NULL for that argument 
explicitly (the same error as the literal-NULL case), then replicates the 
nullable wrapping by hand for the remaining arguments (the source 
array/data/pattern) so the declared return type and per-row NULL propagation 
stay correct. BE unit tests cover a NULL op/digest length/limit coming from a 
BE-only expression (`IF` with a NULL branch); the regression suite adds 
matching cases.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/DateTimeWithPrecision.java:
##########
@@ -43,6 +44,12 @@ public DateTimeWithPrecision(ScalarFunctionParams 
functionParams) {
         super(functionParams);
     }
 
+    @Override
+    public Expression prepareBeforeTypeCoercion() {

Review Comment:
   Fixed in 4ff53fca1aa. `computeSignature` now reads the folded literal with 
`getLongValue()` and does the 0..`TimeStampNsType.SCALE` range check as a 
`long` before narrowing to `int`, so a BIGINT value like 4294967299 is rejected 
instead of wrapping to 3 and passing the check. FE unit test and the regression 
suite cover `now(cast(4294967299 as bigint) + cast(0 as bigint))`.



-- 
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]

Reply via email to