Skip to content

Commit

Permalink
fix some inner join bug
Browse files Browse the repository at this point in the history
  • Loading branch information
seawinde committed Dec 10, 2023
1 parent 16230b5 commit f7f5437
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected Plan rewriteQueryByView(MatchMode matchMode,
queryToViewSlotMappings
);
// Can not rewrite, bail out
if (expressionsRewritten == null
if (expressionsRewritten.isEmpty()
|| expressionsRewritten.stream().anyMatch(expr -> !(expr instanceof NamedExpression))) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
import org.apache.doris.nereids.util.ExpressionUtils;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;

import java.util.ArrayList;
Expand Down Expand Up @@ -199,7 +200,7 @@ protected List<Expression> rewriteExpression(
wiAlias);
if (replacedExpression.anyMatch(slotsToRewrite::contains)) {
// if contains any slot to rewrite, which means can not be rewritten by target, bail out
return null;
return ImmutableList.of();
}
rewrittenExpressions.add(replacedExpression);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ private void init() {
// if inner join add where condition
Set<Expression> predicates = new HashSet<>();
nodePlan.accept(PREDICATE_COLLECTOR, predicates);
predicates.forEach(this.predicates::addPredicate);
predicates.forEach(predicate ->
ExpressionUtils.extractConjunction(predicate).forEach(this.predicates::addPredicate));
});

// TODO Collect predicate from top plan not in hyper graph, should optimize, twice now
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@

import org.apache.doris.catalog.TableIf.TableType;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.ArrayItemReference.ArrayItemSlot;
import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter;
import org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionVisitor;
import org.apache.doris.nereids.trees.plans.Plan;
Expand Down Expand Up @@ -74,7 +73,8 @@ public Expression visitNamedExpression(NamedExpression namedExpression,
}

/**
* The Collector for target named expressions
* The Collector for target named expressions in the whole plan, and will be used to
* replace the target expression later
* TODO Collect named expression by targetTypes, tableIdentifiers
*/
public static class NamedExpressionCollector
Expand All @@ -83,15 +83,9 @@ public static class NamedExpressionCollector
public static final NamedExpressionCollector INSTANCE = new NamedExpressionCollector();

@Override
public Void visitSlotReference(SlotReference slotReference, ExpressionReplaceContext context) {
context.getExprIdExpressionMap().put(slotReference.getExprId(), slotReference);
return super.visitSlotReference(slotReference, context);
}

@Override
public Void visitArrayItemSlot(ArrayItemSlot arrayItemSlot, ExpressionReplaceContext context) {
context.getExprIdExpressionMap().put(arrayItemSlot.getExprId(), arrayItemSlot);
return super.visitArrayItemSlot(arrayItemSlot, context);
public Void visitSlot(Slot slot, ExpressionReplaceContext context) {
context.getExprIdExpressionMap().put(slot.getExprId(), slot);
return super.visit(slot, context);
}

@Override
Expand Down Expand Up @@ -121,9 +115,15 @@ public ExpressionReplaceContext(List<Expression> targetExpressions,
this.targetExpressions = targetExpressions;
this.targetTypes = targetTypes;
this.tableIdentifiers = tableIdentifiers;
// collect only named expressions and replace them with linage identifier later
// collect the named expressions used in target expression and will be replaced later
this.exprIdExpressionMap = targetExpressions.stream()
.map(each -> each.collectToList(NamedExpression.class::isInstance))
.map(each -> {
// if Alias, shuttle form the child of alias to retain the alias
if (each instanceof Alias && !each.children().isEmpty()) {
return each.child(0).collectToList(NamedExpression.class::isInstance);
}
return each.collectToList(NamedExpression.class::isInstance);
})
.flatMap(Collection::stream)
.map(NamedExpression.class::cast)
.collect(Collectors.toMap(NamedExpression::getExprId, expr -> expr));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ public static Expression replace(Expression expr, Map<? extends Expression, ? ex
*/
public static Expression replace(Expression expr, Map<? extends Expression, ? extends Expression> replaceMap,
boolean withAlias) {
return expr.accept(ExpressionReplacer.INSTANCE, ExpressionReplacerContext.of(replaceMap, true));
return expr.accept(ExpressionReplacer.INSTANCE, ExpressionReplacerContext.of(replaceMap, withAlias));
}

/**
Expand Down

0 comments on commit f7f5437

Please sign in to comment.