github-actions[bot] commented on code in PR #67818:
URL: https://github.com/apache/doris/pull/67818#discussion_r4002155712
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/CollectList.java:
##########
@@ -39,7 +39,7 @@
* AggregateFunction 'collect_list'. This class is generated by
GenerateFunction.
*/
public class CollectList extends NotNullableAggregateFunction
- implements UnaryExpression, ExplicitlyCastableSignature {
+ implements UnaryExpression, ExplicitlyCastableSignature,
NullIgnoringAggregateFunction {
Review Comment:
[P1] Fence non-movable aggregate arguments before inferring a child filter
This marker also enables `InferAggNotNull`, which reasons only about the
argument's folded NULL result and then inserts the inferred predicate below the
aggregate. For example, with a nullable `v`,
`collect_list(CAST(IF(assert_true(random() < 0, 'boom'), v, NULL) AS BIGINT))`
keeps the full expression inside the aggregate during normalization.
Substituting NULL for `v` makes the two `IF` branches equal, so inference
produces `v IS NOT NULL`; the resulting child filter removes the NULL row
before `assert_true` is evaluated. The original plan raises, whereas the
rewritten plan returns the empty-list identity, and a volatile-only expression
likewise changes evaluation cardinality. Please reject null inference when an
aggregate argument contains `NoneMovableFunction`/volatile expressions, or
materialize the complete argument below the generated filter while preventing
later pushdown from crossing that boundary.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/BoolOr.java:
##########
@@ -35,7 +35,7 @@
* AggregateFunction 'bool_or'.
*/
public class BoolOr extends NullableAggregateFunction
- implements UnaryExpression, ExplicitlyCastableSignature {
+ implements UnaryExpression, ExplicitlyCastableSignature,
NullIgnoringAggregateFunction {
Review Comment:
[P1] Preserve sensitive child predicates during null inference
This marker makes a bare-slot query such as `SELECT bool_or(v) FROM t WHERE
assert_true(random() < 0, 'boom')` eligible for `InferAggNotNull`. On an
all-NULL `v`, the rule adds `v IS NOT NULL`; filter pushdown moves it through
the slot-only project and `MergeFilters` combines it ahead of the existing
assertion. The backend then evaluates the cheaper null check first and
short-circuits the all-false block, so the query returns the empty aggregate
result instead of raising as the original plan does. This is distinct from the
complex-argument case because the aggregate argument here is only `v`; an
argument-only fence will not help. Please treat volatile/`NoneMovableFunction`
expressions in the aggregate child evaluation domain as barriers to this
inferred filter, and add an all-NULL regression for this shape.
--
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]
