Skip to content
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

Gen4 fallback planning #7370

Merged
merged 5 commits into from
Jan 28, 2021
Merged

Gen4 fallback planning #7370

merged 5 commits into from
Jan 28, 2021

Conversation

systay
Copy link
Collaborator

@systay systay commented Jan 25, 2021

Tries using the gen4 planner, and if that fails, uses the v3 planner instead.

This way we can compose planners in more ways.

Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay mentioned this pull request Jan 25, 2021
13 tasks
Signed-off-by: Andres Taylor <andres@planetscale.com>
Comment on lines 116 to 121
case Gen4WithFallback:
fp := &fallbackPlanner{
primary: buildSelectPlan,
fallback: gen4Planner,
}
return fp.plan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

primary should be gen4, right?

Comment on lines 123 to 128
// fail - unknown planner
return func(query string) func(sqlparser.Statement, ContextVSchema) (engine.Primitive, error) {
return func(sqlparser.Statement, ContextVSchema) (engine.Primitive, error) {
return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unknown planner selected %d", vschema.Planner())
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: error can be added as return type to avoid creating this method.

Comment on lines 598 to 599
filenames := []string{"large_cases.txt"}
vschema := &vschemaWrapper{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this removed on purpose?

func createInstructionFor(query string, stmt sqlparser.Statement, vschema ContextVSchema) (engine.Primitive, error) {
type selectPlanner func(query string) func(sqlparser.Statement, ContextVSchema) (engine.Primitive, error)

func createInstructionFor(query string, stmt sqlparser.Statement, vschema ContextVSchema, sel selectPlanner) (engine.Primitive, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we do not need to pass selectPlanner here and getConfiguredPlanner(vschema) can be used directly.

Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay merged commit 002b800 into vitessio:master Jan 28, 2021
@systay systay deleted the gen4-fail branch January 28, 2021 19:45
@askdba askdba added this to the v10.0 milestone Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants