Skip to content

Commit

Permalink
planner: costs of Selection operators from DataSources are not accumu…
Browse files Browse the repository at this point in the history
…lated (pingcap#36244)

close pingcap#36243
  • Loading branch information
qw4990 authored Jul 15, 2022
1 parent ea306fc commit a33d971
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 0 deletions.
5 changes: 5 additions & 0 deletions planner/core/physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,11 @@ type PhysicalSelection struct {
basePhysicalPlan

Conditions []expression.Expression

// The flag indicates whether this Selection is from a DataSource.
// The flag is only used by cost model for compatibility and will be removed later.
// Please see https://github.com/pingcap/tidb/issues/36243 for more details.
fromDataSource bool
}

// Clone implements PhysicalPlan interface.
Expand Down
3 changes: 3 additions & 0 deletions planner/core/plan_cost.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ func (p *PhysicalSelection) GetPlanCost(taskType property.TaskType, costFlag uin
return 0, errors.Errorf("unknown task type %v", taskType)
}
selfCost = getCardinality(p.children[0], costFlag) * cpuFactor
if p.fromDataSource {
selfCost = 0 // for compatibility, see https://github.com/pingcap/tidb/issues/36243
}
case modelVer2: // selection cost: rows * num-filters * cpu-factor
var cpuFactor float64
switch taskType {
Expand Down
37 changes: 37 additions & 0 deletions planner/core/plan_cost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package core_test

import (
"fmt"
"strconv"
"strings"
"testing"

Expand Down Expand Up @@ -967,3 +968,39 @@ func TestTrueCardCost(t *testing.T) {
checkPlanCost(`select * from t where a>10 limit 10`)
checkPlanCost(`select sum(a), b*2 from t use index(b) group by b order by sum(a) limit 10`)
}

func TestIssue36243(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)

tk.MustExec("use test")
tk.MustExec(`create table t (a int)`)
tk.MustExec(`insert into mysql.expr_pushdown_blacklist values ('>','tikv','')`)
tk.MustExec(`admin reload expr_pushdown_blacklist`)
tk.MustExec(`set @@tidb_enable_new_cost_interface=1`)

getCost := func() (selCost, readerCost float64) {
res := tk.MustQuery(`explain format=verbose select * from t where a>0`).Rows()
// TableScan -> TableReader -> Selection
require.Equal(t, len(res), 3)
require.Contains(t, res[0][0], "Selection")
require.Contains(t, res[1][0], "TableReader")
require.Contains(t, res[2][0], "Scan")
var err error
selCost, err = strconv.ParseFloat(res[0][2].(string), 64)
require.NoError(t, err)
readerCost, err = strconv.ParseFloat(res[1][2].(string), 64)
require.NoError(t, err)
return
}

tk.MustExec(`set @@tidb_cost_model_version=1`)
// Selection has the same cost with TableReader, ignore Selection cost for compatibility in cost model ver1.
selCost, readerCost := getCost()
require.Equal(t, selCost, readerCost)

tk.MustExec(`set @@tidb_cost_model_version=2`)
selCost, readerCost = getCost()
require.True(t, selCost > readerCost)
}
1 change: 1 addition & 0 deletions planner/core/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,7 @@ func (t *copTask) handleRootTaskConds(ctx sessionctx.Context, newTask *rootTask)
selectivity = SelectionFactor
}
sel := PhysicalSelection{Conditions: t.rootTaskConds}.Init(ctx, newTask.p.statsInfo().Scale(selectivity), newTask.p.SelectBlockOffset())
sel.fromDataSource = true
sel.SetChildren(newTask.p)
newTask.p = sel
sel.cost = newTask.cost()
Expand Down

0 comments on commit a33d971

Please sign in to comment.