Skip to content

Commit

Permalink
Merge pull request #81364 from rytaft/backport21.2-81331
Browse files Browse the repository at this point in the history
release-21.2: opt: do not use placeholder fast path if types do not match
  • Loading branch information
rytaft authored May 17, 2022
2 parents b4acf0d + 3aa546a commit 20e293b
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 0 deletions.
27 changes: 27 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/prepare
Original file line number Diff line number Diff line change
Expand Up @@ -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)
13 changes: 13 additions & 0 deletions pkg/sql/opt/xform/placeholder_fast_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
26 changes: 26 additions & 0 deletions pkg/sql/opt/xform/testdata/placeholder-fast-path/scan
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 20e293b

Please sign in to comment.