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

plan: make query on partition table not cacheable #16375

Merged
merged 7 commits into from
Apr 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion executor/prepared.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (e *PrepareExec) Next(ctx context.Context, req *chunk.Chunk) error {
SchemaVersion: e.is.SchemaMetaVersion(),
}

prepared.UseCache = plannercore.PreparedPlanCacheEnabled() && plannercore.Cacheable(stmt)
prepared.UseCache = plannercore.PreparedPlanCacheEnabled() && plannercore.Cacheable(stmt, e.is)

// We try to build the real statement of preparedStmt.
for i := range prepared.Params {
Expand Down
24 changes: 23 additions & 1 deletion planner/core/cacheable_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@ package core
import (
"github.com/pingcap/parser/ast"
"github.com/pingcap/tidb/expression"
"github.com/pingcap/tidb/infoschema"
"github.com/pingcap/tidb/types/parser_driver"
"github.com/pingcap/tidb/util/logutil"
"go.uber.org/zap"
)

// Cacheable checks whether the input ast is cacheable.
// Handle "ignore_plan_cache()" hint
// If there are multiple hints, only one will take effect
func Cacheable(node ast.Node) bool {
func Cacheable(node ast.Node, is infoschema.InfoSchema) bool {
switch node.(type) {
case *ast.SelectStmt:
for _, hints := range (node.(*ast.SelectStmt)).TableHints {
Expand All @@ -48,6 +51,7 @@ func Cacheable(node ast.Node) bool {
}
checker := cacheableChecker{
cacheable: true,
schema: is,
}
node.Accept(&checker)
return checker.cacheable
Expand All @@ -60,6 +64,7 @@ func Cacheable(node ast.Node) bool {
// NOTE: we can add more rules in the future.
type cacheableChecker struct {
cacheable bool
schema infoschema.InfoSchema
}

// Enter implements Visitor interface.
Expand Down Expand Up @@ -105,10 +110,27 @@ func (checker *cacheableChecker) Enter(in ast.Node) (out ast.Node, skipChildren
checker.cacheable = false
return in, true
}
case *ast.TableName:
if checker.isPartitionTable(node) {
checker.cacheable = false
return in, true
}
}
return in, false
}

func (checker *cacheableChecker) isPartitionTable(tn *ast.TableName) bool {
tb, err := checker.schema.TableByName(tn.Schema, tn.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change may have some performance impact.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why would this PR incur significant performance regression? it just adds one more check for TableName.

Because of the check here. @eurekaka

Copy link
Contributor

@eurekaka eurekaka Apr 22, 2020

Choose a reason for hiding this comment

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

Are those 2 map lookups so expensive?

Copy link
Contributor

Choose a reason for hiding this comment

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

schema.TableByName() is O(N) operation.
And the cacheableChecker itself need to handle each tableName in the AST.

if err != nil {
logutil.BgLogger().Error("Error occur in checking cacheable", zap.Error(err))
return false
}
if tb.Meta().Partition != nil {
return true
}
return false
}

// Leave implements Visitor interface.
func (checker *cacheableChecker) Leave(in ast.Node) (out ast.Node, ok bool) {
return in, checker.cacheable
Expand Down
110 changes: 74 additions & 36 deletions planner/core/cacheable_checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package core
package core_test

import (
. "github.com/pingcap/check"
"github.com/pingcap/parser/ast"
"github.com/pingcap/parser/model"
"github.com/pingcap/tidb/expression"
"github.com/pingcap/tidb/infoschema"
"github.com/pingcap/tidb/planner/core"
"github.com/pingcap/tidb/types/parser_driver"
"github.com/pingcap/tidb/util/testkit"
)

var _ = Suite(&testCacheableSuite{})
Expand All @@ -27,42 +30,55 @@ type testCacheableSuite struct {
}

func (s *testCacheableSuite) TestCacheable(c *C) {
store, dom, err := newStoreWithBootstrap()
c.Assert(err, IsNil)
tk := testkit.NewTestKit(c, store)
defer func() {
dom.Close()
store.Close()
}()
tk.MustExec("use test")
tk.MustExec("create table t1(a int, b int) partition by range(a) ( partition p0 values less than (6), partition p1 values less than (11) )")
tk.MustExec("create table t2(a int, b int) partition by hash(a) partitions 11")
tk.MustExec("create table t3(a int, b int)")
tbl := &ast.TableName{Schema: model.NewCIStr("test"), Name: model.NewCIStr("t3")}
is := infoschema.GetInfoSchema(tk.Se)
// test non-SelectStmt/-InsertStmt/-DeleteStmt/-UpdateStmt/-SelectStmt
var stmt ast.Node = &ast.UnionStmt{}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

stmt = &ast.ShowStmt{}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

stmt = &ast.LoadDataStmt{}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

tableRefsClause := &ast.TableRefsClause{TableRefs: &ast.Join{Left: &ast.TableSource{Source: &ast.TableName{}}}}
tableRefsClause := &ast.TableRefsClause{TableRefs: &ast.Join{Left: &ast.TableSource{Source: tbl}}}
// test InsertStmt
stmt = &ast.InsertStmt{Table: tableRefsClause}
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

// test DeleteStmt
whereExpr := &ast.FuncCallExpr{}
stmt = &ast.DeleteStmt{
TableRefs: tableRefsClause,
Where: whereExpr,
}
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

for funcName := range expression.UnCacheableFunctions {
whereExpr.FnName = model.NewCIStr(funcName)
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)
}

whereExpr.FnName = model.NewCIStr(ast.Rand)
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

stmt = &ast.DeleteStmt{
TableRefs: tableRefsClause,
Where: &ast.ExistsSubqueryExpr{},
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt := &ast.Limit{
Count: &driver.ParamMarkerExpr{},
Expand All @@ -71,7 +87,7 @@ func (s *testCacheableSuite) TestCacheable(c *C) {
TableRefs: tableRefsClause,
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt = &ast.Limit{
Offset: &driver.ParamMarkerExpr{},
Expand All @@ -80,41 +96,41 @@ func (s *testCacheableSuite) TestCacheable(c *C) {
TableRefs: tableRefsClause,
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt = &ast.Limit{}
stmt = &ast.DeleteStmt{
TableRefs: tableRefsClause,
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

stmt.(*ast.DeleteStmt).TableHints = append(stmt.(*ast.DeleteStmt).TableHints, &ast.TableOptimizerHint{
HintName: model.NewCIStr(HintIgnorePlanCache),
HintName: model.NewCIStr(core.HintIgnorePlanCache),
})
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

// test UpdateStmt
whereExpr = &ast.FuncCallExpr{}
stmt = &ast.UpdateStmt{
TableRefs: tableRefsClause,
Where: whereExpr,
}
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

for funcName := range expression.UnCacheableFunctions {
whereExpr.FnName = model.NewCIStr(funcName)
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)
}

whereExpr.FnName = model.NewCIStr(ast.Rand)
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

stmt = &ast.UpdateStmt{
TableRefs: tableRefsClause,
Where: &ast.ExistsSubqueryExpr{},
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt = &ast.Limit{
Count: &driver.ParamMarkerExpr{},
Expand All @@ -123,7 +139,7 @@ func (s *testCacheableSuite) TestCacheable(c *C) {
TableRefs: tableRefsClause,
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt = &ast.Limit{
Offset: &driver.ParamMarkerExpr{},
Expand All @@ -132,81 +148,103 @@ func (s *testCacheableSuite) TestCacheable(c *C) {
TableRefs: tableRefsClause,
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt = &ast.Limit{}
stmt = &ast.UpdateStmt{
TableRefs: tableRefsClause,
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

stmt.(*ast.UpdateStmt).TableHints = append(stmt.(*ast.UpdateStmt).TableHints, &ast.TableOptimizerHint{
HintName: model.NewCIStr(HintIgnorePlanCache),
HintName: model.NewCIStr(core.HintIgnorePlanCache),
})
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

// test SelectStmt
whereExpr = &ast.FuncCallExpr{}
stmt = &ast.SelectStmt{
Where: whereExpr,
}
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

for funcName := range expression.UnCacheableFunctions {
whereExpr.FnName = model.NewCIStr(funcName)
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)
}

whereExpr.FnName = model.NewCIStr(ast.Rand)
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

stmt = &ast.SelectStmt{
Where: &ast.ExistsSubqueryExpr{},
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt = &ast.Limit{
Count: &driver.ParamMarkerExpr{},
}
stmt = &ast.SelectStmt{
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt = &ast.Limit{
Offset: &driver.ParamMarkerExpr{},
}
stmt = &ast.SelectStmt{
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

limitStmt = &ast.Limit{}
stmt = &ast.SelectStmt{
Limit: limitStmt,
}
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

paramExpr := &driver.ParamMarkerExpr{}
orderByClause := &ast.OrderByClause{Items: []*ast.ByItem{{Expr: paramExpr}}}
stmt = &ast.SelectStmt{
OrderBy: orderByClause,
}
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

valExpr := &driver.ValueExpr{}
orderByClause = &ast.OrderByClause{Items: []*ast.ByItem{{Expr: valExpr}}}
stmt = &ast.SelectStmt{
OrderBy: orderByClause,
}
c.Assert(Cacheable(stmt), IsTrue)
c.Assert(core.Cacheable(stmt, is), IsTrue)

stmt.(*ast.SelectStmt).TableHints = append(stmt.(*ast.SelectStmt).TableHints, &ast.TableOptimizerHint{
HintName: model.NewCIStr(HintIgnorePlanCache),
HintName: model.NewCIStr(core.HintIgnorePlanCache),
})
c.Assert(Cacheable(stmt), IsFalse)
c.Assert(core.Cacheable(stmt, is), IsFalse)

boundExpr := &ast.FrameBound{Expr: &driver.ParamMarkerExpr{}}
c.Assert(Cacheable(boundExpr), IsFalse)
c.Assert(core.Cacheable(boundExpr, is), IsFalse)

// Partition table can not be cached.
join := &ast.Join{
Left: &ast.TableName{Schema: model.NewCIStr("test"), Name: model.NewCIStr("t1")},
Right: &ast.TableName{Schema: model.NewCIStr("test"), Name: model.NewCIStr("t2")},
}
stmt = &ast.SelectStmt{
From: &ast.TableRefsClause{
TableRefs: join,
},
}
c.Assert(core.Cacheable(stmt, is), IsFalse)

join = &ast.Join{
Left: &ast.TableName{Schema: model.NewCIStr("test"), Name: model.NewCIStr("t3")},
}
stmt = &ast.SelectStmt{
From: &ast.TableRefsClause{
TableRefs: join,
},
}
c.Assert(core.Cacheable(stmt, is), IsTrue)
}
37 changes: 5 additions & 32 deletions planner/core/common_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,12 @@ func (e *Execute) getPhysicalPlan(ctx context.Context, sctx sessionctx.Context,
}
e.names = names
e.Plan = p
isRange := e.isRangePartition(p)
_, isTableDual := p.(*PhysicalTableDual)
if !isTableDual && prepared.UseCache && !isRange {
if !isTableDual && prepared.UseCache {
err = e.setFoundInPlanCache(sctx, true)
if err != nil {
return err
}
cached := NewPSTMTPlanCacheValue(p, names, stmtCtx.TblInfo2UnionScan)
preparedStmt.NormalizedPlan, preparedStmt.PlanDigest = NormalizePlan(p)
stmtCtx.SetPlanDigest(preparedStmt.NormalizedPlan, preparedStmt.PlanDigest)
Expand Down Expand Up @@ -526,36 +529,6 @@ func (e *Execute) rebuildRange(p Plan) error {
return nil
}

func checkRangePartitionInfo(pi *model.PartitionInfo) bool {
if pi != nil && pi.Type == model.PartitionTypeRange {
return true
}
return false
}

// Prepare plan cache is not support query plan on range partition table.
func (e *Execute) isRangePartition(p Plan) bool {
isRange := false
switch x := p.(type) {
case *PhysicalTableReader:
ts := x.TablePlans[0].(*PhysicalTableScan)
return checkRangePartitionInfo(ts.Table.Partition)
case *PhysicalIndexLookUpReader:
is := x.IndexPlans[0].(*PhysicalIndexScan)
return checkRangePartitionInfo(is.Table.Partition)
case *PhysicalIndexReader:
is := x.IndexPlans[0].(*PhysicalIndexScan)
return checkRangePartitionInfo(is.Table.Partition)
case PhysicalPlan:
for _, child := range x.Children() {
if e.isRangePartition(child) {
isRange = true
}
}
}
return isRange
}

func (e *Execute) buildRangeForIndexScan(sctx sessionctx.Context, is *PhysicalIndexScan) ([]*ranger.Range, error) {
if len(is.IdxCols) == 0 {
return ranger.FullRange(), nil
Expand Down