-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for LEFT/RIGHT/FULL/INNER lateral join #390
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,6 @@ | |
import io.prestosql.sql.planner.plan.ProjectNode; | ||
import io.prestosql.sql.planner.plan.SimplePlanRewriter; | ||
import io.prestosql.sql.planner.plan.ValuesNode; | ||
import io.prestosql.sql.tree.BooleanLiteral; | ||
import io.prestosql.sql.tree.DefaultExpressionTraversalVisitor; | ||
import io.prestosql.sql.tree.DereferenceExpression; | ||
import io.prestosql.sql.tree.ExistsPredicate; | ||
|
@@ -61,6 +60,7 @@ | |
import static io.prestosql.sql.analyzer.SemanticExceptions.notSupportedException; | ||
import static io.prestosql.sql.planner.ExpressionNodeInliner.replaceExpression; | ||
import static io.prestosql.sql.planner.optimizations.PlanNodeSearcher.searchFrom; | ||
import static io.prestosql.sql.tree.BooleanLiteral.TRUE_LITERAL; | ||
import static io.prestosql.sql.tree.ComparisonExpression.Operator.EQUAL; | ||
import static io.prestosql.sql.util.AstUtils.nodeContains; | ||
import static java.lang.String.format; | ||
|
@@ -227,10 +227,10 @@ private PlanBuilder appendScalarSubqueryApplyNode(PlanBuilder subPlan, SubqueryE | |
} | ||
|
||
// The subquery's EnforceSingleRowNode always produces a row, so the join is effectively INNER | ||
return appendLateralJoin(subPlan, subqueryPlan, scalarSubquery.getQuery(), correlationAllowed, LateralJoinNode.Type.INNER); | ||
return appendLateralJoin(subPlan, subqueryPlan, scalarSubquery.getQuery(), correlationAllowed, LateralJoinNode.Type.INNER, TRUE_LITERAL); | ||
} | ||
|
||
public PlanBuilder appendLateralJoin(PlanBuilder subPlan, PlanBuilder subqueryPlan, Query query, boolean correlationAllowed, LateralJoinNode.Type type) | ||
public PlanBuilder appendLateralJoin(PlanBuilder subPlan, PlanBuilder subqueryPlan, Query query, boolean correlationAllowed, LateralJoinNode.Type type, Expression filterCondition) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Joins don't have "filter" conditions. Rather, it's a "join" condition or criteria. The distinction is subtle but important. A filter determines whether a row is preserved or removed, but the join condition determines whether the rows are considered as candidates to be joined. Depending on the type of join, rows that don't satisfy the join condition may be dropped or joined with a synthetic row containing nulls (e.g., for outer joins). So I'd rename this argument to just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm aware of the naming bias. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Ok, let's leave it for now. We can clean this up in the future for both |
||
{ | ||
PlanNode subqueryNode = subqueryPlan.getRoot(); | ||
Map<Expression, Expression> correlation = extractCorrelation(subPlan, subqueryNode); | ||
|
@@ -247,6 +247,7 @@ public PlanBuilder appendLateralJoin(PlanBuilder subPlan, PlanBuilder subqueryPl | |
subqueryNode, | ||
ImmutableList.copyOf(SymbolsExtractor.extractUnique(correlation.values())), | ||
type, | ||
filterCondition, | ||
query), | ||
analysis.getParameters()); | ||
} | ||
|
@@ -279,7 +280,7 @@ private PlanBuilder appendExistSubqueryApplyNode(PlanBuilder subPlan, ExistsPred | |
|
||
PlanNode subqueryPlanRoot = subqueryPlan.getRoot(); | ||
if (isAggregationWithEmptyGroupBy(subqueryPlanRoot)) { | ||
subPlan.getTranslations().put(existsPredicate, BooleanLiteral.TRUE_LITERAL); | ||
subPlan.getTranslations().put(existsPredicate, TRUE_LITERAL); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. separete commit |
||
return subPlan; | ||
} | ||
|
||
|
@@ -288,7 +289,7 @@ private PlanBuilder appendExistSubqueryApplyNode(PlanBuilder subPlan, ExistsPred | |
|
||
Symbol exists = symbolAllocator.newSymbol("exists", BOOLEAN); | ||
subPlan.getTranslations().put(existsPredicate, exists); | ||
ExistsPredicate rewrittenExistsPredicate = new ExistsPredicate(BooleanLiteral.TRUE_LITERAL); | ||
ExistsPredicate rewrittenExistsPredicate = new ExistsPredicate(TRUE_LITERAL); | ||
return appendApplyNode( | ||
subPlan, | ||
existsPredicate.getSubquery(), | ||
|
@@ -499,7 +500,7 @@ private PlanBuilder createPlanBuilder(Node node) | |
private Set<Expression> extractOuterColumnReferences(PlanNode planNode) | ||
{ | ||
// at this point all the column references are already rewritten to SymbolReference | ||
// when reference expression is not rewritten that means it cannot be satisfied within given PlaNode | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this fix to a separate commit, since it's unrelated to this change. |
||
// when reference expression is not rewritten that means it cannot be satisfied within given PlanNode | ||
// see that TranslationMap only resolves (local) fields in current scope | ||
return ExpressionExtractor.extractExpressions(planNode).stream() | ||
.flatMap(expression -> extractColumnReferences(expression, analysis.getColumnReferences()).stream()) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it could be hard, but can divide this commit to first introduce the expression into
LateralJoinNode
and then we could have a commit which would addRIGHT
andFULL
toLateralJoinType
.