Skip to content

Commit

Permalink
opt: fix BPCHAR type and CASE typing
Browse files Browse the repository at this point in the history 
This commit addresses inconsistencies from Postgres' behavior. First, it
makes the `BPCHAR` type distinct from `CHAR`. The former is a
blank-padded character type with no type width, meaning that it could
have any length. The latter is a blank-padded character type with a type
width of exactly 1 - it is essentially an alias of `CHAR(1)`.
Previously, a column of type `BPCHAR` behaved the same as a column of
type `CHAR(1)` - it enforced a length limit of 1.

Second, the typing of `CASE` and `CASE`-like expressions has been fixed.
The branches of these conditionals is no longer forced to have the same
type-width.

Fixes cockroachdb#127889
Fixes cockroachdb#108360

Release note (bug fix): A bug has been fixed that caused incorrect
evaluation of `CASE`, `COALESCE`, and `IF` expressions with branches
producing fixed-width string-like types, such as `CHAR`. In addition,
the `BPCHAR` type has been fixed so that it no longer incorrectly
imposes a length limit of 1.
  • Loading branch information
@mgartner
mgartner committed Sep 5, 2024
1 parent 6c51566 commit 4a11c7d
Show file tree
Hide file tree
Showing 30 changed files with 315 additions and 119 deletions.
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/local-read-committed/generated_test.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/local-repeatable-read/generated_test.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/bpchar
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
query T
SELECT 'foo'::BPCHAR
----
foo

statement ok
CREATE TABLE t (c BPCHAR PRIMARY KEY, FAMILY (c))

statement ok
INSERT INTO t VALUES ('foo'), ('ba'), ('c'), ('foobarbaz')

query T rowsort
SELECT c FROM t
----
foo
ba
c
foobarbaz

query T
SELECT create_statement FROM [SHOW CREATE TABLE t]
----
CREATE TABLE public.t (
c BPCHAR NOT NULL,
CONSTRAINT t_pkey PRIMARY KEY (c ASC),
FAMILY fam_0_c (c)
)
46 changes: 46 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/case
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,49 @@ SELECT CASE WHEN f = 0
FROM a;

subtest end

# Regression test for #127889. CASE-like expressions should not impose type
# widths of one branch on other branches.
subtest regression_127889

query T
SELECT CASE WHEN true THEN 'foo'::TEXT ELSE 'b'::CHAR END
----
foo

query T
SELECT COALESCE(NULL::CHAR, 'bar'::CHAR(2))
----
ba

query T
SELECT IF(false, 'foo'::CHAR, 'bar'::CHAR(2))
----
ba

query T
SELECT CASE WHEN false THEN 'b'::CHAR ELSE 'foo'::TEXT END
----
foo

query T
SELECT (CASE WHEN false THEN 'b'::CHAR ELSE 'foo'::TEXT END)::CHAR
----
f

query T
SELECT (CASE WHEN false THEN 'b'::CHAR ELSE 'foo'::TEXT END)::BPCHAR
----
foo

query R
SELECT CASE WHEN true THEN 1.2345::DECIMAL(5, 4) ELSE NULL::DECIMAL(10, 2) END
----
1.2345

query R
SELECT CASE WHEN false THEN NULL::DECIMAL(10, 2) ELSE 1.2345::DECIMAL(5, 4) END
----
1.2345

subtest end
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/cast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@ CREATE TABLE def_assn_cast (
id INT4,
a INT4 DEFAULT 1.0::FLOAT4,
b VARCHAR DEFAULT 'true'::BOOL,
c NAME DEFAULT 'foo'::BPCHAR
c NAME DEFAULT 'foo'::CHAR
)

statement ok
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/decimal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ NaN NaN NaN NaN NaN
# TODO(#115679): There are a few differences vs postgres in the number of
# decimal places and negative zeros.
query RRRRR
WITH v(id, x) AS (VALUES (1, '0'::numeric), (2, '1'::numeric), (3, '-1'::numeric),
WITH v(id, x) AS (VALUES (1, '0'::numeric), (2, '1'::numeric), (3, '-1'::numeric),
(4, '4.2'::numeric), (5, 'inf'::numeric), (6, '-inf'::numeric), (7, 'nan'::numeric)
)
SELECT x1, x2,
Expand Down
30 changes: 30 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/drop_function
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,3 +326,33 @@ DROP TABLE t1_with_b_2_ref;

statement ok
DROP FUNCTION f_b;

statement ok
CREATE FUNCTION f_char(c CHAR) RETURNS INT LANGUAGE SQL AS 'SELECT 1'

statement ok
DROP FUNCTION f_char(BPCHAR)

statement ok
CREATE FUNCTION f_char(c CHAR(2)) RETURNS INT LANGUAGE SQL AS 'SELECT 1'

statement ok
DROP FUNCTION f_char(BPCHAR)

statement ok
CREATE FUNCTION f_char(c BPCHAR) RETURNS INT LANGUAGE SQL AS 'SELECT 1'

statement ok
DROP FUNCTION f_char(BPCHAR)

statement ok
CREATE FUNCTION f_char(c BPCHAR) RETURNS INT LANGUAGE SQL AS 'SELECT 1'

statement ok
DROP FUNCTION f_char(CHAR)

statement ok
CREATE FUNCTION f_char(c BPCHAR) RETURNS INT LANGUAGE SQL AS 'SELECT 1'

statement ok
DROP FUNCTION f_char(CHAR(2))
62 changes: 31 additions & 31 deletions pkg/sql/logictest/testdata/logic_test/grant_table
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,18 +250,18 @@ test pg_catalog box2d
test pg_catalog box2d[] type admin ALL false
test pg_catalog box2d[] type public USAGE false
test pg_catalog box2d[] type root ALL false
test pg_catalog bpchar type admin ALL false
test pg_catalog bpchar type public USAGE false
test pg_catalog bpchar type root ALL false
test pg_catalog bpchar[] type admin ALL false
test pg_catalog bpchar[] type public USAGE false
test pg_catalog bpchar[] type root ALL false
test pg_catalog bytes type admin ALL false
test pg_catalog bytes type public USAGE false
test pg_catalog bytes type root ALL false
test pg_catalog bytes[] type admin ALL false
test pg_catalog bytes[] type public USAGE false
test pg_catalog bytes[] type root ALL false
test pg_catalog char type admin ALL false
test pg_catalog char type public USAGE false
test pg_catalog char type root ALL false
test pg_catalog char[] type admin ALL false
test pg_catalog char[] type public USAGE false
test pg_catalog char[] type root ALL false
test pg_catalog date type admin ALL false
test pg_catalog date type public USAGE false
test pg_catalog date type root ALL false
Expand Down Expand Up @@ -651,14 +651,14 @@ test pg_catalog box2d type admin ALL
test pg_catalog box2d type root ALL false
test pg_catalog box2d[] type admin ALL false
test pg_catalog box2d[] type root ALL false
test pg_catalog bpchar type admin ALL false
test pg_catalog bpchar type root ALL false
test pg_catalog bpchar[] type admin ALL false
test pg_catalog bpchar[] type root ALL false
test pg_catalog bytes type admin ALL false
test pg_catalog bytes type root ALL false
test pg_catalog bytes[] type admin ALL false
test pg_catalog bytes[] type root ALL false
test pg_catalog char type admin ALL false
test pg_catalog char type root ALL false
test pg_catalog char[] type admin ALL false
test pg_catalog char[] type root ALL false
test pg_catalog date type admin ALL false
test pg_catalog date type root ALL false
test pg_catalog date[] type admin ALL false
Expand Down Expand Up @@ -810,9 +810,9 @@ test pg_catalog vector[] type root ALL
test pg_catalog void type admin ALL false
test pg_catalog void type root ALL false
test public NULL schema admin ALL true
test public NULL schema root ALL true
test public NULL schema public CREATE false
test public NULL schema public USAGE false
test public NULL schema root ALL true

# With no database set, we show the grants everywhere
statement ok
Expand Down Expand Up @@ -1279,14 +1279,14 @@ a pg_catalog box2d type admin
a pg_catalog box2d type root ALL false
a pg_catalog box2d[] type admin ALL false
a pg_catalog box2d[] type root ALL false
a pg_catalog bpchar type admin ALL false
a pg_catalog bpchar type root ALL false
a pg_catalog bpchar[] type admin ALL false
a pg_catalog bpchar[] type root ALL false
a pg_catalog bytes type admin ALL false
a pg_catalog bytes type root ALL false
a pg_catalog bytes[] type admin ALL false
a pg_catalog bytes[] type root ALL false
a pg_catalog char type admin ALL false
a pg_catalog char type root ALL false
a pg_catalog char[] type admin ALL false
a pg_catalog char[] type root ALL false
a pg_catalog date type admin ALL false
a pg_catalog date type root ALL false
a pg_catalog date[] type admin ALL false
Expand Down Expand Up @@ -1463,14 +1463,14 @@ defaultdb pg_catalog box2d type admin
defaultdb pg_catalog box2d type root ALL false
defaultdb pg_catalog box2d[] type admin ALL false
defaultdb pg_catalog box2d[] type root ALL false
defaultdb pg_catalog bpchar type admin ALL false
defaultdb pg_catalog bpchar type root ALL false
defaultdb pg_catalog bpchar[] type admin ALL false
defaultdb pg_catalog bpchar[] type root ALL false
defaultdb pg_catalog bytes type admin ALL false
defaultdb pg_catalog bytes type root ALL false
defaultdb pg_catalog bytes[] type admin ALL false
defaultdb pg_catalog bytes[] type root ALL false
defaultdb pg_catalog char type admin ALL false
defaultdb pg_catalog char type root ALL false
defaultdb pg_catalog char[] type admin ALL false
defaultdb pg_catalog char[] type root ALL false
defaultdb pg_catalog date type admin ALL false
defaultdb pg_catalog date type root ALL false
defaultdb pg_catalog date[] type admin ALL false
Expand Down Expand Up @@ -1647,14 +1647,14 @@ postgres pg_catalog box2d type admin
postgres pg_catalog box2d type root ALL false
postgres pg_catalog box2d[] type admin ALL false
postgres pg_catalog box2d[] type root ALL false
postgres pg_catalog bpchar type admin ALL false
postgres pg_catalog bpchar type root ALL false
postgres pg_catalog bpchar[] type admin ALL false
postgres pg_catalog bpchar[] type root ALL false
postgres pg_catalog bytes type admin ALL false
postgres pg_catalog bytes type root ALL false
postgres pg_catalog bytes[] type admin ALL false
postgres pg_catalog bytes[] type root ALL false
postgres pg_catalog char type admin ALL false
postgres pg_catalog char type root ALL false
postgres pg_catalog char[] type admin ALL false
postgres pg_catalog char[] type root ALL false
postgres pg_catalog date type admin ALL false
postgres pg_catalog date type root ALL false
postgres pg_catalog date[] type admin ALL false
Expand Down Expand Up @@ -1831,14 +1831,14 @@ system pg_catalog box2d type admin
system pg_catalog box2d type root ALL false
system pg_catalog box2d[] type admin ALL false
system pg_catalog box2d[] type root ALL false
system pg_catalog bpchar type admin ALL false
system pg_catalog bpchar type root ALL false
system pg_catalog bpchar[] type admin ALL false
system pg_catalog bpchar[] type root ALL false
system pg_catalog bytes type admin ALL false
system pg_catalog bytes type root ALL false
system pg_catalog bytes[] type admin ALL false
system pg_catalog bytes[] type root ALL false
system pg_catalog char type admin ALL false
system pg_catalog char type root ALL false
system pg_catalog char[] type admin ALL false
system pg_catalog char[] type root ALL false
system pg_catalog date type admin ALL false
system pg_catalog date type root ALL false
system pg_catalog date[] type admin ALL false
Expand Down Expand Up @@ -2404,14 +2404,14 @@ test pg_catalog box2d type admin
test pg_catalog box2d type root ALL false
test pg_catalog box2d[] type admin ALL false
test pg_catalog box2d[] type root ALL false
test pg_catalog bpchar type admin ALL false
test pg_catalog bpchar type root ALL false
test pg_catalog bpchar[] type admin ALL false
test pg_catalog bpchar[] type root ALL false
test pg_catalog bytes type admin ALL false
test pg_catalog bytes type root ALL false
test pg_catalog bytes[] type admin ALL false
test pg_catalog bytes[] type root ALL false
test pg_catalog char type admin ALL false
test pg_catalog char type root ALL false
test pg_catalog char[] type admin ALL false
test pg_catalog char[] type root ALL false
test pg_catalog date type admin ALL false
test pg_catalog date type root ALL false
test pg_catalog date[] type admin ALL false
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/pg_builtins
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ ORDER BY t.oid
----
text 25 -1
float8 701 -1
bpchar 1042 5
bpchar 1042 -1
varchar 1043 68
bit 1560 1
varbit 1562 16
Expand All @@ -539,7 +539,7 @@ ORDER BY t.oid
----
text NULL
float8 NULL
bpchar 1
bpchar NULL
varchar 64
bit 1
varbit 16
Expand All @@ -558,7 +558,7 @@ ORDER BY t.oid
----
text NULL
float8 NULL
bpchar 1
bpchar NULL
varchar 64
bit 1
varbit 16
Expand Down
Loading

0 comments on commit 4a11c7d

Please sign in to comment.