Merge pull request #67687 from cockroachdb/blathers/backport-release-…

…20.2-67651 release-20.2: sql: require placeholder types to be identical to use a cached plan
cockroachdb · Sep 13, 2021 · 15b962c · 15b962c
2 parents 7721697 + 6637a33
commit 15b962c
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 11 deletions.
diff --git a/pkg/sql/plan_opt.go b/pkg/sql/plan_opt.go
@@ -97,7 +97,7 @@ func (p *planner) prepareUsingOptimizer(ctx context.Context) (planFlags, error)
 		if ok && cachedData.PrepareMetadata != nil {
 			pm := cachedData.PrepareMetadata
 			// Check that the type hints match (the type hints affect type checking).
-			if !pm.TypeHints.Equals(p.semaCtx.Placeholders.TypeHints) {
+			if !pm.TypeHints.Identical(p.semaCtx.Placeholders.TypeHints) {
 				opc.log(ctx, "query cache hit but type hints don't match")
 			} else {
 				isStale, err := cachedData.Memo.IsStale(ctx, p.EvalContext(), &opc.catalog)

diff --git a/pkg/sql/sem/tree/placeholders.go b/pkg/sql/sem/tree/placeholders.go
@@ -40,8 +40,8 @@ func (idx PlaceholderIdx) String() string {
 // placeholders in the statement. Entries that don't yet have a type are nil.
 type PlaceholderTypes []*types.T
 
-// Equals returns true if two PlaceholderTypes contain the same types.
-func (pt PlaceholderTypes) Equals(other PlaceholderTypes) bool {
+// Identical returns true if two PlaceholderTypes contain the same types.
+func (pt PlaceholderTypes) Identical(other PlaceholderTypes) bool {
 	if len(pt) != len(other) {
 		return false
 	}
@@ -50,7 +50,7 @@ func (pt PlaceholderTypes) Equals(other PlaceholderTypes) bool {
 		case t == nil && other[i] == nil:
 		case t == nil || other[i] == nil:
 			return false
-		case !t.Equivalent(other[i]):
+		case !t.Identical(other[i]):
 			return false
 		}
 	}

diff --git a/pkg/sql/sem/tree/placeholders_test.go b/pkg/sql/sem/tree/placeholders_test.go
@@ -19,12 +19,12 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 )
 
-func TestPlaceholderTypesEquals(t *testing.T) {
+func TestPlaceholderTypesIdentical(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 	testCases := []struct {
-		a, b  PlaceholderTypes
-		equal bool
+		a, b      PlaceholderTypes
+		identical bool
 	}{
 		{ // 0
 			PlaceholderTypes{},
@@ -61,14 +61,29 @@ func TestPlaceholderTypesEquals(t *testing.T) {
 			PlaceholderTypes{types.Int, nil},
 			false,
 		},
+		{ // 7
+			PlaceholderTypes{types.Int, nil},
+			PlaceholderTypes{types.Int4, nil},
+			false,
+		},
+		{ // 8
+			PlaceholderTypes{types.Int},
+			PlaceholderTypes{types.Int4},
+			false,
+		},
+		{ // 9
+			PlaceholderTypes{types.Int4},
+			PlaceholderTypes{types.Int4},
+			true,
+		},
 	}
 	for i, tc := range testCases {
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
-			res := tc.a.Equals(tc.b)
-			if res != tc.equal {
-				t.Errorf("%v vs %v: expected %t, got %t", tc.a, tc.b, tc.equal, res)
+			res := tc.a.Identical(tc.b)
+			if res != tc.identical {
+				t.Errorf("%v vs %v: expected %t, got %t", tc.a, tc.b, tc.identical, res)
 			}
-			res2 := tc.b.Equals(tc.a)
+			res2 := tc.b.Identical(tc.a)
 			if res != res2 {
 				t.Errorf("%v vs %v: not commutative", tc.a, tc.b)
 			}