Skip to content

Commit

Permalink
opt: eliminate assignment casts with identical source and target types
Browse files Browse the repository at this point in the history
The `EliminateAssignmentCast` rule has been combined with the
`EliminateCast` rule. Now an assignment cast is eliminated if the source
and target type are identical. This now possible thanks to some changes
to type resolution, including:

  1. Placeholder types are resolved with unspecified type modifiers.
     This ensures that assignment casts won't be eliminated if the a
     placeholder value does not conform to the target type's modifiers.

  2. When constant integer `NumVal`s are resolved as a typed-value,
     they are validated to ensure they fit within their type's width.
     There may be more types we need to perform similar validation for,
     such as floats (see #73743).

  3. Columns in values expressions with values that have non-identical
     types but the same type OID will be typed with type modifiers. For
     example, if a values expression has a CHAR(1) value and a CHAR(3)
     value in the same column, the column will be typed as a CHAR
     without an explicit width.

  4. Type modifiers are now correctly omitted from array content types
     when arrays contain constants.

Fixes #73450

Release note (bug fix): A bug has been fixed that caused incorrect
evaluation of placeholder values in EXECUTE statements. The bug
presented when the PREPARE statement cast a placeholder value, for
example `PREPARE s AS SELECT $1::INT2`. If the assigned value for `$1`
exceeded the maximum width value of the cast target type, the result
value of the cast could be incorrect. This bug has been present since
version 19.1 or earlier.
  • Loading branch information
mgartner committed Dec 16, 2021
1 parent 152ab0f commit eb2a6c9
Show file tree
Hide file tree
Showing 17 changed files with 224 additions and 195 deletions.
89 changes: 89 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/cast
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ CREATE TABLE assn_cast (
qc "char",
b BIT,
i INT,
i2 INT2,
f4 FLOAT4,
t timestamp,
d DECIMAL(10, 0),
a DECIMAL(10, 0)[],
Expand Down Expand Up @@ -141,6 +143,33 @@ EXECUTE insert_i('1')
statement error value type string doesn't match type int of column \"i\"
INSERT INTO assn_cast(i) VALUES ('1'::STRING)

statement error integer out of range for type int2
INSERT INTO assn_cast(i2) VALUES (999999999)

statement ok
PREPARE insert_i2 AS INSERT INTO assn_cast(i2) VALUES ($1)

statement error integer out of range for type int2
EXECUTE insert_i2(99999999)

statement ok
INSERT INTO assn_cast(f4) VALUES (18754999.99)

statement ok
PREPARE insert_f4 AS INSERT INTO assn_cast(f4) VALUES ($1)

statement ok
EXECUTE insert_f4(18754999.99)

# TODO(mgartner): Values are not correctly truncated when cast to FLOAT4, either
# in an assignment or explicit context. The columns should have a value of
# 1.8755e+07. See #73743.
query F
SELECT f4 FROM assn_cast WHERE f4 IS NOT NULL
----
1.875499999e+07
1.875499999e+07

statement ok
INSERT INTO assn_cast(t) VALUES ('1970-01-01'::timestamptz)

Expand Down Expand Up @@ -177,6 +206,15 @@ SELECT d FROM assn_cast WHERE d IS NOT NULL
123
68

statement ok
INSERT INTO assn_cast(a) VALUES (ARRAY[])

statement ok
INSERT INTO assn_cast(a) VALUES (ARRAY[NULL])

statement ok
INSERT INTO assn_cast(a) VALUES (ARRAY[1.1])

statement ok
INSERT INTO assn_cast(a) VALUES (ARRAY[2.88, NULL, 15])

Expand All @@ -192,13 +230,30 @@ PREPARE insert_a AS INSERT INTO assn_cast(a) VALUES ($1)
statement ok
EXECUTE insert_a(ARRAY[7.77, 8.88::DECIMAL(10, 2)])

statement ok
PREPARE insert_a2 AS INSERT INTO assn_cast(a) VALUES (ARRAY[$1])

statement ok
EXECUTE insert_a2(20.2)

statement ok
PREPARE insert_a3 AS INSERT INTO assn_cast(a) VALUES (ARRAY[30.12, $1, 32.1])

statement ok
EXECUTE insert_a3(30.9)

query T rowsort
SELECT a FROM assn_cast WHERE a IS NOT NULL
----
{}
{NULL}
{1}
{3,NULL,15}
{4,NULL,16}
{6,7}
{8,9}
{20}
{30,31,32}

statement ok
INSERT INTO assn_cast(s) VALUES (1)
Expand Down Expand Up @@ -350,3 +405,37 @@ SELECT '-'::regclass, '-'::regclass::oid,
'-'::regrole, '-'::regrole::oid
----
- 0 - 0 - 0 - 0 - 0 - 0

# Regression test for #73450. Eliding casts should not cause incorrect results.
subtest regression_73450

statement ok
PREPARE s73450_a AS SELECT $1::INT2

statement error integer out of range for type int2
EXECUTE s73450_a(999999)

statement ok
PREPARE s73450_b AS SELECT $1::CHAR

query T
EXECUTE s73450_b('foo')
----
f

statement ok
CREATE TABLE t73450 (c CHAR);
INSERT INTO t73450 VALUES ('f')

query T
SELECT * FROM t73450 WHERE c = 'foo'::CHAR
----
f

statement ok
PREPARE s73450_c AS SELECT * FROM t73450 WHERE c = $1::CHAR

query T
EXECUTE s73450_c('foo')
----
f
18 changes: 9 additions & 9 deletions pkg/sql/opt/exec/execbuilder/testdata/insert
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ vectorized: true
│ columns: (x, v, rowid_default)
│ estimated row count: 10 (missing stats)
│ render rowid_default: unique_rowid()
│ render x: crdb_internal.assignment_cast(x, NULL::INT8)
│ render v: crdb_internal.assignment_cast(v, NULL::INT8)
│ render x: x
│ render v: v
└── • top-k
│ columns: (x, v)
Expand Down Expand Up @@ -355,8 +355,8 @@ vectorized: true
│ columns: (x, v, rowid_default)
│ estimated row count: 1 (missing stats)
│ render rowid_default: unique_rowid()
│ render x: crdb_internal.assignment_cast(x, NULL::INT8)
│ render v: crdb_internal.assignment_cast(v, NULL::INT8)
│ render x: x
│ render v: v
└── • scan
columns: (x, v)
Expand Down Expand Up @@ -471,8 +471,8 @@ vectorized: true
│ columns: (length, "?column?", rowid_default)
│ estimated row count: 10 (missing stats)
│ render rowid_default: unique_rowid()
│ render length: crdb_internal.assignment_cast(length, NULL::INT8)
│ render ?column?: crdb_internal.assignment_cast("?column?", NULL::INT8)
│ render length: length
│ render ?column?: "?column?"
└── • top-k
│ columns: (length, "?column?", column12)
Expand Down Expand Up @@ -600,9 +600,9 @@ vectorized: true
│ columns: (a, b, c, rowid_default)
│ estimated row count: 1,000 (missing stats)
│ render rowid_default: unique_rowid()
│ render a: crdb_internal.assignment_cast(a, NULL::INT8)
│ render b: crdb_internal.assignment_cast(b, NULL::INT8)
│ render c: crdb_internal.assignment_cast(c, NULL::INT8)
│ render a: a
│ render b: b
│ render c: c
└── • scan
columns: (a, b, c)
Expand Down
24 changes: 8 additions & 16 deletions pkg/sql/opt/norm/rules/scalar.opt
Original file line number Diff line number Diff line change
Expand Up @@ -58,26 +58,18 @@ $item
=>
(SimplifyCoalesce $args)

# EliminateCast discards the cast operator if its input already has a type
# that's identical to the desired static type.
# EliminateCast discards a cast or assignment cast operator if its input already
# has a type that's identical to the desired static type.
#
# Note that CastExpr removes unnecessary casts during type-checking; this rule
# can still be helpful if some other rule creates an unnecessary CastExpr.
[EliminateCast, Normalize]
(Cast $input:* $targetTyp:* & (HasColType $input $targetTyp))
=>
$input

# EliminateAssignmentCast discards the assignment cast operator if the input
# type and the target type are identical and the assignment cast is guaranteed
# to be a no-op. See AssignmentCastIsNoop for an explanation of what makes an
# assignment cast a no-op.
[EliminateAssignmentCast, Normalize]
(AssignmentCast
#
# EliminateCast is marked as high-priority so that it matches before
# FoldAssignmentCast.
[EliminateCast, Normalize, HighPriority]
(Cast | AssignmentCast
$input:*
$targetTyp:* &
(HasColType $input $targetTyp) &
(AssignmentCastIsNoop $targetTyp)
$targetTyp:* & (HasColType $input $targetTyp)
)
=>
$input
Expand Down
28 changes: 0 additions & 28 deletions pkg/sql/opt/norm/scalar_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,31 +368,3 @@ func (c *CustomFuncs) SplitTupleEq(lhs, rhs *memo.TupleExpr) memo.FiltersExpr {
}
return res
}

// AssignmentCastIsNoop returns true if an assignment cast from t to t is
// guaranteed to be a no-op: it never alters the input value and never errors.
// These conditions must hold in the unintuitive, but crucial case where a value
// does not fit within the width of t.
//
// The width of a value may be wider than its type's width when executing
// prepared statements. For example, no width validation is performed when
// assigning a string to a placeholder that has been typed as a CHAR(1). An
// assignment cast from a value with more than one character, even if typed as a
// CHAR(1), to a CHAR(1) will result in an error, so it is not guaranteed to be
// a no-op.
//
// Currently, AssignmentCastIsNoop returns true only for the UUID type. UUIDs
// have no width modifier, and values assigned to UUID placeholders are
// validated, so an assignment cast from UUID to UUID is guaranteed to be a
// no-op.
//
// TODO(mgartner): There are other types for which assignment casts are
// guaranteed no-ops. We should expand this list.
func (c *CustomFuncs) AssignmentCastIsNoop(t *types.T) bool {
switch t.Family() {
case types.UuidFamily:
return true
default:
return false
}
}
2 changes: 1 addition & 1 deletion pkg/sql/opt/norm/testdata/rules/fold_constants
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ insert assn_cast

# Do not fold non-constants even if they have an identical type as the target
# type.
norm expect-not=FoldAssignmentCast
norm expect-not=FoldAssignmentCast disable=EliminateCast
INSERT INTO assn_cast(c) VALUES ($1::CHAR)
----
insert assn_cast
Expand Down
47 changes: 10 additions & 37 deletions pkg/sql/opt/norm/testdata/rules/groupby
Original file line number Diff line number Diff line change
Expand Up @@ -953,17 +953,14 @@ insert xy
└── limit
├── columns: y:9!null y_default:10
├── cardinality: [0 - 1]
├── immutable
├── key: ()
├── fd: ()-->(9,10)
├── anti-join (hash)
│ ├── columns: y:9!null y_default:10
│ ├── immutable
│ ├── fd: ()-->(9,10)
│ ├── limit hint: 1.00
│ ├── project
│ │ ├── columns: y_default:10 y:9!null
│ │ ├── immutable
│ │ ├── fd: ()-->(9,10)
│ │ ├── select
│ │ │ ├── columns: xy.y:6!null
Expand All @@ -974,8 +971,7 @@ insert xy
│ │ │ └── xy.y:6 = 0 [outer=(6), constraints=(/6: [/0 - /0]; tight), fd=()-->(6)]
│ │ └── projections
│ │ ├── CAST(NULL AS INT8) [as=y_default:10]
│ │ └── assignment-cast: INT8 [as=y:9, outer=(6), immutable]
│ │ └── xy.y:6
│ │ └── xy.y:6 [as=y:9, outer=(6)]
│ ├── scan xy
│ │ ├── columns: x:11!null
│ │ └── key: (11)
Expand Down Expand Up @@ -1056,16 +1052,13 @@ insert xy
└── upsert-distinct-on
├── columns: y:9 y_default:10
├── grouping columns: y:9
├── immutable
├── lax-key: (9)
├── fd: ()-->(9,10)
├── anti-join (hash)
│ ├── columns: y:9 y_default:10
│ ├── immutable
│ ├── fd: ()-->(9,10)
│ ├── project
│ │ ├── columns: y_default:10 y:9
│ │ ├── immutable
│ │ ├── fd: ()-->(9,10)
│ │ ├── select
│ │ │ ├── columns: xy.y:6
Expand All @@ -1076,8 +1069,7 @@ insert xy
│ │ │ └── xy.y:6 IS NULL [outer=(6), constraints=(/6: [/NULL - /NULL]; tight), fd=()-->(6)]
│ │ └── projections
│ │ ├── CAST(NULL AS INT8) [as=y_default:10]
│ │ └── assignment-cast: INT8 [as=y:9, outer=(6), immutable]
│ │ └── xy.y:6
│ │ └── xy.y:6 [as=y:9, outer=(6)]
│ ├── scan xy
│ │ ├── columns: x:11!null
│ │ └── key: (11)
Expand Down Expand Up @@ -1687,17 +1679,14 @@ insert a
└── limit
├── columns: "?column?":17!null i:18!null "?column?":19!null f_default:20 j_default:21
├── cardinality: [0 - 1]
├── immutable
├── key: ()
├── fd: ()-->(17-21)
├── anti-join (hash)
│ ├── columns: "?column?":17!null i:18!null "?column?":19!null f_default:20 j_default:21
│ ├── immutable
│ ├── fd: ()-->(17-21)
│ ├── limit hint: 1.00
│ ├── project
│ │ ├── columns: f_default:20 j_default:21 "?column?":17!null i:18!null "?column?":19!null
│ │ ├── immutable
│ │ ├── fd: ()-->(17-21)
│ │ ├── select
│ │ │ ├── columns: a.i:9!null
Expand All @@ -1710,8 +1699,7 @@ insert a
│ │ ├── CAST(NULL AS FLOAT8) [as=f_default:20]
│ │ ├── CAST(NULL AS JSONB) [as=j_default:21]
│ │ ├── 1 [as="?column?":17]
│ │ ├── assignment-cast: INT8 [as=i:18, outer=(9), immutable]
│ │ │ └── a.i:9
│ │ ├── a.i:9 [as=i:18, outer=(9)]
│ │ └── 'foo' [as="?column?":19]
│ ├── select
│ │ ├── columns: a.i:23!null s:25!null
Expand Down Expand Up @@ -2465,18 +2453,8 @@ insert a
│ │ │ │ │ ├── columns: column1:12 column2:13!null column3:14!null column4:15!null
│ │ │ │ │ ├── cardinality: [2 - 2]
│ │ │ │ │ ├── volatile
│ │ │ │ │ ├── tuple
│ │ │ │ │ │ ├── assignment-cast: INT8
│ │ │ │ │ │ │ └── unique_rowid()
│ │ │ │ │ │ ├── 'foo'
│ │ │ │ │ │ ├── 1
│ │ │ │ │ │ └── 1.0
│ │ │ │ │ └── tuple
│ │ │ │ │ ├── assignment-cast: INT8
│ │ │ │ │ │ └── unique_rowid()
│ │ │ │ │ ├── 'bar'
│ │ │ │ │ ├── 2
│ │ │ │ │ └── 2.0
│ │ │ │ │ ├── (unique_rowid(), 'foo', 1, 1.0)
│ │ │ │ │ └── (unique_rowid(), 'bar', 2, 2.0)
│ │ │ │ └── projections
│ │ │ │ └── CAST(NULL AS JSONB) [as=j_default:16]
│ │ │ ├── scan a
Expand Down Expand Up @@ -2526,27 +2504,22 @@ insert a
└── upsert-distinct-on
├── columns: i:16!null "?column?":17!null i:18!null f_default:19 j_default:20
├── grouping columns: i:18!null
├── immutable
├── key: (18)
├── fd: ()-->(17,19,20), (18)-->(16,17,19,20)
├── fd: ()-->(17,19,20), (16)==(18), (18)==(16), (18)-->(16,17,19,20)
├── anti-join (hash)
│ ├── columns: i:16!null "?column?":17!null i:18!null f_default:19 j_default:20
│ ├── immutable
│ ├── fd: ()-->(17,19,20)
│ ├── fd: ()-->(17,19,20), (16)==(18), (18)==(16)
│ ├── project
│ │ ├── columns: f_default:19 j_default:20 i:16!null "?column?":17!null i:18!null
│ │ ├── immutable
│ │ ├── fd: ()-->(17,19,20)
│ │ ├── fd: ()-->(17,19,20), (16)==(18), (18)==(16)
│ │ ├── scan a
│ │ │ └── columns: a.i:9!null
│ │ └── projections
│ │ ├── CAST(NULL AS FLOAT8) [as=f_default:19]
│ │ ├── CAST(NULL AS JSONB) [as=j_default:20]
│ │ ├── assignment-cast: INT8 [as=i:16, outer=(9), immutable]
│ │ │ └── a.i:9
│ │ ├── a.i:9 [as=i:16, outer=(9)]
│ │ ├── 'foo' [as="?column?":17]
│ │ └── assignment-cast: INT8 [as=i:18, outer=(9), immutable]
│ │ └── a.i:9
│ │ └── a.i:9 [as=i:18, outer=(9)]
│ ├── select
│ │ ├── columns: a.i:22!null s:24!null
│ │ ├── key: (22)
Expand Down
Loading

0 comments on commit eb2a6c9

Please sign in to comment.