github-actions[bot] commented on code in PR #66482:
URL: https://github.com/apache/doris/pull/66482#discussion_r3754766217
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SubqueryToApply.java:
##########
@@ -503,6 +535,46 @@ private Pair<LogicalPlan, Optional<Expression>>
addApply(SubqueryExpr subquery,
return Pair.of(logicalProject, newCorrelatedOuterExpr);
}
+ /**
+ * simplify the conjunct that contains mark join slots and infer the
behavior of each
+ * mark join slot, return the rewritten conjunct together with the mark
slots info.
+ *
+ * for each mark slot, the pair in the returned map has:
+ * Pair.first: whether the null and false values of the mark slot are
indistinguishable,
+ * i.e. the mark slot can be treated as a non-nullable
boolean. it only affects
+ * how the mark value is computed (treating null as false) and
never changes the
+ * number of output rows, so it's safe for every join type and
the filter.
+ * Pair.second: whether the original mark join can be directly eliminated
and turned into a
+ * plain semi join. a plain semi join only outputs the
matched rows, while a
+ * mark join keeps all original rows and adds a mark column,
so eliminating the
+ * mark join is only safe when discarding the unmatched rows
is already part of
+ * the containing join's semantics (inner, cross and semi
joins).
+ *
+ * when Pair.second is true, the mark slot is replaced by the true literal
in the returned
+ * conjunct, and the caller can drop the mark join slot to turn the mark
join into a plain
+ * semi join.
+ */
+ private Pair<Expression, Map<MarkJoinSlotReference, Pair<Boolean,
Boolean>>> simplifyConjunctWithMarkJoinSlot(
+ Expression conjunct, Plan plan, CascadesContext cascadesContext) {
+ ExpressionRewriteContext rewriteContext = new
ExpressionRewriteContext(plan, cascadesContext);
+ Map<MarkJoinSlotReference, Pair<Boolean, Boolean>> markSlotsInfo;
+ if (conjunct.containsType(MarkJoinSlotReference.class)) {
+ markSlotsInfo = ExpressionUtils.inferMarkSlotNotNullMap(conjunct,
rewriteContext);
+ } else {
+ markSlotsInfo = Maps.newHashMap();
+ }
+ Map<MarkJoinSlotReference, BooleanLiteral> replaceMap =
Maps.newHashMap();
+ for (Map.Entry<MarkJoinSlotReference, Pair<Boolean, Boolean>> entry :
markSlotsInfo.entrySet()) {
+ if (entry.getValue().second) {
Review Comment:
[P1] Do not eliminate a mark join across non-movable evaluation
Pair.second is a row-truth proof, but dropping the Apply also changes which
rows reach other expressions. For example, after subquery replacement:
```text
Filter(ifnull(ifnull(M,FALSE) AND assert_true(guard,'bad'), FALSE))
Project(..., M)
Apply(IN, mark=M)
Scan(t)
Scan(s)
```
Both M=FALSE and M=NULL fold to FALSE, so this loop replaces M with TRUE and
`addApply` turns the mark Apply into a semi join. In the original plan a mixed
match/nonmatch batch reaches the filter; `VCompoundPred` evaluates the RHS for
that batch, so `assert_true` can throw on an unmatched `guard=FALSE` row. The
semi join prunes that row first and suppresses the error, violating
`AssertTrue`'s `NoneMovableFunction` contract. Please fence Pair.second when
the conjunct contains `NoneMovableFunction` (and audit volatile expressions for
the same evaluation-domain issue), with an error-behavior regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SubqueryToApply.java:
##########
@@ -118,20 +118,15 @@ public List<Rule> buildRules() {
ctx.statementContext,
shouldOutputMarkJoinSlot.get(i));
SubqueryContext context = new
SubqueryContext(subqueryExprs);
Expression conjunct =
replaceSubquery.replace(oldConjuncts.get(i), context);
- // TODO: The way to optimize null aware mark join is
not right.
- // remove it temporary until we refactor it.
- // ExpressionRewriteContext rewriteContext = new
ExpressionRewriteContext(ctx.cascadesContext);
- // boolean isMarkSlotNotNull =
conjunct.containsType(MarkJoinSlotReference.class)
- // ?
ExpressionUtils.canInferNotNullForMarkSlot(
- //
TrySimplifyPredicateWithMarkJoinSlot.INSTANCE.rewrite(conjunct,
- // rewriteContext),
rewriteContext)
- // : false;
- boolean isMarkSlotNotNull = false;
+ Pair<Expression, Map<MarkJoinSlotReference,
Pair<Boolean, Boolean>>> simplifyResult =
Review Comment:
[P1] Preserve mark-free subtrees below NULL-observing wrappers
This enables Pair.first for arbitrary filter expressions, but the
simplifier's local `And`/`Or` neutral elements are not sound when a parent
observes their value. A reduced failing tree is:
```text
Filter(ifnull(M, flag OR FALSE))
Project(key, flag, M)
Apply(IN, mark=M)
Scan(t)
Scan(s)
```
`TrySimplifyPredicateWithMarkJoinSlot` rewrites the mark-free `flag OR
FALSE` subtree to FALSE, so inference sees `ifnull(M,FALSE)` and returns
Pair.first=true. For an outer row with `key=NULL`, `flag=TRUE`, and a nonempty
`s`, SQL has `M=NULL` and keeps the row (`ifnull(NULL,TRUE)=TRUE`). Pair.first
then lets `InApplyToJoin` move the nullable equality out of `markConjuncts`,
computing M as FALSE, and the same filter rejects the row. Please restrict this
proof to contexts where no wrapper can observe NULL (or conservatively model
the non-mark inputs through the whole expression), and add this NULL-bearing
result regression.
--
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]
