-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Determine automatically if push join to table scan #6818
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
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 |
|---|---|---|
|
|
@@ -135,7 +135,7 @@ public class FeaturesConfig | |
| private DataSize filterAndProjectMinOutputPageSize = DataSize.of(500, KILOBYTE); | ||
| private int filterAndProjectMinOutputPageRowCount = 256; | ||
| private int maxGroupingSets = 2048; | ||
| private JoinPushdownMode joinPushdownMode = JoinPushdownMode.DISABLED; | ||
| private JoinPushdownMode joinPushdownMode = JoinPushdownMode.AUTOMATIC; | ||
|
Member
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. see conversation about code level documentation in the other pr
Member
Author
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. Added comment as a separate commit before introducing |
||
|
|
||
| public enum JoinReorderingStrategy | ||
| { | ||
|
|
@@ -179,9 +179,12 @@ public enum JoinPushdownMode | |
| * Try to push all joins except cross-joins to connector. | ||
| */ | ||
| EAGER, | ||
| // TODO Add cost based logic to join pushdown | ||
| // AUTOMATIC, | ||
| /**/; | ||
| /** | ||
| * Determine automatically if push join to connector based on table statistics. | ||
| * Do not perform join in absence of table statistics. | ||
| */ | ||
| AUTOMATIC, | ||
|
Member
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. it is safe to make it the default |
||
| /**/ | ||
| } | ||
|
|
||
| public double getCpuCostWeight() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| import com.google.common.collect.ImmutableMap; | ||
| import com.google.common.collect.ImmutableSet; | ||
| import io.trino.Session; | ||
| import io.trino.cost.PlanNodeStatsEstimate; | ||
| import io.trino.matching.Capture; | ||
| import io.trino.matching.Captures; | ||
| import io.trino.matching.Pattern; | ||
|
|
@@ -32,6 +33,7 @@ | |
| import io.trino.sql.ExpressionUtils; | ||
| import io.trino.sql.analyzer.FeaturesConfig.JoinPushdownMode; | ||
| import io.trino.sql.planner.Symbol; | ||
| import io.trino.sql.planner.TypeProvider; | ||
| import io.trino.sql.planner.iterative.Rule; | ||
| import io.trino.sql.planner.plan.JoinNode; | ||
| import io.trino.sql.planner.plan.Patterns; | ||
|
|
@@ -62,6 +64,7 @@ | |
| import static io.trino.sql.planner.plan.Patterns.Join.left; | ||
| import static io.trino.sql.planner.plan.Patterns.Join.right; | ||
| import static io.trino.sql.planner.plan.Patterns.tableScan; | ||
| import static java.lang.Double.isNaN; | ||
| import static java.util.Objects.requireNonNull; | ||
|
|
||
| public class PushJoinIntoTableScan | ||
|
|
@@ -114,6 +117,10 @@ public Result apply(JoinNode joinNode, Captures captures, Context context) | |
| return Result.empty(); | ||
| } | ||
|
|
||
| if (skipJoinPushdownBasedOnCost(joinNode, context)) { | ||
| return Result.empty(); | ||
| } | ||
|
|
||
| Map<String, ColumnHandle> leftAssignments = left.getAssignments().entrySet().stream() | ||
| .collect(toImmutableMap(entry -> entry.getKey().getName(), Map.Entry::getValue)); | ||
|
|
||
|
|
@@ -162,6 +169,43 @@ public Result apply(JoinNode joinNode, Captures captures, Context context) | |
| return Result.ofPlanNode(new TableScanNode(joinNode.getId(), handle, joinNode.getOutputSymbols(), newAssignments.build(), newEnforcedConstraint, false)); | ||
| } | ||
|
|
||
| private boolean skipJoinPushdownBasedOnCost(JoinNode joinNode, Context context) | ||
| { | ||
| if (getJoinPushdownMode(context.getSession()) != JoinPushdownMode.AUTOMATIC) { | ||
| return false; | ||
| } | ||
|
|
||
| TypeProvider types = context.getSymbolAllocator().getTypes(); | ||
|
|
||
| // returning as quickly as possible to avoid unnecessary, costly work | ||
|
|
||
| PlanNodeStatsEstimate leftStats = context.getStatsProvider().getStats(joinNode.getLeft()); | ||
| double leftOutputSize = leftStats.getOutputSizeInBytes(joinNode.getLeft().getOutputSymbols(), types); | ||
| if (isNaN(leftOutputSize)) { | ||
| return true; | ||
| } | ||
|
|
||
| PlanNodeStatsEstimate rightStats = context.getStatsProvider().getStats(joinNode.getRight()); | ||
| double rightOutputSize = rightStats.getOutputSizeInBytes(joinNode.getRight().getOutputSymbols(), types); | ||
| if (isNaN(rightOutputSize)) { | ||
| return true; | ||
| } | ||
|
|
||
| PlanNodeStatsEstimate joinStats = context.getStatsProvider().getStats(joinNode); | ||
| double joinOutputSize = joinStats.getOutputSizeInBytes(joinNode.getOutputSymbols(), types); | ||
| if (isNaN(joinOutputSize)) { | ||
| return true; | ||
| } | ||
|
|
||
| if (joinOutputSize > leftOutputSize + rightOutputSize) { | ||
|
Member
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. Consider adding some factor here, e.g pushed down join should produce 2x less rows than in trino. Such factor might need to be empirically established
Member
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. so you mean to replace
Member
Author
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. Yeah - I find initial value of a factor |
||
| // This is poor man's estimation if it makes more sense to perform join in source database or Trino. | ||
| // The assumption here is that cost of performing join in source database is less than or equal to cost of join in Trino. | ||
| // We resolve tie for pessimistic case (both join costs equal) on cost of sending the data from source database to Trino. | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| private TupleDomain<ColumnHandle> deriveConstraint(TupleDomain<ColumnHandle> sourceConstraint, Map<ColumnHandle, ColumnHandle> columnMapping, boolean nullable) | ||
| { | ||
| TupleDomain<ColumnHandle> constraint = sourceConstraint; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Even if number of rows after pushdown is smaller then without pushdown it could significantly increase cpu overhead of underlying source (table scans might be much cheaper than join). I think it would be great to determine what's the impact of pushdown on underlying connectors. It could be that join pushdown is beneficial only when joins are very non selective and users don't want cpu of underlying connector to increase significantly.
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.
Agreed. Yet I would assume that you will still be able to disable pushdown on per-connector level in configuration. As well as per-query using session.
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.
Totally -- #6874 provides both catalog level config and session toggle.