Skip to content

Commit

Permalink
Merge #36066
Browse files Browse the repository at this point in the history 
36066: opt: Round decimal values before check constraints r=andy-kimball a=andy-kimball

opt: Round decimal values before check constraints

The PG spec requires the following DECIMAL type behavior:

  If the scale of a value to be stored is greater than the declared scale
  of the column, the system will round the value to the specified number
  of fractional digits.

This is currently happening at the very end of the mutation operator, after
any check constraints. This commit moves the rounding to occur before check
constraints. Rounding must be performed on inserted and updated values before
computed columns are evaluated as well, since computed columns should run on
the final values to be inserted/updated.

Fixes #35364

Release note (sql change): Computed columns are now evaluated after rounding
any decimal values in input columns. Previously, computed columns used input
values before rounding.

Co-authored-by: Andrew Kimball <[email protected]>
  • Loading branch information
@andy-kimball
craig[bot] and andy-kimball committed Mar 25, 2019
2 parents 43c375d + 95030f2 commit 96dc666
Show file tree
Hide file tree
Showing 23 changed files with 1,180 additions and 245 deletions.
4 changes: 4 additions & 0 deletions docs/generated/sql/functions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,10 @@ may increase either contention or retry errors, or both.</p>
</span></td></tr>
<tr><td><code>crdb_internal.pretty_key(raw_key: <a href="bytes.html">bytes</a>, skip_fields: <a href="int.html">int</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>This function is used only by CockroachDB’s developers for testing purposes.</p>
</span></td></tr>
<tr><td><code>crdb_internal.round_decimal_values(val: <a href="decimal.html">decimal</a>, scale: <a href="int.html">int</a>) &rarr; <a href="decimal.html">decimal</a></code></td><td><span class="funcdesc"><p>This function is used internally to round decimal values during mutations.</p>
</span></td></tr>
<tr><td><code>crdb_internal.round_decimal_values(val: <a href="decimal.html">decimal</a>[], scale: <a href="int.html">int</a>) &rarr; <a href="decimal.html">decimal</a>[]</code></td><td><span class="funcdesc"><p>This function is used internally to round decimal array values during mutations.</p>
</span></td></tr>
<tr><td><code>crdb_internal.set_vmodule(vmodule_string: <a href="string.html">string</a>) &rarr; <a href="int.html">int</a></code></td><td><span class="funcdesc"><p>Set the equivalent of the <code>--vmodule</code> flag on the gateway node processing this request; it affords control over the logging verbosity of different files. Example syntax: <code>crdb_internal.set_vmodule('recordio=2,file=1,gfs*=3')</code>. Reset with: <code>crdb_internal.set_vmodule('')</code>. Raising the verbosity can severely affect performance.</p>
</span></td></tr>
<tr><td><code>current_database() &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the current database.</p>
Expand Down
19 changes: 19 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/insert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,3 +686,22 @@ INSERT INTO t35611 (a) VALUES (1)

statement ok
COMMIT

# ------------------------------------------------------------------------------
# Regression for #35364.
# ------------------------------------------------------------------------------
subtest regression_35364

statement ok
CREATE TABLE t35364(x DECIMAL(1,0) CHECK (x = 0))

statement ok
INSERT INTO t35364(x) VALUES (0.1)

query T
SELECT x FROM t35364
----
0

statement ok
DROP TABLE t35364
19 changes: 19 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/update
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,3 +527,22 @@ query II
SELECT * FROM t32054
----
NULL NULL

# ------------------------------------------------------------------------------
# Regression for #35364.
# ------------------------------------------------------------------------------
subtest regression_35364

statement ok
CREATE TABLE t35364(x DECIMAL(1,0) CHECK (x >= 1))

statement ok
INSERT INTO t35364 VALUES (1)

statement ok
UPDATE t35364 SET x=0.5

query T
SELECT x FROM t35364
----
1
66 changes: 66 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -868,3 +868,69 @@ INSERT INTO test35040(a,b) VALUES (0,1) ON CONFLICT(a) DO UPDATE SET c = 1111111

statement ok
DROP TABLE test35040

# ------------------------------------------------------------------------------
# Regression for #35364.
# ------------------------------------------------------------------------------
subtest regression_35364

statement ok
CREATE TABLE t35364(x INT PRIMARY KEY, y DECIMAL(10,1) CHECK(y >= 8.0), UNIQUE INDEX (y))

statement ok
INSERT INTO t35364(x, y) VALUES (1, 10.2)

# 10.18 should be mapped to 10.2 before the left outer join so that the conflict
# can be detected, and 7.95 should be mapped to 8.0 so that check constraint
# will pass.
statement ok
INSERT INTO t35364(x, y) VALUES (2, 10.18) ON CONFLICT (y) DO UPDATE SET y=7.95

query IT
SELECT * FROM t35364
----
1 8.0

statement ok
DROP TABLE t35364

# Check UPSERT syntax.
statement ok
CREATE TABLE t35364(
x DECIMAL(10,0) CHECK (x >= 0) PRIMARY KEY,
y DECIMAL(10,0) CHECK (y >= 0)
)

statement ok
UPSERT INTO t35364 (x) VALUES (-0.1)

query TT
SELECT * FROM t35364
----
-0 NULL

statement ok
UPSERT INTO t35364 (x, y) VALUES (-0.2, -0.3)

query TT
SELECT * FROM t35364
----
-0 -0

statement ok
UPSERT INTO t35364 (x, y) VALUES (1.5, 2.5)

query TT rowsort
SELECT * FROM t35364
----
-0 -0
2 3

statement ok
INSERT INTO t35364 (x) VALUES (1.5) ON CONFLICT (x) DO UPDATE SET x=2.5, y=3.5

query TT rowsort
SELECT * FROM t35364
----
-0 -0
3 4
20 changes: 20 additions & 0 deletions pkg/sql/opt/cat/column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,26 @@ type Column interface {
// DatumType returns the data type of the column.
DatumType() types.T

// ColTypePrecision returns the precision of the column's SQL data type. This
// is only defined for the Decimal data type and represents the max number of
// decimal digits in the decimal (including fractional digits). If precision
// is 0, then the decimal has no max precision.
ColTypePrecision() int

// ColTypeWidth returns the width of the column's SQL data type. This has
// different meanings depending on the data type:
//
// Decimal : scale
// Int : # bits (16, 32, 64, etc)
// Bit Array: # bits
// String : rune count
//
// TODO(andyk): It'd be better to expose the attributes of the column type
// using a different type or interface. However, currently that's hard to do,
// since using sqlbase.ColumnType creates an import cycle, and there's no good
// way to create a coltypes.T from sqlbase.ColumnType.
ColTypeWidth() int

// ColTypeStr returns the SQL data type of the column, as a string. Note that
// this is sometimes different than DatumType().String(), since datum types
// are a subset of column types.
Expand Down
18 changes: 18 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/insert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,3 +494,21 @@ render · · (z)
└── scan · · (a, b, c) +c
· table abc@abc_c_idx · ·
· spans ALL · ·

# ------------------------------------------------------------------------------
# Regression for #35364. This tests behavior that is different between the CBO
# and the HP. The CBO will (deliberately) round any input columns *before*
# evaluating any computed columns, as well as rounding the output.
# ------------------------------------------------------------------------------

statement ok
CREATE TABLE t35364(
x DECIMAL(10,0) CHECK(round(x) = x) PRIMARY KEY,
y DECIMAL(10,0) DEFAULT (1.5),
z DECIMAL(10,0) AS (x+y+2.5) STORED CHECK(z >= 7)
)

query TTT
INSERT INTO t35364 (x) VALUES (1.5) RETURNING *
----
2 2 7
23 changes: 23 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/update
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,3 +322,26 @@ render · · (a) +
└── scan · · (a, b, c, rowid[hidden]) +c
· table abc@abc_c_idx · ·
· spans ALL · ·

# ------------------------------------------------------------------------------
# Regression for #35364. This tests behavior that is different between the CBO
# and the HP. The CBO will (deliberately) round any input columns *before*
# evaluating any computed columns, as well as rounding the output.
# ------------------------------------------------------------------------------

statement ok
CREATE TABLE t35364(
x DECIMAL(10,0) CHECK(round(x) = x) PRIMARY KEY,
y DECIMAL(10,0) DEFAULT (1.5),
z DECIMAL(10,0) AS (x+y+2.5) STORED CHECK(z >= 7)
)

query TTT
INSERT INTO t35364 (x) VALUES (1.5) RETURNING *
----
2 2 7

query TTT
UPDATE t35364 SET x=2.5 RETURNING *
----
3 2 8
31 changes: 31 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,3 +342,34 @@ render · · (z)
└── scan · · (a, b, c) +c
· table abc@abc_c_idx · ·
· spans ALL · ·

# ------------------------------------------------------------------------------
# Regression for #35364. This tests behavior that is different between the CBO
# and the HP. The CBO will (deliberately) round any input columns *before*
# evaluating any computed columns, as well as rounding the output.
# ------------------------------------------------------------------------------

statement ok
CREATE TABLE t35364(
x DECIMAL(10,0) CHECK(round(x) = x) PRIMARY KEY,
y DECIMAL(10,0) DEFAULT (1.5),
z DECIMAL(10,0) AS (x+y+2.5) STORED CHECK(z >= 7)
)

query TTT
UPSERT INTO t35364 (x) VALUES (1.5) RETURNING *
----
2 2 7

query TTT
UPSERT INTO t35364 (x, y) VALUES (1.5, 2.5) RETURNING *
----
2 3 8

query TTT
INSERT INTO t35364 (x) VALUES (1.5) ON CONFLICT (x) DO UPDATE SET x=2.5 RETURNING *
----
3 3 9

statement error pq: failed to satisfy CHECK constraint \(z >= 7\)
UPSERT INTO t35364 (x) VALUES (0)
12 changes: 6 additions & 6 deletions pkg/sql/opt/memo/testdata/logprops/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ project
│ └── upsert_rowid:20 => rowid:4
├── side-effects, mutations
└── project
├── columns: upsert_a:17(int) upsert_b:18(int) upsert_c:19(int) upsert_rowid:20(int) x:5(int!null) y:6(int!null) column8:8(int) column9:9(int) a:10(int) b:11(int) c:12(int) rowid:13(int)
├── columns: upsert_a:17(int) upsert_b:18(int) upsert_c:19(int) upsert_rowid:20(int) x:5(int!null) y:6(int!null) column8:8(int) column9:9(int) a:10(int) b:11(int) c:12(int) rowid:13(int) column14:14(int!null) column15:15(int) column16:16(int)
├── side-effects
├── key: (5,13)
├── fd: ()-->(6,9), (5)-->(8), (13)-->(10-12), (10)-->(11-13), (11,12)~~>(10,13), (5,13)-->(17-19), (8,13)-->(20)
├── prune: (5,6,8-13,17-20)
├── fd: ()-->(6,9,14), (5)-->(8), (13)-->(10-12), (10)-->(11-13), (11,12)~~>(10,13), (12)-->(15), (15)-->(16), (5,13)-->(17), (13,15)-->(18), (13,16)-->(19), (8,13)-->(20)
├── prune: (5,6,8-20)
├── interesting orderings: (+5) (+6) (+13) (+10) (+11,+12,+13)
├── project
│ ├── columns: column16:16(int) x:5(int!null) y:6(int!null) column8:8(int) column9:9(int) a:10(int) b:11(int) c:12(int) rowid:13(int) column14:14(int!null) column15:15(int)
Expand Down Expand Up @@ -400,11 +400,11 @@ project
│ ├── cardinality: [2 - ]
│ ├── side-effects, mutations
│ └── project
│ ├── columns: upsert_b:14(int) upsert_c:15(int) upsert_rowid:16(int) column1:5(int) column6:6(int!null) column7:7(int) column8:8(int) a:9(int) b:10(int) c:11(int) rowid:12(int)
│ ├── columns: upsert_b:14(int) upsert_c:15(int) upsert_rowid:16(int) column1:5(int) column6:6(int!null) column7:7(int) column8:8(int) a:9(int) b:10(int) c:11(int) rowid:12(int) column13:13(int)
│ ├── cardinality: [2 - ]
│ ├── side-effects
│ ├── fd: ()-->(6,8), (12)-->(9-11), (9)-->(10-12), (10,11)~~>(9,12), (10,12)-->(14), (7,12)-->(16)
│ ├── prune: (5-12,14-16)
│ ├── fd: ()-->(6,8), (12)-->(9-11), (9)-->(10-12), (10,11)~~>(9,12), (10)-->(13), (10,12)-->(14), (12,13)-->(15), (7,12)-->(16)
│ ├── prune: (5-16)
│ ├── interesting orderings: (+12) (+9) (+10,+11,+12)
│ ├── project
│ │ ├── columns: column13:13(int) column1:5(int) column6:6(int!null) column7:7(int) column8:8(int) a:9(int) b:10(int) c:11(int) rowid:12(int)
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/memo/testdata/stats/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ select
│ ├── side-effects, mutations
│ ├── stats: [rows=200, distinct(1)=181.351171, null(1)=0, distinct(2)=200, null(2)=0]
│ └── project
│ ├── columns: upsert_x:13(string) upsert_y:14(int) upsert_z:15(float) a:4(int!null) b:5(string!null) column8:8(float) x:9(string) y:10(int) z:11(float)
│ ├── columns: upsert_x:13(string) upsert_y:14(int) upsert_z:15(float) a:4(int!null) b:5(string!null) column8:8(float) x:9(string) y:10(int) z:11(float) column12:12(int!null)
│ ├── stats: [rows=200, distinct(13)=181.351171, null(13)=0, distinct(14)=200, null(14)=0]
│ ├── fd: ()-->(5,8), (9)-->(10,11), (9)-->(13), (4,9)-->(14)
│ ├── fd: ()-->(5,8,12), (9)-->(10,11), (9)-->(13), (4,9)-->(14)
│ ├── project
│ │ ├── columns: column12:12(int!null) a:4(int!null) b:5(string!null) column8:8(float) x:9(string) y:10(int) z:11(float)
│ │ ├── stats: [rows=200, distinct(5,9)=181.351171, null(5,9)=0, distinct(4,9,12)=200, null(4,9,12)=0]
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/opt/norm/testdata/rules/prune_cols
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1845,9 +1845,9 @@ upsert a
└── projections
└── CASE WHEN k IS NULL THEN column7 ELSE i + 1 END [type=int, outer=(7,9,10)]

# No pruning when RETURNING clause is present.
# TODO(andyk): Need to prune output columns.
opt expect-not=(PruneMutationFetchCols,PruneMutationInputCols)
# Prune update columns replaced by upsert columns.
# TODO(andyk): Need to also prune output columns.
opt expect=PruneMutationInputCols expect-not=PruneMutationFetchCols
INSERT INTO a (k, s) VALUES (1, 'foo') ON CONFLICT (k) DO UPDATE SET i=a.i+1 RETURNING *
----
upsert a
Expand Down
Loading

0 comments on commit 96dc666

Please sign in to comment.