Skip to content

Commit

Permalink
Merge branch 'master' into issue_41058
Browse files Browse the repository at this point in the history
  • Loading branch information
fengou1 committed Feb 6, 2023
2 parents 01a64e9 + ba41d92 commit 562efde
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 47 deletions.
41 changes: 21 additions & 20 deletions expression/builtin_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -1565,17 +1565,33 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express
arg0Type, arg1Type := args[0].GetType(), args[1].GetType()
arg0IsInt := arg0Type.EvalType() == types.ETInt
arg1IsInt := arg1Type.EvalType() == types.ETInt
arg0IsString := arg0Type.EvalType() == types.ETString
arg1IsString := arg1Type.EvalType() == types.ETString
arg0, arg0IsCon := args[0].(*Constant)
arg1, arg1IsCon := args[1].(*Constant)
isExceptional, finalArg0, finalArg1 := false, args[0], args[1]
isPositiveInfinite, isNegativeInfinite := false, false
// int non-constant [cmp] non-int constant
if arg0IsInt && !arg0IsCon && !arg1IsInt && arg1IsCon {
if MaybeOverOptimized4PlanCache(ctx, []Expression{arg1}) {
ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: '%v' may be converted to INT", arg1.String()))
if MaybeOverOptimized4PlanCache(ctx, args) {
// To keep the result be compatible with MySQL, refine `int non-constant <cmp> str constant`
// here and skip this refine operation in all other cases for safety.
if (arg0IsInt && !arg0IsCon && arg1IsString && arg1IsCon) || (arg1IsInt && !arg1IsCon && arg0IsString && arg0IsCon) {
var reason error
if arg1IsString {
reason = errors.Errorf("skip plan-cache: '%v' may be converted to INT", arg1.String())
} else { // arg0IsString
reason = errors.Errorf("skip plan-cache: '%v' may be converted to INT", arg0.String())
}
ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(reason)
RemoveMutableConst(ctx, args)
} else {
return args
}

} else if !ctx.GetSessionVars().StmtCtx.UseCache {
// We should remove the mutable constant for correctness, because its value may be changed.
RemoveMutableConst(ctx, args)
}
// int non-constant [cmp] non-int constant
if arg0IsInt && !arg0IsCon && !arg1IsInt && arg1IsCon {
arg1, isExceptional = RefineComparedConstant(ctx, *arg0Type, arg1, c.op)
// Why check not null flag
// eg: int_col > const_val(which is less than min_int32)
Expand Down Expand Up @@ -1603,11 +1619,6 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express
}
// non-int constant [cmp] int non-constant
if arg1IsInt && !arg1IsCon && !arg0IsInt && arg0IsCon {
if MaybeOverOptimized4PlanCache(ctx, []Expression{arg0}) {
ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: '%v' may be converted to INT", arg0.String()))
RemoveMutableConst(ctx, args)
}

arg0, isExceptional = RefineComparedConstant(ctx, *arg1Type, arg0, symmetricOp[c.op])
if !isExceptional || (isExceptional && mysql.HasNotNullFlag(arg1Type.GetFlag())) {
finalArg0 = arg0
Expand All @@ -1625,11 +1636,6 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express
}
// int constant [cmp] year type
if arg0IsCon && arg0IsInt && arg1Type.GetType() == mysql.TypeYear && !arg0.Value.IsNull() {
if MaybeOverOptimized4PlanCache(ctx, []Expression{arg0}) {
ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: '%v' may be converted to YEAR", arg0.String()))
RemoveMutableConst(ctx, args)
}

adjusted, failed := types.AdjustYear(arg0.Value.GetInt64(), false)
if failed == nil {
arg0.Value.SetInt64(adjusted)
Expand All @@ -1638,11 +1644,6 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express
}
// year type [cmp] int constant
if arg1IsCon && arg1IsInt && arg0Type.GetType() == mysql.TypeYear && !arg1.Value.IsNull() {
if MaybeOverOptimized4PlanCache(ctx, []Expression{arg1}) {
ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: '%v' may be converted to YEAR", arg1.String()))
RemoveMutableConst(ctx, args)
}

adjusted, failed := types.AdjustYear(arg1.Value.GetInt64(), false)
if failed == nil {
arg1.Value.SetInt64(adjusted)
Expand Down
2 changes: 1 addition & 1 deletion planner/core/expression_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1563,7 +1563,7 @@ func (er *expressionRewriter) inToExpression(lLen int, not bool, tp *types.Field
continue // no need to refine it
}
er.sctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: '%v' may be converted to INT", c.String()))
expression.RemoveMutableConst(er.sctx, args)
expression.RemoveMutableConst(er.sctx, []expression.Expression{c})
}
args[i], isExceptional = expression.RefineComparedConstant(er.sctx, *leftFt, c, opcode.EQ)
if isExceptional {
Expand Down
19 changes: 0 additions & 19 deletions planner/core/plan_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,22 +506,3 @@ func TestPlanCacheWithLimit(t *testing.T) {
tk.MustExec("execute stmt using @a")
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 skip plan-cache: limit count more than 10000"))
}

func TestIssue40679(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("create table t (a int, key(a));")
tk.MustExec("prepare st from 'select * from t use index(a) where a < ?'")
tk.MustExec("set @a1=1.1")
tk.MustExec("execute st using @a1")

tkProcess := tk.Session().ShowProcess()
ps := []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
rows := tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Rows()
require.True(t, strings.Contains(rows[1][0].(string), "RangeScan")) // RangeScan not FullScan

tk.MustExec("execute st using @a1")
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 skip plan-cache: '1.1' may be converted to INT"))
}
5 changes: 3 additions & 2 deletions session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -4150,8 +4150,9 @@ func (s *session) EncodeSessionStates(ctx context.Context, sctx sessionctx.Conte
sessionStates.SystemVars = make(map[string]string)
for _, sv := range variable.GetSysVars() {
switch {
case sv.Hidden, sv.HasNoneScope(), sv.HasInstanceScope(), !sv.HasSessionScope():
// Hidden and none-scoped variables cannot be modified.
case sv.HasNoneScope(), sv.HasInstanceScope(), !sv.HasSessionScope():
// Hidden attribute is deprecated.
// None-scoped variables cannot be modified.
// Instance-scoped variables don't need to be encoded.
// Noop variables should also be migrated even if they are noop.
continue
Expand Down
3 changes: 2 additions & 1 deletion sessionctx/sessionstates/session_states_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ func TestSystemVars(t *testing.T) {
},
{
// hidden variable
inSessionStates: false,
inSessionStates: true,
varName: variable.TiDBTxnReadTS,
expectedValue: "",
},
{
// none-scoped variable
Expand Down
2 changes: 2 additions & 0 deletions sessionctx/variable/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,12 @@ go_test(
"//parser/mysql",
"//parser/terror",
"//planner/core",
"//sessionctx/sessionstates",
"//sessionctx/stmtctx",
"//testkit",
"//testkit/testsetup",
"//types",
"//util",
"//util/chunk",
"//util/execdetails",
"//util/gctuner",
Expand Down
6 changes: 2 additions & 4 deletions sessionctx/variable/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -2320,16 +2320,16 @@ func (s *SessionVars) GetTemporaryTable(tblInfo *model.TableInfo) tableutil.Temp
// EncodeSessionStates saves session states into SessionStates.
func (s *SessionVars) EncodeSessionStates(ctx context.Context, sessionStates *sessionstates.SessionStates) (err error) {
// Encode user-defined variables.
s.userVars.lock.RLock()
sessionStates.UserVars = make(map[string]*types.Datum, len(s.userVars.values))
sessionStates.UserVarTypes = make(map[string]*ptypes.FieldType, len(s.userVars.types))
s.userVars.lock.RLock()
defer s.userVars.lock.RUnlock()
for name, userVar := range s.userVars.values {
sessionStates.UserVars[name] = userVar.Clone()
}
for name, userVarType := range s.userVars.types {
sessionStates.UserVarTypes[name] = userVarType.Clone()
}
s.userVars.lock.RUnlock()

// Encode other session contexts.
sessionStates.PreparedStmtID = s.preparedStmtID
Expand Down Expand Up @@ -2357,11 +2357,9 @@ func (s *SessionVars) EncodeSessionStates(ctx context.Context, sessionStates *se
// DecodeSessionStates restores session states from SessionStates.
func (s *SessionVars) DecodeSessionStates(ctx context.Context, sessionStates *sessionstates.SessionStates) (err error) {
// Decode user-defined variables.
s.userVars.values = make(map[string]types.Datum, len(sessionStates.UserVars))
for name, userVar := range sessionStates.UserVars {
s.SetUserVarVal(name, *userVar.Clone())
}
s.userVars.types = make(map[string]*ptypes.FieldType, len(sessionStates.UserVarTypes))
for name, userVarType := range sessionStates.UserVarTypes {
s.SetUserVarType(name, userVarType.Clone())
}
Expand Down
36 changes: 36 additions & 0 deletions sessionctx/variable/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package variable_test

import (
"context"
"strconv"
"sync"
"testing"
"time"
Expand All @@ -25,10 +27,12 @@ import (
"github.com/pingcap/tidb/parser/auth"
"github.com/pingcap/tidb/parser/mysql"
plannercore "github.com/pingcap/tidb/planner/core"
"github.com/pingcap/tidb/sessionctx/sessionstates"
"github.com/pingcap/tidb/sessionctx/stmtctx"
"github.com/pingcap/tidb/sessionctx/variable"
"github.com/pingcap/tidb/testkit"
"github.com/pingcap/tidb/types"
util2 "github.com/pingcap/tidb/util"
"github.com/pingcap/tidb/util/chunk"
"github.com/pingcap/tidb/util/execdetails"
"github.com/pingcap/tidb/util/mock"
Expand Down Expand Up @@ -533,3 +537,35 @@ func TestPretectedTSList(t *testing.T) {
require.Equal(t, uint64(0), lst.GetMinProtectedTS(1))
require.Equal(t, 0, lst.Size())
}

func TestUserVarConcurrently(t *testing.T) {
sv := variable.NewSessionVars(nil)
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
var wg util2.WaitGroupWrapper
wg.Run(func() {
for i := 0; ; i++ {
select {
case <-time.After(time.Millisecond):
name := strconv.Itoa(i)
sv.SetUserVarVal(name, types.Datum{})
sv.GetUserVarVal(name)
case <-ctx.Done():
return
}
}
})
wg.Run(func() {
for {
select {
case <-time.After(time.Millisecond):
var states sessionstates.SessionStates
require.NoError(t, sv.EncodeSessionStates(ctx, &states))
require.NoError(t, sv.DecodeSessionStates(ctx, &states))
case <-ctx.Done():
return
}
}
})
wg.Wait()
cancel()
}

0 comments on commit 562efde

Please sign in to comment.