Skip to content

Commit

Permalink
sql: add ReType to resolveAndRequireType to fix vector engine panic
Browse files Browse the repository at this point in the history 
Fixes #66708

The vector engine needs exact type coercion, specifically when piping
computed column values into downstream operators.  Without this fix
the computed column was left as an int64 instead of cast back to the
required int16 type.

Exposed by sqlsmith, kudos to @michae2 for the review help

Release note: None
  • Loading branch information
@cucaroach
cucaroach committed Jun 28, 2021
1 parent e05dfe0 commit 394c407
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 20 deletions.
4 changes: 2 additions & 2 deletions pkg/sql/opt/exec/execbuilder/testdata/hash_sharded_index
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ vectorized: true
└── • render
│ columns: (crdb_internal_a_shard_11_comp, column1)
│ estimated row count: 2
│ render crdb_internal_a_shard_11_comp: mod(fnv32(COALESCE(column1::STRING, '')), 11)
│ render crdb_internal_a_shard_11_comp: mod(fnv32(COALESCE(column1::STRING, '')), 11)::INT4
│ render column1: column1
│
└── • values
Expand Down Expand Up @@ -63,7 +63,7 @@ vectorized: true
└── • render
│ columns: (crdb_internal_a_shard_12_comp, rowid_default, column1)
│ estimated row count: 2
│ render crdb_internal_a_shard_12_comp: mod(fnv32(COALESCE(column1::STRING, '')), 12)
│ render crdb_internal_a_shard_12_comp: mod(fnv32(COALESCE(column1::STRING, '')), 12)::INT4
│ render rowid_default: unique_rowid()
│ render column1: column1
│
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/optbuilder/scope.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ func (s *scope) resolveAndRequireType(expr tree.Expr, desired *types.T) tree.Typ
if err != nil {
panic(err)
}
return s.ensureNullType(texpr, desired)
return tree.ReType(s.ensureNullType(texpr, desired), desired)
}

// ensureNullType tests the type of the given expression. If types.Unknown, then
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/optbuilder/testdata/insert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1168,13 +1168,13 @@ insert decimals
│ │ │ │ │ ├── columns: column1:7!null column2:8
│ │ │ │ │ └── (1.1, ARRAY[0.95,NULL,15])
│ │ │ │ └── projections
│ │ │ │ └── 1.23 [as=c_default:9]
│ │ │ │ └── 1.23::DECIMAL(10,1) [as=c_default:9]
│ │ │ └── projections
│ │ │ ├── crdb_internal.round_decimal_values(column1:7, 0) [as=a:10]
│ │ │ ├── crdb_internal.round_decimal_values(column2:8, 1) [as=b:11]
│ │ │ └── crdb_internal.round_decimal_values(c_default:9, 1) [as=c_default:12]
│ │ └── projections
│ │ └── a:10 + c_default:12 [as=d_comp:13]
│ │ └── (a:10 + c_default:12::DECIMAL)::DECIMAL(10,1) [as=d_comp:13]
│ └── projections
│ └── crdb_internal.round_decimal_values(d_comp:13, 1) [as=d_comp:14]
└── projections
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/optbuilder/testdata/update
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1631,15 +1631,15 @@ update decimals
│ │ │ │ │ ├── columns: a:7!null b:8 c:9 d:10 crdb_internal_mvcc_timestamp:11 tableoid:12
│ │ │ │ │ └── computed column expressions
│ │ │ │ │ └── d:10
│ │ │ │ │ └── a:7::DECIMAL + c:9::DECIMAL
│ │ │ │ │ └── (a:7::DECIMAL + c:9::DECIMAL)::DECIMAL(10,1)
│ │ │ │ └── projections
│ │ │ │ ├── 1.1 [as=a_new:13]
│ │ │ │ └── ARRAY[0.95,NULL,15] [as=b_new:14]
│ │ │ └── projections
│ │ │ ├── crdb_internal.round_decimal_values(a_new:13, 0) [as=a_new:15]
│ │ │ └── crdb_internal.round_decimal_values(b_new:14, 1) [as=b_new:16]
│ │ └── projections
│ │ └── a_new:15 + c:9::DECIMAL [as=d_comp:17]
│ │ └── (a_new:15 + c:9::DECIMAL)::DECIMAL(10,1) [as=d_comp:17]
│ └── projections
│ └── crdb_internal.round_decimal_values(d_comp:17, 1) [as=d_comp:18]
└── projections
Expand Down
52 changes: 52 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/update-col-cast-bug
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
exec-ddl
create table t (a int2, b int2, c int2 as (a + b) virtual)
----

build format=show-types
update t set a = (with cte as (select 1:::int8) select t.c from cte limit 1)
----
with &1 (cte)
├── project
│ ├── columns: int8:13(int!null)
│ ├── values
│ │ └── () [type=tuple]
│ └── projections
│ └── 1 [as=int8:13, type=int]
└── update t
├── columns: <none>
├── fetch columns: a:7(int2) b:8(int2) t.c:9(int2) rowid:10(int)
├── update-mapping:
│ ├── a_new:16 => a:1
│ └── c_comp:17 => t.c:3
└── project
├── columns: c_comp:17(int2) a:7(int2) b:8(int2) t.c:9(int2) rowid:10(int!null) crdb_internal_mvcc_timestamp:11(decimal) tableoid:12(oid) a_new:16(int2)
├── project
│ ├── columns: a_new:16(int2) a:7(int2) b:8(int2) t.c:9(int2) rowid:10(int!null) crdb_internal_mvcc_timestamp:11(decimal) tableoid:12(oid)
│ ├── project
│ │ ├── columns: t.c:9(int2) a:7(int2) b:8(int2) rowid:10(int!null) crdb_internal_mvcc_timestamp:11(decimal) tableoid:12(oid)
│ │ ├── scan t
│ │ │ ├── columns: a:7(int2) b:8(int2) rowid:10(int!null) crdb_internal_mvcc_timestamp:11(decimal) tableoid:12(oid)
│ │ │ └── computed column expressions
│ │ │ └── t.c:9
│ │ │ └── (a:7::INT8 + b:8::INT8)::INT2 [type=int2]
│ │ └── projections
│ │ └── (a:7::INT8 + b:8::INT8)::INT2 [as=t.c:9, type=int2]
│ └── projections
│ └── subquery [as=a_new:16, type=int2]
│ └── max1-row
│ ├── columns: c:15(int2)
│ └── limit
│ ├── columns: c:15(int2)
│ ├── project
│ │ ├── columns: c:15(int2)
│ │ ├── limit hint: 1.00
│ │ ├── with-scan &1 (cte)
│ │ │ ├── columns: int8:14(int!null)
│ │ │ ├── mapping:
│ │ │ │ └── int8:13(int) => int8:14(int)
│ │ │ └── limit hint: 1.00
│ │ └── projections
│ │ └── t.c:9 [as=c:15, type=int2]
│ └── 1 [type=int]
└── projections
└── (a_new:16::INT8 + b:8::INT8)::INT2 [as=c_comp:17, type=int2]
24 changes: 12 additions & 12 deletions pkg/sql/opt/optbuilder/testdata/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1722,13 +1722,13 @@ upsert decimals
│ │ │ │ │ │ │ │ │ ├── columns: column1:7!null column2:8
│ │ │ │ │ │ │ │ │ └── (1.1, ARRAY[0.95])
│ │ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ │ └── 1.23 [as=c_default:9]
│ │ │ │ │ │ │ │ └── 1.23::DECIMAL(10,1) [as=c_default:9]
│ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ ├── crdb_internal.round_decimal_values(column1:7, 0) [as=a:10]
│ │ │ │ │ │ │ ├── crdb_internal.round_decimal_values(column2:8, 1) [as=b:11]
│ │ │ │ │ │ │ └── crdb_internal.round_decimal_values(c_default:9, 1) [as=c_default:12]
│ │ │ │ │ │ └── projections
│ │ │ │ │ │ └── a:10 + c_default:12 [as=d_comp:13]
│ │ │ │ │ │ └── (a:10 + c_default:12::DECIMAL)::DECIMAL(10,1) [as=d_comp:13]
│ │ │ │ │ └── projections
│ │ │ │ │ └── crdb_internal.round_decimal_values(d_comp:13, 1) [as=d_comp:14]
│ │ │ │ └── aggregations
Expand All @@ -1742,11 +1742,11 @@ upsert decimals
│ │ │ │ ├── columns: decimals.a:15!null decimals.b:16 c:17 d:18 crdb_internal_mvcc_timestamp:19 tableoid:20
│ │ │ │ └── computed column expressions
│ │ │ │ └── d:18
│ │ │ │ └── decimals.a:15::DECIMAL + c:17::DECIMAL
│ │ │ │ └── (decimals.a:15::DECIMAL + c:17::DECIMAL)::DECIMAL(10,1)
│ │ │ └── filters
│ │ │ └── a:10 = decimals.a:15
│ │ └── projections
│ │ └── decimals.a:15::DECIMAL + c:17::DECIMAL [as=d_comp:21]
│ │ └── (decimals.a:15::DECIMAL + c:17::DECIMAL)::DECIMAL(10,1) [as=d_comp:21]
│ └── projections
│ ├── CASE WHEN decimals.a:15 IS NULL THEN a:10 ELSE decimals.a:15 END [as=upsert_a:22]
│ ├── CASE WHEN decimals.a:15 IS NULL THEN c_default:12 ELSE c:17 END [as=upsert_c:23]
Expand Down Expand Up @@ -1794,13 +1794,13 @@ upsert decimals
│ │ │ │ │ │ │ │ │ └── (1.1,)
│ │ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ │ ├── NULL::DECIMAL(5,1)[] [as=b_default:8]
│ │ │ │ │ │ │ │ └── 1.23 [as=c_default:9]
│ │ │ │ │ │ │ │ └── 1.23::DECIMAL(10,1) [as=c_default:9]
│ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ ├── crdb_internal.round_decimal_values(column1:7, 0) [as=a:10]
│ │ │ │ │ │ │ ├── crdb_internal.round_decimal_values(b_default:8, 1) [as=b_default:11]
│ │ │ │ │ │ │ └── crdb_internal.round_decimal_values(c_default:9, 1) [as=c_default:12]
│ │ │ │ │ │ └── projections
│ │ │ │ │ │ └── a:10 + c_default:12 [as=d_comp:13]
│ │ │ │ │ │ └── (a:10 + c_default:12::DECIMAL)::DECIMAL(10,1) [as=d_comp:13]
│ │ │ │ │ └── projections
│ │ │ │ │ └── crdb_internal.round_decimal_values(d_comp:13, 1) [as=d_comp:14]
│ │ │ │ └── aggregations
Expand All @@ -1814,11 +1814,11 @@ upsert decimals
│ │ │ │ ├── columns: decimals.a:15!null b:16 c:17 d:18 crdb_internal_mvcc_timestamp:19 tableoid:20
│ │ │ │ └── computed column expressions
│ │ │ │ └── d:18
│ │ │ │ └── decimals.a:15::DECIMAL + c:17::DECIMAL
│ │ │ │ └── (decimals.a:15::DECIMAL + c:17::DECIMAL)::DECIMAL(10,1)
│ │ │ └── filters
│ │ │ └── a:10 = decimals.a:15
│ │ └── projections
│ │ └── decimals.a:15::DECIMAL + c:17::DECIMAL [as=d_comp:21]
│ │ └── (decimals.a:15::DECIMAL + c:17::DECIMAL)::DECIMAL(10,1) [as=d_comp:21]
│ └── projections
│ ├── CASE WHEN decimals.a:15 IS NULL THEN a:10 ELSE decimals.a:15 END [as=upsert_a:22]
│ ├── CASE WHEN decimals.a:15 IS NULL THEN b_default:11 ELSE b:16 END [as=upsert_b:23]
Expand Down Expand Up @@ -1874,13 +1874,13 @@ upsert decimals
│ │ │ │ │ │ │ │ │ │ │ ├── columns: column1:7!null column2:8
│ │ │ │ │ │ │ │ │ │ │ └── (1.1, ARRAY[0.95])
│ │ │ │ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ │ │ │ └── 1.23 [as=c_default:9]
│ │ │ │ │ │ │ │ │ │ └── 1.23::DECIMAL(10,1) [as=c_default:9]
│ │ │ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ │ │ ├── crdb_internal.round_decimal_values(column1:7, 0) [as=a:10]
│ │ │ │ │ │ │ │ │ ├── crdb_internal.round_decimal_values(column2:8, 1) [as=b:11]
│ │ │ │ │ │ │ │ │ └── crdb_internal.round_decimal_values(c_default:9, 1) [as=c_default:12]
│ │ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ │ └── a:10 + c_default:12 [as=d_comp:13]
│ │ │ │ │ │ │ │ └── (a:10 + c_default:12::DECIMAL)::DECIMAL(10,1) [as=d_comp:13]
│ │ │ │ │ │ │ └── projections
│ │ │ │ │ │ │ └── crdb_internal.round_decimal_values(d_comp:13, 1) [as=d_comp:14]
│ │ │ │ │ │ └── aggregations
Expand All @@ -1894,15 +1894,15 @@ upsert decimals
│ │ │ │ │ │ ├── columns: decimals.a:15!null decimals.b:16 c:17 d:18 crdb_internal_mvcc_timestamp:19 tableoid:20
│ │ │ │ │ │ └── computed column expressions
│ │ │ │ │ │ └── d:18
│ │ │ │ │ │ └── decimals.a:15::DECIMAL + c:17::DECIMAL
│ │ │ │ │ │ └── (decimals.a:15::DECIMAL + c:17::DECIMAL)::DECIMAL(10,1)
│ │ │ │ │ └── filters
│ │ │ │ │ └── a:10 = decimals.a:15
│ │ │ │ └── projections
│ │ │ │ └── ARRAY[0.99] [as=b_new:21]
│ │ │ └── projections
│ │ │ └── crdb_internal.round_decimal_values(b_new:21, 1) [as=b_new:22]
│ │ └── projections
│ │ └── decimals.a:15::DECIMAL + c:17::DECIMAL [as=d_comp:23]
│ │ └── (decimals.a:15::DECIMAL + c:17::DECIMAL)::DECIMAL(10,1) [as=d_comp:23]
│ └── projections
│ ├── CASE WHEN decimals.a:15 IS NULL THEN a:10 ELSE decimals.a:15 END [as=upsert_a:24]
│ ├── CASE WHEN decimals.a:15 IS NULL THEN b:11 ELSE b_new:22 END [as=upsert_b:25]
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/xform/testdata/external/trading-mutation
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1602,7 +1602,7 @@ update cardsinfo [as=ci]
│ └── const-agg [as=q:37, outer=(37)]
│ └── q:37
└── projections
├── crdb_internal.round_decimal_values(buyprice:23::DECIMAL - discount:25::DECIMAL, 4) [as=discountbuyprice_comp:53, outer=(23,25), immutable]
├── crdb_internal.round_decimal_values((buyprice:23::DECIMAL - discount:25::DECIMAL)::DECIMAL(10,4), 4) [as=discountbuyprice_comp:53, outer=(23,25), immutable]
├── CAST(NULL AS STRING) [as=notes_default:50]
├── 0 [as=oldinventory_default:51]
└── COALESCE(sum_int:47, 0) [as=actualinventory_new:49, outer=(47)]

0 comments on commit 394c407

Please sign in to comment.