Skip to content

Commit

Permalink
planner: fix the issue where TiDB generates multiple plandigests for …
Browse files Browse the repository at this point in the history
…'IN (...)'. (#47216)

close #33559
  • Loading branch information
King-Dylan authored Nov 1, 2023
1 parent d380ef4 commit 89945f5
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 6 deletions.
55 changes: 50 additions & 5 deletions pkg/expression/explain.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,31 @@ func (expr *ScalarFunction) ExplainNormalizedInfo() string {
return expr.explainInfo(true)
}

// ExplainNormalizedInfo4InList implements the Expression interface.
func (expr *ScalarFunction) ExplainNormalizedInfo4InList() string {
var buffer bytes.Buffer
fmt.Fprintf(&buffer, "%s(", expr.FuncName.L)
switch expr.FuncName.L {
case ast.Cast:
for _, arg := range expr.GetArgs() {
buffer.WriteString(arg.ExplainNormalizedInfo4InList())
buffer.WriteString(", ")
buffer.WriteString(expr.RetType.String())
}
case ast.In:
buffer.WriteString("...")
default:
for i, arg := range expr.GetArgs() {
buffer.WriteString(arg.ExplainNormalizedInfo4InList())
if i+1 < len(expr.GetArgs()) {
buffer.WriteString(", ")
}
}
}
buffer.WriteString(")")
return buffer.String()
}

// ExplainInfo implements the Expression interface.
func (col *Column) ExplainInfo() string {
return col.String()
Expand All @@ -78,6 +103,14 @@ func (col *Column) ExplainNormalizedInfo() string {
return "?"
}

// ExplainNormalizedInfo4InList implements the Expression interface.
func (col *Column) ExplainNormalizedInfo4InList() string {
if col.OrigName != "" {
return col.OrigName
}
return "?"
}

// ExplainInfo implements the Expression interface.
func (expr *Constant) ExplainInfo() string {
dt, err := expr.Eval(chunk.Row{})
Expand All @@ -92,6 +125,11 @@ func (expr *Constant) ExplainNormalizedInfo() string {
return "?"
}

// ExplainNormalizedInfo4InList implements the Expression interface.
func (expr *Constant) ExplainNormalizedInfo4InList() string {
return "?"
}

func (expr *Constant) format(dt types.Datum) string {
switch dt.Kind() {
case types.KindNull:
Expand Down Expand Up @@ -142,14 +180,21 @@ func ExplainExpressionList(exprs []Expression, schema *Schema) string {
// In some scenarios, the expr's order may not be stable when executing multiple times.
// So we add a sort to make its explain result stable.
func SortedExplainExpressionList(exprs []Expression) []byte {
return sortedExplainExpressionList(exprs, false)
return sortedExplainExpressionList(exprs, false, false)
}

// SortedExplainExpressionListIgnoreInlist generates explain information for a list of expressions in order.
func SortedExplainExpressionListIgnoreInlist(exprs []Expression) []byte {
return sortedExplainExpressionList(exprs, false, true)
}

func sortedExplainExpressionList(exprs []Expression, normalized bool) []byte {
func sortedExplainExpressionList(exprs []Expression, normalized bool, ignoreInlist bool) []byte {
buffer := bytes.NewBufferString("")
exprInfos := make([]string, 0, len(exprs))
for _, expr := range exprs {
if normalized {
if ignoreInlist {
exprInfos = append(exprInfos, expr.ExplainNormalizedInfo4InList())
} else if normalized {
exprInfos = append(exprInfos, expr.ExplainNormalizedInfo())
} else {
exprInfos = append(exprInfos, expr.ExplainInfo())
Expand All @@ -167,7 +212,7 @@ func sortedExplainExpressionList(exprs []Expression, normalized bool) []byte {

// SortedExplainNormalizedExpressionList is same like SortedExplainExpressionList, but use for generating normalized information.
func SortedExplainNormalizedExpressionList(exprs []Expression) []byte {
return sortedExplainExpressionList(exprs, true)
return sortedExplainExpressionList(exprs, true, false)
}

// SortedExplainNormalizedScalarFuncList is same like SortedExplainExpressionList, but use for generating normalized information.
Expand All @@ -176,7 +221,7 @@ func SortedExplainNormalizedScalarFuncList(exprs []*ScalarFunction) []byte {
for i := range exprs {
expressions[i] = exprs[i]
}
return sortedExplainExpressionList(expressions, true)
return sortedExplainExpressionList(expressions, true, false)
}

// ExplainColumnList generates explain information for a list of columns.
Expand Down
3 changes: 3 additions & 0 deletions pkg/expression/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ type Expression interface {
// ExplainNormalizedInfo returns operator normalized information for generating digest.
ExplainNormalizedInfo() string

// ExplainNormalizedInfo4InList returns operator normalized information for plan digest.
ExplainNormalizedInfo4InList() string

// HashCode creates the hashcode for expression which can be used to identify itself from other expression.
// It generated as the following:
// Constant: ConstantFlag+encoded value
Expand Down
1 change: 1 addition & 0 deletions pkg/expression/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ func (m *MockExpr) resolveIndicesByVirtualExpr(schema *Schema) bool
func (m *MockExpr) RemapColumn(_ map[int64]*Column) (Expression, error) { return m, nil }
func (m *MockExpr) ExplainInfo() string { return "" }
func (m *MockExpr) ExplainNormalizedInfo() string { return "" }
func (m *MockExpr) ExplainNormalizedInfo4InList() string { return "" }
func (m *MockExpr) HashCode(sc *stmtctx.StatementContext) []byte { return nil }
func (m *MockExpr) Vectorized() bool { return false }
func (m *MockExpr) SupportReverseEval() bool { return false }
Expand Down
2 changes: 1 addition & 1 deletion pkg/planner/core/casetest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_test(
],
data = glob(["testdata/**"]),
flaky = True,
shard_count = 20,
shard_count = 21,
deps = [
"//pkg/domain",
"//pkg/parser",
Expand Down
37 changes: 37 additions & 0 deletions pkg/planner/core/casetest/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,43 @@ func TestNormalizedPlan(t *testing.T) {
}
}

func TestPlanDigest4InList(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t")
tk.MustExec("create table t (a int);")
tk.MustExec("set global tidb_ignore_inlist_plan_digest=true;")
tk.Session().GetSessionVars().PlanID.Store(0)
queriesGroup1 := []string{
"select * from t where a in (1, 2);",
"select a in (1, 2) from t;",
}
queriesGroup2 := []string{
"select * from t where a in (1, 2, 3);",
"select a in (1, 2, 3) from t;",
}
for i := 0; i < len(queriesGroup1); i++ {
query1 := queriesGroup1[i]
query2 := queriesGroup2[i]
t.Run(query1+" vs "+query2, func(t *testing.T) {
tk.MustExec(query1)
info1 := tk.Session().ShowProcess()
require.NotNil(t, info1)
p1, ok := info1.Plan.(core.Plan)
require.True(t, ok)
_, digest1 := core.NormalizePlan(p1)
tk.MustExec(query2)
info2 := tk.Session().ShowProcess()
require.NotNil(t, info2)
p2, ok := info2.Plan.(core.Plan)
require.True(t, ok)
_, digest2 := core.NormalizePlan(p2)
require.Equal(t, digest1, digest2)
})
}
}

func TestIssue47634(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
Expand Down
7 changes: 7 additions & 0 deletions pkg/planner/core/explain.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/pingcap/tidb/pkg/parser/mysql"
"github.com/pingcap/tidb/pkg/planner/property"
"github.com/pingcap/tidb/pkg/planner/util"
"github.com/pingcap/tidb/pkg/sessionctx/variable"
"github.com/pingcap/tidb/pkg/statistics"
"github.com/pingcap/tidb/pkg/types"
"github.com/pingcap/tidb/pkg/util/logutil"
Expand Down Expand Up @@ -365,6 +366,9 @@ func (p *PhysicalSelection) ExplainInfo() string {

// ExplainNormalizedInfo implements Plan interface.
func (p *PhysicalSelection) ExplainNormalizedInfo() string {
if variable.IgnoreInlistPlanDigest.Load() {
return string(expression.SortedExplainExpressionListIgnoreInlist(p.Conditions))
}
return string(expression.SortedExplainNormalizedExpressionList(p.Conditions))
}

Expand Down Expand Up @@ -403,6 +407,9 @@ func (p *PhysicalExpand) explainInfoV2() string {

// ExplainNormalizedInfo implements Plan interface.
func (p *PhysicalProjection) ExplainNormalizedInfo() string {
if variable.IgnoreInlistPlanDigest.Load() {
return string(expression.SortedExplainExpressionListIgnoreInlist(p.Exprs))
}
return string(expression.SortedExplainNormalizedExpressionList(p.Exprs))
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -2403,6 +2403,12 @@ var defaultSysVars = []*SysVar{
s.EnableReuseCheck = TiDBOptOn(val)
return nil
}},
{Scope: ScopeGlobal, Name: TiDBIgnoreInlistPlanDigest, Value: BoolToOnOff(DefTiDBIgnoreInlistPlanDigest), Type: TypeBool, SetGlobal: func(ctx context.Context, vars *SessionVars, s string) error {
IgnoreInlistPlanDigest.Store(TiDBOptOn(s))
return nil
}, GetGlobal: func(ctx context.Context, vars *SessionVars) (string, error) {
return BoolToOnOff(IgnoreInlistPlanDigest.Load()), nil
}},
{Scope: ScopeGlobal, Name: TiDBTTLJobEnable, Value: BoolToOnOff(DefTiDBTTLJobEnable), Type: TypeBool, SetGlobal: func(ctx context.Context, vars *SessionVars, s string) error {
EnableTTLJob.Store(TiDBOptOn(s))
return nil
Expand Down
16 changes: 16 additions & 0 deletions pkg/sessionctx/variable/sysvar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,22 @@ func TestSetJobScheduleWindow(t *testing.T) {
require.Equal(t, "16:11 +0800", val)
}

func TestTiDBIgnoreInlistPlanDigest(t *testing.T) {
vars := NewSessionVars(nil)
mock := NewMockGlobalAccessor4Tests()
mock.SessionVars = vars
vars.GlobalVarsAccessor = mock
initValue, err := mock.GetGlobalSysVar(TiDBIgnoreInlistPlanDigest)
require.NoError(t, err)
require.Equal(t, initValue, Off)
// Set to On(init at start)
err1 := mock.SetGlobalSysVar(context.Background(), TiDBIgnoreInlistPlanDigest, On)
require.NoError(t, err1)
NewVal, err2 := mock.GetGlobalSysVar(TiDBIgnoreInlistPlanDigest)
require.NoError(t, err2)
require.Equal(t, NewVal, On)
}

func TestTiDBEnableResourceControl(t *testing.T) {
// setup the hooks for test
// NOTE: the default system variable is true but the switch is false
Expand Down
5 changes: 5 additions & 0 deletions pkg/sessionctx/variable/tidb_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,9 @@ const (
// TiDBStmtSummaryMaxSQLLength indicates the max length of displayed normalized sql and sample sql.
TiDBStmtSummaryMaxSQLLength = "tidb_stmt_summary_max_sql_length"

// TiDBIgnoreInlistPlanDigest enables TiDB to generate the same plan digest with SQL using different in-list arguments.
TiDBIgnoreInlistPlanDigest = "tidb_ignore_inlist_plan_digest"

// TiDBCapturePlanBaseline indicates whether the capture of plan baselines is enabled.
TiDBCapturePlanBaseline = "tidb_capture_plan_baselines"

Expand Down Expand Up @@ -1282,6 +1285,7 @@ const (
DefTiDBStmtSummaryMaxStmtCount = 3000
DefTiDBStmtSummaryMaxSQLLength = 4096
DefTiDBCapturePlanBaseline = Off
DefTiDBIgnoreInlistPlanDigest = false
DefTiDBEnableIndexMerge = true
DefEnableLegacyInstanceScope = true
DefTiDBTableCacheLease = 3 // 3s
Expand Down Expand Up @@ -1523,6 +1527,7 @@ var (
ServiceScope = atomic.NewString("")
SchemaVersionCacheLimit = atomic.NewInt64(DefTiDBSchemaVersionCacheLimit)
CloudStorageURI = atomic.NewString("")
IgnoreInlistPlanDigest = atomic.NewBool(DefTiDBIgnoreInlistPlanDigest)
)

var (
Expand Down

0 comments on commit 89945f5

Please sign in to comment.