Baymine commented on code in PR #66505:
URL: https://github.com/apache/doris/pull/66505#discussion_r3812825689
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyConditionalFunction.java:
##########
@@ -123,4 +137,113 @@ nullIf, new Nullable(nullIf.child(0)), ctx.rewriteContext
return nullIf;
}
}
+
+ /*
+ * if(cond, x, x) => x
+ * Both branches are structurally identical, so the branch value is
returned regardless of
+ * the condition. Removing the condition is only sound when it cannot
change observable
+ * behavior, so the rewrite fires only when:
+ * 1. then and else are structurally equal;
+ * 2. the condition is deterministic (no rand()/now()/unique functions)
so dropping its
+ * evaluation cannot remove an observable side effect;
+ * 3. neither the condition NOR the branch itself contains a function
whose evaluation is
+ * observable even when deterministic and error-free, i.e.
NoneMovableFunction
+ * (contractually "should not prune", e.g. assert_true) or sleep() (a
deterministic
+ * ScalarFunction whose whole point is the blocking side effect — the
BE also refuses to
+ * fold it, see FoldConstantRuleOnBE). The branch must be checked
too: BE's
+ * VectorizedFnCall::_do_execute evaluates the then- and
else-argument columns
+ * unconditionally before FunctionIf selects between them, so
if(cond, sleep(1), sleep(1))
+ * already runs sleep() twice per block; collapsing it to a single
sleep(1) would halve
+ * that observable side effect even though the two branches are
structurally identical;
+ * 4. every subtree of the condition that may throw is also evaluated
UNCONDITIONALLY by the
+ * surviving branch, so removing the condition cannot suppress a
runtime error the original
+ * expression would have raised (e.g. Case3's ROUND(cost/denom,8)
appears both in the
+ * condition and unconditionally inside CEIL(...) in the branch).
+ * Nullability is preserved automatically: If.nullable() = then.nullable()
|| else.nullable(),
+ * which equals then.nullable() when the branches are identical.
+ */
+ private static Expression rewriteIf(ExpressionMatchingContext<If> ctx) {
+ If ifExpr = ctx.expr;
+ Expression condition = ifExpr.child(0);
+ Expression thenBranch = ifExpr.child(1);
+ Expression elseBranch = ifExpr.child(2);
+ if (!thenBranch.equals(elseBranch)) {
+ return ifExpr;
+ }
+ if (condition.containsNondeterministic()) {
Review Comment:
For the volatile expressions like rand(), non-deterministic UDFs, and unique
functions, the optimizer might not arbitrarily change their evaluation count or
location.
--
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]
