From 3aa546a19dac80756e7814a96ed897f245eb4d1f Mon Sep 17 00:00:00 2001
From: Rebecca Taft <becca@cockroachlabs.com>
Date: Mon, 16 May 2022 16:49:30 -0500
Subject: [PATCH] opt: do not use placeholder fast path if types do not match

Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes #81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column a was of type
DECIMAL, the following prepared query could produce incorrect results
when executed:
SELECT * FROM t_dec WHERE a = $1::INT8;
---
 pkg/sql/logictest/testdata/logic_test/prepare | 27 +++++++++++++++++++
 pkg/sql/opt/xform/placeholder_fast_path.go    | 13 +++++++++
 .../xform/testdata/placeholder-fast-path/scan | 26 ++++++++++++++++++
 3 files changed, 66 insertions(+)

diff --git a/pkg/sql/logictest/testdata/logic_test/prepare b/pkg/sql/logictest/testdata/logic_test/prepare
index 394c5be447dd..25c4f1f35c34 100644
--- a/pkg/sql/logictest/testdata/logic_test/prepare
+++ b/pkg/sql/logictest/testdata/logic_test/prepare
@@ -1390,3 +1390,30 @@ PREPARE q64765 as SELECT * FROM t64765 WHERE x = $1 AND y = $2
 
 statement ok
 EXECUTE q64765(1, 1)
+
+# Regression test for #81315.
+statement ok
+CREATE TABLE t81315 (a DECIMAL NOT NULL PRIMARY KEY, b INT);
+PREPARE q81315 AS SELECT * FROM t81315 WHERE a = $1::INT8;
+INSERT INTO t81315 VALUES (1, 100), (2, 200)
+
+query TI
+SELECT * FROM t81315 WHERE a = 1
+----
+1  100
+
+query TI
+EXECUTE q81315 (1)
+----
+1  100
+
+statement ok
+CREATE TABLE t81315_2 (
+  k INT PRIMARY KEY,
+  a INT
+);
+PREPARE q81315_2 AS SELECT * FROM t81315_2 WHERE k = $1;
+INSERT INTO t81315_2 VALUES (1, 1)
+
+statement error expected EXECUTE parameter expression to have type int
+EXECUTE q81315_2(1::DECIMAL)
diff --git a/pkg/sql/opt/xform/placeholder_fast_path.go b/pkg/sql/opt/xform/placeholder_fast_path.go
index a562b2a124fe..31cf0708d756 100644
--- a/pkg/sql/opt/xform/placeholder_fast_path.go
+++ b/pkg/sql/opt/xform/placeholder_fast_path.go
@@ -15,6 +15,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/props/physical"
+	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/errors"
@@ -198,6 +199,9 @@ func (o *Optimizer) TryPlaceholderFastPath() (_ opt.Expr, ok bool, err error) {
 		for j := range sel.Filters {
 			eq := sel.Filters[j].Condition.(*memo.EqExpr)
 			if v := eq.Left.(*memo.VariableExpr); v.Col == col {
+				if !verifyType(o.mem.Metadata(), col, eq.Right.DataType()) {
+					return nil, false, nil
+				}
 				span[i] = eq.Right
 				break
 			}
@@ -221,3 +225,12 @@ func (o *Optimizer) TryPlaceholderFastPath() (_ opt.Expr, ok bool, err error) {
 
 	return placeholderScan, true, nil
 }
+
+// verifyType checks that the type of the index column col matches the
+// given type. We disallow mixed-type comparisons because it would result in
+// incorrect encodings (See #4313 and #81315).
+// TODO(rytaft): We may be able to use the placeholder fast path for
+// this case if we add logic similar to UnifyComparisonTypes.
+func verifyType(md *opt.Metadata, col opt.ColumnID, typ *types.T) bool {
+	return typ.Family() == types.UnknownFamily || md.ColumnMeta(col).Type.Equivalent(typ)
+}
diff --git a/pkg/sql/opt/xform/testdata/placeholder-fast-path/scan b/pkg/sql/opt/xform/testdata/placeholder-fast-path/scan
index a556f2fe6305..aa0c3572f7a1 100644
--- a/pkg/sql/opt/xform/testdata/placeholder-fast-path/scan
+++ b/pkg/sql/opt/xform/testdata/placeholder-fast-path/scan
@@ -300,3 +300,29 @@ placeholder-fast-path
 SELECT * FROM xyz WHERE x = $1 AND y = $2
 ----
 no fast path
+
+# Regression test for #81315. Do not use the placeholder fast path
+# if the types do not match.
+exec-ddl
+CREATE TABLE t_dec (a DECIMAL NOT NULL PRIMARY KEY, b INT);
+----
+
+# TODO(rytaft): We may be able to use the placeholder fast path for
+# this case if we add logic similar to UnifyComparisonTypes.
+placeholder-fast-path
+SELECT * FROM t_dec WHERE a = $1::INT8;
+----
+no fast path
+
+placeholder-fast-path
+SELECT * FROM t_dec WHERE a = $1;
+----
+placeholder-scan t_dec
+ ├── columns: a:1!null b:2
+ ├── cardinality: [0 - 1]
+ ├── immutable, has-placeholder
+ ├── stats: [rows=1, distinct(1)=1, null(1)=0]
+ ├── key: ()
+ ├── fd: ()-->(1,2)
+ └── span
+      └── $1