planner: check whether current task is valid when comparing task costs (

#34606) ref #33945
pingcap · May 16, 2022 · 180ae24 · 180ae24
1 parent 9392793
commit 180ae24
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 2 deletions.
diff --git a/planner/core/find_best_task.go b/planner/core/find_best_task.go
@@ -289,6 +289,12 @@ func (p *baseLogicalPlan) enumeratePhysicalPlans4Task(physicalPlans []PhysicalPl
 
 // compareTaskCost compares cost of curTask and bestTask and returns whether curTask's cost is smaller than bestTask's.
 func compareTaskCost(ctx sessionctx.Context, curTask, bestTask task) (curIsBetter bool, err error) {
+	if curTask.invalid() {
+		return false, nil
+	}
+	if bestTask.invalid() {
+		return true, nil
+	}
 	if ctx.GetSessionVars().EnableNewCostInterface { // use the new cost interface
 		curCost, err := getTaskPlanCost(curTask)
 		if err != nil {
@@ -298,9 +304,9 @@ func compareTaskCost(ctx sessionctx.Context, curTask, bestTask task) (curIsBette
 		if err != nil {
 			return false, err
 		}
-		return curCost < bestCost || (bestTask.invalid() && !curTask.invalid()), nil
+		return curCost < bestCost, nil
 	}
-	return curTask.cost() < bestTask.cost() || (bestTask.invalid() && !curTask.invalid()), nil
+	return curTask.cost() < bestTask.cost(), nil
 }
 
 func getTaskPlanCost(t task) (float64, error) {

diff --git a/planner/core/plan_cost_test.go b/planner/core/plan_cost_test.go
@@ -19,6 +19,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/pingcap/failpoint"
 	"github.com/pingcap/tidb/domain"
 	"github.com/pingcap/tidb/parser/model"
 	"github.com/pingcap/tidb/testkit"
@@ -71,6 +72,11 @@ func checkCost(t *testing.T, tk *testkit.TestKit, q, info string) {
 }
 
 func TestNewCostInterfaceTiKV(t *testing.T) {
+	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/planner/core/DisableProjectionPostOptimization", "return(true)"))
+	defer func() {
+		require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/planner/core/DisableProjectionPostOptimization"))
+	}()
+
 	store, clean := testkit.CreateMockStore(t)
 	defer clean()
 	tk := testkit.NewTestKit(t, store)
@@ -255,6 +261,11 @@ func TestNewCostInterfaceTiKV(t *testing.T) {
 }
 
 func TestNewCostInterfaceTiFlash(t *testing.T) {
+	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/planner/core/DisableProjectionPostOptimization", "return(true)"))
+	defer func() {
+		require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/planner/core/DisableProjectionPostOptimization"))
+	}()
+
 	store, clean := testkit.CreateMockStore(t)
 	defer clean()
 	tk := testkit.NewTestKit(t, store)
@@ -438,6 +449,11 @@ func TestNewCostInterfaceTiFlash(t *testing.T) {
 }
 
 func TestNewCostInterfaceRandGen(t *testing.T) {
+	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/planner/core/DisableProjectionPostOptimization", "return(true)"))
+	defer func() {
+		require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/planner/core/DisableProjectionPostOptimization"))
+	}()
+
 	store, clean := testkit.CreateMockStore(t)
 	defer clean()
 	tk := testkit.NewTestKit(t, store)

diff --git a/planner/core/rule_eliminate_projection.go b/planner/core/rule_eliminate_projection.go
@@ -19,6 +19,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/pingcap/failpoint"
 	"github.com/pingcap/tidb/expression"
 	"github.com/pingcap/tidb/kv"
 	"github.com/pingcap/tidb/parser/mysql"
@@ -130,6 +131,12 @@ func doPhysicalProjectionElimination(p PhysicalPlan) PhysicalPlan {
 // eliminatePhysicalProjection should be called after physical optimization to
 // eliminate the redundant projection left after logical projection elimination.
 func eliminatePhysicalProjection(p PhysicalPlan) PhysicalPlan {
+	failpoint.Inject("DisableProjectionPostOptimization", func(val failpoint.Value) {
+		if val.(bool) {
+			failpoint.Return(p)
+		}
+	})
+
 	oldSchema := p.Schema()
 	newRoot := doPhysicalProjectionElimination(p)
 	newCols := newRoot.Schema().Columns

diff --git a/planner/core/rule_inject_extra_projection.go b/planner/core/rule_inject_extra_projection.go
@@ -15,6 +15,7 @@
 package core
 
 import (
+	"github.com/pingcap/failpoint"
 	"github.com/pingcap/tidb/expression"
 	"github.com/pingcap/tidb/expression/aggregation"
 	"github.com/pingcap/tidb/kv"
@@ -32,6 +33,12 @@ import (
 // 2. TiDB can be used as a coprocessor, when a plan tree been pushed down to
 // TiDB, we need to inject extra projections for the plan tree as well.
 func InjectExtraProjection(plan PhysicalPlan) PhysicalPlan {
+	failpoint.Inject("DisableProjectionPostOptimization", func(val failpoint.Value) {
+		if val.(bool) {
+			failpoint.Return(plan)
+		}
+	})
+
 	return NewProjInjector().inject(plan)
 }