bind: fix global binding hint for resource groups (#43738) (#43780)

ref #38825, close #43736
pingcap · May 15, 2023 · 2217379 · 2217379
1 parent 5a78c30
commit 2217379
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 25 deletions.
diff --git a/ddl/resourcegrouptest/BUILD.bazel b/ddl/resourcegrouptest/BUILD.bazel
@@ -6,7 +6,7 @@ go_test(
     srcs = ["resource_group_test.go"],
     flaky = True,
     race = "on",
-    shard_count = 4,
+    shard_count = 5,
     deps = [
         "//ddl/internal/callback",
         "//ddl/resourcegroup",

diff --git a/ddl/resourcegrouptest/resource_group_test.go b/ddl/resourcegrouptest/resource_group_test.go
@@ -111,21 +111,21 @@ func TestResourceGroupBasic(t *testing.T) {
 
 	tk.MustExec("create resource group y RU_PER_SEC=4000")
 	checkFunc = func(groupInfo *model.ResourceGroupInfo) {
-		require.Equal(t, true, groupInfo.ID != 0)
-		require.Equal(t, "y", groupInfo.Name.L)
-		require.Equal(t, groupID.Load(), groupInfo.ID)
-		require.Equal(t, uint64(4000), groupInfo.RURate)
-		require.Equal(t, int64(4000), groupInfo.BurstLimit)
+		re.Equal(true, groupInfo.ID != 0)
+		re.Equal("y", groupInfo.Name.L)
+		re.Equal(groupID.Load(), groupInfo.ID)
+		re.Equal(uint64(4000), groupInfo.RURate)
+		re.Equal(int64(4000), groupInfo.BurstLimit)
 	}
 	g = testResourceGroupNameFromIS(t, tk.Session(), "y")
 	checkFunc(g)
 	tk.MustExec("alter resource group y BURSTABLE RU_PER_SEC=5000")
 	checkFunc = func(groupInfo *model.ResourceGroupInfo) {
-		require.Equal(t, true, groupInfo.ID != 0)
-		require.Equal(t, "y", groupInfo.Name.L)
-		require.Equal(t, groupID.Load(), groupInfo.ID)
-		require.Equal(t, uint64(5000), groupInfo.RURate)
-		require.Equal(t, int64(-1), groupInfo.BurstLimit)
+		re.Equal(true, groupInfo.ID != 0)
+		re.Equal("y", groupInfo.Name.L)
+		re.Equal(groupID.Load(), groupInfo.ID)
+		re.Equal(uint64(5000), groupInfo.RURate)
+		re.Equal(int64(-1), groupInfo.BurstLimit)
 	}
 	g = testResourceGroupNameFromIS(t, tk.Session(), "y")
 	checkFunc(g)
@@ -142,8 +142,8 @@ func TestResourceGroupBasic(t *testing.T) {
 	tk.MustGetErrCode("create resource group x  burstable, ru_per_sec=1000, burstable", mysql.ErrParse)
 	tk.MustContainErrMsg("create resource group x burstable, ru_per_sec=1000, burstable", "Dupliated options specified")
 	groups, err := infosync.ListResourceGroups(context.TODO())
-	require.Equal(t, 1, len(groups))
-	require.NoError(t, err)
+	re.Equal(1, len(groups))
+	re.NoError(err)
 
 	// Check information schema table information_schema.resource_groups
 	tk.MustExec("create resource group x RU_PER_SEC=1000 PRIORITY=LOW")
@@ -345,3 +345,24 @@ func TestNewResourceGroupFromOptions(t *testing.T) {
 		}
 	}
 }
+
+func TestBindHints(t *testing.T) {
+	store := testkit.CreateMockStore(t)
+	tk := testkit.NewTestKit(t, store)
+	re := require.New(t)
+
+	tk.MustExec("drop resource group if exists rg1")
+	tk.MustExec("create resource group rg1 RU_PER_SEC=1000")
+
+	tk.MustExec("use test")
+	tk.MustExec("drop table if exists t")
+	tk.MustExec("create table t(a int, b int)")
+
+	tk.MustExec("create global binding for select * from t using select /*+ resource_group(rg1) */ * from t")
+	tk.MustQuery("select * from t")
+	re.Equal("rg1", tk.Session().GetSessionVars().StmtCtx.ResourceGroup)
+	re.Equal("", tk.Session().GetSessionVars().ResourceGroupName)
+	tk.MustQuery("select a, b from t")
+	re.Equal("", tk.Session().GetSessionVars().StmtCtx.ResourceGroup)
+	re.Equal("", tk.Session().GetSessionVars().ResourceGroupName)
+}
diff --git a/planner/optimize.go b/planner/optimize.go
@@ -180,17 +180,6 @@ func Optimize(ctx context.Context, sctx sessionctx.Context, node ast.Node, is in
 		}
 	}
 
-	// Override the resource group if necessary
-	// TODO: we didn't check the existence of the hinted resource group now to save the cost per query
-	if originStmtHints.HasResourceGroup {
-		if variable.EnableResourceControl.Load() {
-			sessVars.ResourceGroupName = originStmtHints.ResourceGroup
-		} else {
-			err := infoschema.ErrResourceGroupSupportDisabled
-			sessVars.StmtCtx.AppendWarning(err)
-		}
-	}
-
 	txnManger := sessiontxn.GetTxnManager(sctx)
 	if _, isolationReadContainTiKV := sessVars.IsolationReadEngines[kv.TiKV]; isolationReadContainTiKV {
 		var fp core.Plan
@@ -315,6 +304,18 @@ func Optimize(ctx context.Context, sctx sessionctx.Context, node ast.Node, is in
 		// Restore the hint to avoid changing the stmt node.
 		hint.BindHint(stmtNode, originHints)
 	}
+
+	// Override the resource group if necessary
+	// TODO: we didn't check the existence of the hinted resource group now to save the cost per query
+	if sessVars.StmtCtx.StmtHints.HasResourceGroup {
+		if variable.EnableResourceControl.Load() {
+			sessVars.ResourceGroupName = sessVars.StmtCtx.StmtHints.ResourceGroup
+		} else {
+			err := infoschema.ErrResourceGroupSupportDisabled
+			sessVars.StmtCtx.AppendWarning(err)
+		}
+	}
+
 	if sessVars.StmtCtx.EnableOptimizerDebugTrace && bestPlanFromBind != nil {
 		core.DebugTraceBestBinding(sctx, chosenBinding.Hint)
 	}

diff --git a/session/session.go b/session/session.go
@@ -2170,7 +2170,7 @@ func (s *session) ExecuteStmt(ctx context.Context, stmtNode ast.StmtNode) (sqlex
 	// Transform abstract syntax tree to a physical plan(stored in executor.ExecStmt).
 	compiler := executor.Compiler{Ctx: s}
 	stmt, err := compiler.Compile(ctx, stmtNode)
-	// session resource-group might be changed by query hint, ensure the resoure it back when
+	// session resource-group might be changed by query hint, ensure restore it back when
 	// the execution finished.
 	if s.GetSessionVars().ResourceGroupName != originalResourceGroup {
 		defer func() {