github-actions[bot] commented on code in PR #62742:
URL: https://github.com/apache/doris/pull/62742#discussion_r3354660430
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AddProjectForVolatileExpression.java:
##########
@@ -269,6 +271,85 @@ public <T extends Expression> Optional<Pair<List<T>,
LogicalProject<Plan>>> rewr
return Optional.of(Pair.of(newTargetsBuilder.build(), new
LogicalProject<>(projects, plan.child(0))));
}
+ private Optional<JoinRewriteResult>
rewriteJoinExpressions(LogicalJoin<Plan, Plan> join,
+ Collection<Expression> targets) {
+ Map<Expression, Integer> volatileExpressionCounter =
Maps.newLinkedHashMap();
+ Map<Expression, Set<Slot>> volatileExpressionSlots =
Maps.newLinkedHashMap();
+ for (Expression target : targets) {
+ target.foreach(e -> {
+ Expression expr = (Expression) e;
+ if (expr.isVolatile()) {
+ volatileExpressionCounter.merge(expr, 1, Integer::sum);
+ Set<Slot> inputSlots = expr.getInputSlots();
+ volatileExpressionSlots
+ .computeIfAbsent(expr, ignored ->
Sets.newLinkedHashSet())
+ .addAll(inputSlots.isEmpty() ?
target.getInputSlots() : inputSlots);
+ }
+ });
+ }
+
+ ImmutableList.Builder<NamedExpression> leftAliases =
ImmutableList.builder();
+ ImmutableList.Builder<NamedExpression> rightAliases =
ImmutableList.builder();
+ Map<Expression, Slot> replaceMap = Maps.newHashMap();
+ Set<Slot> leftOutputSet = join.left().getOutputSet();
+ Set<Slot> rightOutputSet = join.right().getOutputSet();
+ for (Entry<Expression, Integer> entry :
volatileExpressionCounter.entrySet()) {
+ if (entry.getValue() <= 1) {
+ continue;
Review Comment:
This guard still lets an empty-slot volatile expression be projected into
one child when the containing join conjunct references both sides. For example,
after BETWEEN expansion, `t1.id + rand() between t2.id and t2.id + 1`
contributes the same `rand()` node twice; `volatileInputSlots` is empty, while
`inputSlots` collected from the containing targets includes both left and right
slots. The current guard does not skip that case, then the side selection below
falls through to `leftAliases`, so `rand()` is evaluated once per left row and
reused for every matching right row. The original ON predicate evaluates it per
joined pair, so with one left row matching two right rows the original can keep
one pair and reject the other, while the rewritten plan can only keep both or
neither. Please also skip slot-free volatile expressions when the collected
containing `inputSlots` are not fully covered by either child, or otherwise
keep them in the join without child projection. This is distinc
t from the existing direct ON-pushdown comments because it is introduced by
the new repeated-volatile materialization path after the volatile expression
has been selected for aliasing.
--
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]
