Skip to content

Commit

Permalink
Merge #115013
Browse files Browse the repository at this point in the history 
115013: tree: do not elide cast during type checking for placeholders r=rafiss a=rafiss

Keeping the cast allows the type checker to resolve the placeholder correctly, since the cast contains information that hints the placeholder type.

fixes #114867

Release note (bug fix): Fixed a bug that would cause a prepared statement to fail if it referenced an enum as well as a table that has undergone a schema change.

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
  • Loading branch information
@rafiss
craig[bot] and rafiss committed Nov 27, 2023
2 parents 5a8c454 + 2886b1b commit ba6423f
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 15 deletions.
28 changes: 28 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/prepare
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1667,3 +1667,31 @@ RESET prepared_statements_cache_size

statement error EXPLAIN ANALYZE can only be used as a top-level statement
PREPARE p AS EXPLAIN ANALYZE SELECT 1

# Regression test for #114867 to make sure that a statement that uses an enum
# can be re-prepared after a schema change.
subtest reprepare_statement_with_enum

statement ok
CREATE TYPE color AS ENUM ('red', 'blue', 'green');
CREATE TABLE test_114867 (
id UUID DEFAULT gen_random_uuid() PRIMARY KEY,
colors color[] DEFAULT ARRAY[]::color[]
)

statement ok
PREPARE s_114867 AS INSERT INTO test_114867(colors) VALUES (ARRAY[$1::text]::color[]);
EXECUTE s_114867('red')

statement ok
TRUNCATE TABLE test_114867 CASCADE

statement ok
EXECUTE s_114867('red')

query T
SELECT colors FROM test_114867
----
{red}

subtest end
7 changes: 4 additions & 3 deletions pkg/sql/opt/memo/testdata/logprops/with
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,14 @@ WITH foo AS (SELECT $1::INT) SELECT 1 FROM foo
with &1 (foo)
├── columns: "?column?":3(int!null)
├── cardinality: [1 - 1]
├── has-placeholder
├── immutable, has-placeholder
├── key: ()
├── fd: ()-->(3)
├── prune: (3)
├── project
│ ├── columns: int8:1(int)
│ ├── cardinality: [1 - 1]
│ ├── has-placeholder
│ ├── immutable, has-placeholder
│ ├── key: ()
│ ├── fd: ()-->(1)
│ ├── prune: (1)
Expand All @@ -141,7 +141,8 @@ with &1 (foo)
│ │ ├── key: ()
│ │ └── tuple [type=tuple]
│ └── projections
│ └── placeholder: $1 [as=int8:1, type=int]
│ └── cast: INT8 [as=int8:1, type=int, immutable]
│ └── placeholder: $1 [type=int]
└── project
├── columns: "?column?":3(int!null)
├── cardinality: [1 - 1]
Expand Down
22 changes: 11 additions & 11 deletions pkg/sql/opt/optbuilder/testdata/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2352,25 +2352,25 @@ upsert assn_cast
│ ├── qc_cast:16 => qc:3
│ └── column4:13 => i:4
└── project
├── columns: upsert_k:31 upsert_s:32 upsert_d:33 upsert_d_comp:34 column4:13!null k_cast:14!null c_cast:15!null qc_cast:16!null s_default:17 d_default:18 d_comp_cast:20 k:21 c:22 qc:23 i:24 s:25 d:26 d_comp:27 crdb_internal_mvcc_timestamp:28 tableoid:29 d_comp_comp:30
├── columns: upsert_k:31 upsert_s:32 upsert_d:33 upsert_d_comp:34 column4:13!null k_cast:14 c_cast:15!null qc_cast:16!null s_default:17 d_default:18 d_comp_cast:20 k:21 c:22 qc:23 i:24 s:25 d:26 d_comp:27 crdb_internal_mvcc_timestamp:28 tableoid:29 d_comp_comp:30
├── project
│ ├── columns: d_comp_comp:30 column4:13!null k_cast:14!null c_cast:15!null qc_cast:16!null s_default:17 d_default:18 d_comp_cast:20 k:21 c:22 qc:23 i:24 s:25 d:26 d_comp:27 crdb_internal_mvcc_timestamp:28 tableoid:29
│ ├── columns: d_comp_comp:30 column4:13!null k_cast:14 c_cast:15!null qc_cast:16!null s_default:17 d_default:18 d_comp_cast:20 k:21 c:22 qc:23 i:24 s:25 d:26 d_comp:27 crdb_internal_mvcc_timestamp:28 tableoid:29
│ ├── left-join (hash)
│ │ ├── columns: column4:13!null k_cast:14!null c_cast:15!null qc_cast:16!null s_default:17 d_default:18 d_comp_cast:20 k:21 c:22 qc:23 i:24 s:25 d:26 d_comp:27 crdb_internal_mvcc_timestamp:28 tableoid:29
│ │ ├── columns: column4:13!null k_cast:14 c_cast:15!null qc_cast:16!null s_default:17 d_default:18 d_comp_cast:20 k:21 c:22 qc:23 i:24 s:25 d:26 d_comp:27 crdb_internal_mvcc_timestamp:28 tableoid:29
│ │ ├── ensure-upsert-distinct-on
│ │ │ ├── columns: column4:13!null k_cast:14!null c_cast:15!null qc_cast:16!null s_default:17 d_default:18 d_comp_cast:20
│ │ │ ├── grouping columns: k_cast:14!null
│ │ │ ├── columns: column4:13!null k_cast:14 c_cast:15!null qc_cast:16!null s_default:17 d_default:18 d_comp_cast:20
│ │ │ ├── grouping columns: k_cast:14
│ │ │ ├── project
│ │ │ │ ├── columns: d_comp_cast:20 column4:13!null k_cast:14!null c_cast:15!null qc_cast:16!null s_default:17 d_default:18
│ │ │ │ ├── columns: d_comp_cast:20 column4:13!null k_cast:14 c_cast:15!null qc_cast:16!null s_default:17 d_default:18
│ │ │ │ ├── project
│ │ │ │ │ ├── columns: d_comp_comp:19 column4:13!null k_cast:14!null c_cast:15!null qc_cast:16!null s_default:17 d_default:18
│ │ │ │ │ ├── columns: d_comp_comp:19 column4:13!null k_cast:14 c_cast:15!null qc_cast:16!null s_default:17 d_default:18
│ │ │ │ │ ├── project
│ │ │ │ │ │ ├── columns: s_default:17 d_default:18 column4:13!null k_cast:14!null c_cast:15!null qc_cast:16!null
│ │ │ │ │ │ ├── columns: s_default:17 d_default:18 column4:13!null k_cast:14 c_cast:15!null qc_cast:16!null
│ │ │ │ │ │ ├── project
│ │ │ │ │ │ │ ├── columns: k_cast:14!null c_cast:15!null qc_cast:16!null column4:13!null
│ │ │ │ │ │ │ ├── columns: k_cast:14 c_cast:15!null qc_cast:16!null column4:13!null
│ │ │ │ │ │ │ ├── values
│ │ │ │ │ │ │ │ ├── columns: column1:10!null column2:11!null column3:12!null column4:13!null
│ │ │ │ │ │ │ │ └── (1.0, ' ', 'foo', 1)
│ │ │ │ │ │ │ │ ├── columns: column1:10 column2:11!null column3:12!null column4:13!null
│ │ │ │ │ │ │ │ └── (1.0::DECIMAL, ' ', 'foo', 1)
│ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ ├── assignment-cast: INT8 [as=k_cast:14]
│ │ │ │ │ │ │ │ └── column1:10
Expand Down
7 changes: 6 additions & 1 deletion pkg/sql/sem/tree/type_check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,12 @@ func (expr *CastExpr) TypeCheck(
return nil, err
}
expr.Type = exprType
canElideCast := true

// Do not elide casts for placeholders, since if the statement gets re-prepared,
// the cast may be needed to infer the placeholder type.
_, isPlaceholder := expr.Expr.(*Placeholder)
canElideCast := !isPlaceholder

switch {
case isConstant(expr.Expr):
c := expr.Expr.(Constant)
Expand Down

0 comments on commit ba6423f

Please sign in to comment.