From 394c4079294da8718291cdcea81074c855fbcd77 Mon Sep 17 00:00:00 2001 From: Tommy Reilly Date: Fri, 25 Jun 2021 11:46:01 -0400 Subject: [PATCH] sql: add ReType to resolveAndRequireType to fix vector engine panic 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 --- .../execbuilder/testdata/hash_sharded_index | 4 +- pkg/sql/opt/optbuilder/scope.go | 2 +- pkg/sql/opt/optbuilder/testdata/insert | 4 +- pkg/sql/opt/optbuilder/testdata/update | 4 +- .../optbuilder/testdata/update-col-cast-bug | 52 +++++++++++++++++++ pkg/sql/opt/optbuilder/testdata/upsert | 24 ++++----- .../xform/testdata/external/trading-mutation | 2 +- 7 files changed, 72 insertions(+), 20 deletions(-) create mode 100644 pkg/sql/opt/optbuilder/testdata/update-col-cast-bug diff --git a/pkg/sql/opt/exec/execbuilder/testdata/hash_sharded_index b/pkg/sql/opt/exec/execbuilder/testdata/hash_sharded_index index 05a9dd7457ed..35d39ff35bb0 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/hash_sharded_index +++ b/pkg/sql/opt/exec/execbuilder/testdata/hash_sharded_index @@ -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 @@ -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 │ diff --git a/pkg/sql/opt/optbuilder/scope.go b/pkg/sql/opt/optbuilder/scope.go index c0bcc1ab880e..787e12f2fdb6 100644 --- a/pkg/sql/opt/optbuilder/scope.go +++ b/pkg/sql/opt/optbuilder/scope.go @@ -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 diff --git a/pkg/sql/opt/optbuilder/testdata/insert b/pkg/sql/opt/optbuilder/testdata/insert index 90e9eecfc3ac..90cfd4b2d41e 100644 --- a/pkg/sql/opt/optbuilder/testdata/insert +++ b/pkg/sql/opt/optbuilder/testdata/insert @@ -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 diff --git a/pkg/sql/opt/optbuilder/testdata/update b/pkg/sql/opt/optbuilder/testdata/update index ce6d9c5dc1d9..46e189224a6d 100644 --- a/pkg/sql/opt/optbuilder/testdata/update +++ b/pkg/sql/opt/optbuilder/testdata/update @@ -1631,7 +1631,7 @@ 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] @@ -1639,7 +1639,7 @@ update decimals │ │ │ ├── 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 diff --git a/pkg/sql/opt/optbuilder/testdata/update-col-cast-bug b/pkg/sql/opt/optbuilder/testdata/update-col-cast-bug new file mode 100644 index 000000000000..86946a81acd1 --- /dev/null +++ b/pkg/sql/opt/optbuilder/testdata/update-col-cast-bug @@ -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: + ├── 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] diff --git a/pkg/sql/opt/optbuilder/testdata/upsert b/pkg/sql/opt/optbuilder/testdata/upsert index 260517bc6341..3160ec784153 100644 --- a/pkg/sql/opt/optbuilder/testdata/upsert +++ b/pkg/sql/opt/optbuilder/testdata/upsert @@ -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 @@ -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] @@ -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 @@ -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] @@ -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 @@ -1894,7 +1894,7 @@ 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 @@ -1902,7 +1902,7 @@ upsert decimals │ │ │ └── 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] diff --git a/pkg/sql/opt/xform/testdata/external/trading-mutation b/pkg/sql/opt/xform/testdata/external/trading-mutation index 95702925936d..264a3cd8fcd2 100644 --- a/pkg/sql/opt/xform/testdata/external/trading-mutation +++ b/pkg/sql/opt/xform/testdata/external/trading-mutation @@ -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)]