-
Notifications
You must be signed in to change notification settings - Fork 2.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
Planner refactoring #7103
Planner refactoring #7103
Conversation
137943e
to
f2afbe5
Compare
8c463b7
to
e45969c
Compare
85b6065
to
fc17b74
Compare
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: GuptaManan100 <[email protected]>
Signed-off-by: GuptaManan100 <[email protected]>
Test the new planner in plan_test side by side with the old planner Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: GuptaManan100 <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
fc17b74
to
09a6dac
Compare
019ea65
to
b5d0f0f
Compare
Signed-off-by: Andres Taylor <[email protected]>
b5d0f0f
to
9691541
Compare
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
8f861b0
to
44de903
Compare
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
e17943c
to
000426e
Compare
Signed-off-by: GuptaManan100 <[email protected]>
043f79d
to
1c9c5eb
Compare
Signed-off-by: GuptaManan100 <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
… as the v3 Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
… is a single route Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
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.
Overall looks good to me.
go/vt/sysvars/sysvars.go
Outdated
@@ -51,6 +51,7 @@ var ( | |||
SQLSelectLimit = SystemVariable{Name: "sql_select_limit", Default: off} | |||
TransactionMode = SystemVariable{Name: "transaction_mode", IdentifierAsString: true} | |||
Workload = SystemVariable{Name: "workload", IdentifierAsString: true} | |||
PlannerVersion = SystemVariable{Name: "planner_version", IdentifierAsString: true} |
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.
Do we need to allow users to change planner_version at session level? If we do this than the plan cache also would be required to store this information. Allowing at vtgate startup should be good enough.
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.
Fair enough. I'll remove it. I was testing things and wanted to quickly be able to switch, but there are other ways of accomplishing this.
go/vt/vtgate/semantics/analyzer.go
Outdated
|
||
scopes []*scope | ||
exprDeps map[sqlparser.Expr]TableSet | ||
si schemaInformation |
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.
looks like this is not used in the code.
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.
yeah, this is how we will read from the vschema to get tables we have column information about. I'll remove
go/vt/vtgate/semantics/analyzer.go
Outdated
} | ||
|
||
// resolveUnQualifiedColumn | ||
func (a *analyzer) resolveUnQualifiedColumn(current *scope, expr *sqlparser.ColName) (table, error) { |
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.
expr is not used
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.
same as above - once we can query about column info, this is where this would happen. I'll remove
} | ||
) | ||
|
||
// TableSetFor returns the bitmask for this particular tableshoe |
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.
nit: tableshoe
looks incorrect.
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.
Covfefe?
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.
intentional. I see
// Wireup2 does the wire up work for the new planner | ||
Wireup2(semTable *semantics.SemTable) error | ||
|
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.
nit: wireup2
-> wireupv4
if err := qg.collectTables(table.Exprs, semTable); err != nil { | ||
return err | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func (qg *queryGraph) collectTables(t sqlparser.TableExprs, semTable *semantics.SemTable) error { | ||
for _, expr := range t { | ||
if err := qg.collectTable(expr, semTable); err != nil { | ||
return err | ||
} | ||
} | ||
return nil | ||
} |
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.
nit: this could be done inline.
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.
collectTables
is also used from line 69 in this same file, so I just used it here as well. DRY, right?
for _, predicate := range splitAndExpression(nil, table.Condition.On) { | ||
err := qg.collectPredicate(predicate, semTable) | ||
if err != nil { | ||
return err | ||
} | ||
} |
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.
can use collectPredicates
method call here?
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.
no, not really. collectPredicates
extracts predicates from a SELECT struct, and that is not what we have here
go/vt/vtgate/planbuilder/route.go
Outdated
// solvedTables keeps track of which tables this route is covering | ||
solvedTables semantics.TableSet |
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.
nit: could be renamed to containedTables
if len(node.SelectExprs) == 0 { | ||
node.SelectExprs = []sqlparser.SelectExpr{ | ||
&sqlparser.AliasedExpr{ | ||
Expr: sqlparser.NewIntLiteral([]byte{'1'}), | ||
}, | ||
} | ||
} | ||
} |
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.
why is this needed?
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.
sometimes we don't need anything from a route except the number of matching rows. in those cases, we add a single literal because a SELECT with no expressions is not valid
for i, table := range qg.tables { | ||
solves := semTable.TableSetFor(table.alias) | ||
plan, err := createRoutePlan(table, solves, vschema) | ||
if err != nil { | ||
return nil, err | ||
} | ||
plans[i] = plan | ||
} | ||
|
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.
nit: similar to in lefttoright, can be moved to a method.
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
This is a larger rewrite of the vtgate planner. It introduces new passes and intermediate representations of the query.
The old code used these passes over the query:
This refactored planner now uses the following passes:
By splitting the planning process into smaller pieces, each part can be simplified and extended to do more.
Here follows a short description of each new pass.
Semantic Analysis
Responsibilities: Scoping, Binding
Walks the AST and does scoping and binding, so whenever a column name is found, the planner has information about which tables is being referenced. Tables are given a
TableSet
identifier - a bitmask struct that allows the planner to quickly find what dependencies every expression has.Extract Query Graph
Responsibilities: Extract Subqueries, Create Query Graph
The query graph is an intermediate representation that is designed to allow the route planner to quickly consider many different solutions for the query. Instead of keeping the query in the AST, which is limited by the tree structure it has, we produce a graphy representation with all used tables (nodes) in one list, and edges between them in a separate list.
In this pass, subqueries are extracted into a list of queries and the relationships between them. This makes it easier for later passes to plan fully without having to switch back and forth between passes - when doing route planning, we can do all of route planning in one go and don't have to wait for SELECT expressions to be considered before planning subqueries used in SELECT expressions.
Route planning
Responsibilities: Plan how to route the query - plan FROM and WHERE
This pass uses dynamic programming to consider all combinations of tables in order to find the optimal plan. Optimal here means minimal number of route primitives in the plan.
At the end of this stage, we have a tree structure that represents all the route primitives needed and how they should be joined.
Horizon planning
Responsibilities: Plan projections, aggregations, grouping and ordering
Once we have a plan for how to route queries, we plan what projections we need from each route, and how to do
ORDER BY/GROUP BY/LIMIT
et al.Positive outcomes from this refactoring.
Why do this non-trivial piece of work?
We still have a number of query types that are not supported. In order to be able to support more queries, we needed to extend the planner. Instead of adding to the legacy planner which is not very easy to work with, we felt that it was time to introduce this new design, which not only will allow us to support these queries, it also sets us up to be able to do more optimisations in the future.