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

planner: don't DoOptimze when build show #12005

Merged
merged 6 commits into from
Sep 9, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
don't DoOptimze when build show
  • Loading branch information
winoros committed Sep 3, 2019
commit f6122cf6928a993cce047b1de4f1cbc2a0e16f11
7 changes: 6 additions & 1 deletion executor/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,11 +1183,16 @@ func (b *executorBuilder) buildTableDual(v *plannercore.PhysicalTableDual) Execu
return nil
}
base := newBaseExecutor(b.ctx, v.Schema(), v.ExplainID())
base.initCap = v.RowCount
e := &TableDualExec{
baseExecutor: base,
numDualRows: v.RowCount,
}
if v.SourcePlan != nil {
sourceExec := b.build(v.SourcePlan)
e.sourceExec = sourceExec
} else {
base.initCap = v.RowCount
}
return e
}

Expand Down
6 changes: 6 additions & 0 deletions executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,9 @@ type TableDualExec struct {
// numDualRows can only be 0 or 1.
numDualRows int
numReturned int

// Temp solution to make the logic of processing show statement more reasonable.
sourceExec Executor
}

// Open implements the Executor Open interface.
Expand All @@ -963,6 +966,9 @@ func (e *TableDualExec) Open(ctx context.Context) error {

// Next implements the Executor Next interface.
func (e *TableDualExec) Next(ctx context.Context, req *chunk.Chunk) error {
if e.sourceExec != nil {
return e.sourceExec.Next(ctx, req)
}
req.Reset()
if e.numReturned >= e.numDualRows {
return nil
Expand Down
1 change: 1 addition & 0 deletions planner/core/common_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ type Deallocate struct {
}

// Show represents a show plan.
// TODO: make as a logical data source or table dual.
type Show struct {
physicalSchemaProducer

Expand Down
4 changes: 2 additions & 2 deletions planner/core/find_best_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ func (p *LogicalTableDual) findBestTask(prop *property.PhysicalProperty) (task,
return invalidTask, nil
}
dual := PhysicalTableDual{
RowCount: p.RowCount,
placeHolder: p.placeHolder,
RowCount: p.RowCount,
SourcePlan: p.sourcePlan,
}.Init(p.ctx, p.stats)
dual.SetSchema(p.schema)
return &rootTask{p: dual}, nil
Expand Down
6 changes: 2 additions & 4 deletions planner/core/logical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,8 @@ type LogicalTableDual struct {
logicalSchemaProducer

RowCount int
// placeHolder indicates if this dual plan is a place holder in query optimization
// for data sources like `Show`, if true, the dual plan would be substituted by
// `Show` in the final plan.
placeHolder bool

sourcePlan Plan
}

// LogicalUnionScan is only used in non read-only txn.
Expand Down
6 changes: 2 additions & 4 deletions planner/core/physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,8 @@ type PhysicalTableDual struct {
physicalSchemaProducer

RowCount int
// placeHolder indicates if this dual plan is a place holder in query optimization
// for data sources like `Show`, if true, the dual plan would be substituted by
// `Show` in the final plan.
placeHolder bool

SourcePlan Plan

// names is used for OutputNames() method. Dual may be inited when building point get plan.
// So it needs to hold names for itself.
Expand Down
19 changes: 2 additions & 17 deletions planner/core/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ func (b *PlanBuilder) buildShow(ctx context.Context, show *ast.ShowStmt) (Plan,
for _, col := range p.schema.Columns {
col.UniqueID = b.ctx.GetSessionVars().AllocPlanColumnID()
}
mockTablePlan := LogicalTableDual{placeHolder: true}.Init(b.ctx)
mockTablePlan := LogicalTableDual{sourcePlan: p}.Init(b.ctx)
mockTablePlan.SetSchema(p.schema)
var err error
var np LogicalPlan
Expand Down Expand Up @@ -1406,26 +1406,11 @@ func (b *PlanBuilder) buildShow(ctx context.Context, show *ast.ShowStmt) (Plan,
}
proj.SetSchema(schema)
proj.SetChildren(np)
physical, err := DoOptimize(ctx, b.optFlag|flagEliminateProjection, proj)
if err != nil {
return nil, err
}
return substitutePlaceHolderDual(physical, p), nil
return proj, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to eliminate projection here?

Copy link
Member Author

@winoros winoros Sep 5, 2019

Choose a reason for hiding this comment

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

Prvious one is not a valid project elimination.
For now, show cannot remove any of its output column.
So the project should be remained.

Also note that, now the output is logical plan. It will pass all optimizations. So the project cannot be removed now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Prvious one is not a valid project elimination.
For now, show cannot remove any of its output column.
So the project should be remained.

Sorry I didn't understand this, please explain a little bit more. Also, shall we set flagEliminateProjection of b.optFlag here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, sorry i misread the comment.
We can add flag here.
But the project in the that test won't be removed

}
return p, nil
}

func substitutePlaceHolderDual(src PhysicalPlan, dst PhysicalPlan) PhysicalPlan {
if dual, ok := src.(*PhysicalTableDual); ok && dual.placeHolder {
return dst
}
for i, child := range src.Children() {
newChild := substitutePlaceHolderDual(child, dst)
src.SetChild(i, newChild)
}
return src
}

func (b *PlanBuilder) buildSimple(node ast.StmtNode) (Plan, error) {
p := &Simple{Statement: node}

Expand Down