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 Oct 11, 2024
1 parent 965cf7a commit f3de707
Show file tree
Hide file tree
Showing 27 changed files with 305 additions and 117 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.

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)
)
47 changes: 47 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/case
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# LogicTest: local

# 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(drewk): 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 @@ -250,3 +250,33 @@ statement error pgcode 42883 unknown function: f114677\(\)
SHOW CREATE FUNCTION f114677;

subtest end

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))
60 changes: 30 additions & 30 deletions pkg/sql/logictest/testdata/logic_test/grant_table
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,18 +243,18 @@ test pg_catalog box2d type
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 @@ -635,14 +635,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 @@ -1239,14 +1239,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 @@ -1415,14 +1415,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 @@ -1591,14 +1591,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 @@ -1767,14 +1767,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 @@ -2315,14 +2315,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
6 changes: 1 addition & 5 deletions pkg/sql/logictest/testdata/logic_test/typing
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,15 +289,12 @@ WHERE t102110_1.t NOT BETWEEN t102110_1.t AND
(CASE WHEN NULL THEN t102110_2.c ELSE t102110_1.t END);
----

# IF is typed differently (Postgres does not support IF).
query T
SELECT t102110_1.t FROM t102110_1, t102110_2
WHERE t102110_1.t NOT BETWEEN t102110_1.t AND
IF(NULL, t102110_2.c, t102110_1.t);
----
tt

# TODO(#108360): implicit casts from STRING to CHAR should use bpchar.
statement ok
CREATE TABLE t108360_1 (t TEXT);
INSERT INTO t108360_1 VALUES ('tt');
Expand All @@ -306,10 +303,9 @@ statement ok
CREATE TABLE t108360_2 (c CHAR);
INSERT INTO t108360_2 VALUES ('c');

# This returns one row with 'tt' in Postgres. The results are different because
# Postgres casts t108360_1.t to bpchar rather than char.
query T
SELECT (CASE WHEN t108360_1.t > t108360_2.c THEN t108360_1.t ELSE t108360_2.c END)
FROM t108360_1, t108360_2
WHERE t108360_1.t = (CASE WHEN t108360_1.t > t108360_2.c THEN t108360_1.t ELSE t108360_2.c END);
----
tt
7 changes: 7 additions & 0 deletions pkg/sql/logictest/tests/fakedist-disk/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/sql/logictest/tests/fakedist-vec-off/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/sql/logictest/tests/fakedist/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/sql/logictest/tests/local-legacy-schema-changer/generated_test.go
View file

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

Loading

0 comments on commit f3de707

Please sign in to comment.