Skip to content

Commit 921d818

Browse files
craig[bot]rafiss
craig[bot]
and
rafiss
committed
Merge #152633
152633: sql/types: fix type serialization in mixed version state r=rafiss a=rafiss In 2fad508 we removed the downgradeType function and changed upgradeType to only overwrite the OID if it was not already set. However, this causes issues in mixed-version clusters, since the 25.3 nodes will always unconditionally overwrite the OID based on the value of the VisibleType field. To account for this, this patch updates the hardcoded types to have VisibleType set, so that nodes on the newer version will serialize the type with this field set (previously downgradeType would have done that work). With the field set, upgradeType does the correct thing in the mixed version state. informs #152629 fixes #152419 Release note: None Co-authored-by: Rafi Shamim <[email protected]>
2 parents fbc73a7 + 17a8a60 commit 921d818

File tree

18 files changed

+313
-181
lines changed

18 files changed

+313
-181
lines changed

‎pkg/cli/testdata/doctor/test_recreate_zipdir-json‎

Lines changed: 6 additions & 6 deletions
Large diffs are not rendered by default.

‎pkg/sql/catalog/bootstrap/testdata/testdata‎

Lines changed: 94 additions & 94 deletions
Large diffs are not rendered by default.

‎pkg/sql/crdb_internal_test.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,10 +328,6 @@ SELECT column_name, character_maximum_length, numeric_precision, numeric_precisi
328328
for i := range tableDesc.Columns {
329329
col := &tableDesc.Columns[i]
330330
if col.Name == "k" {
331-
// TODO(knz): post-2.2, visible types for integer types are gone.
332-
if col.Type.InternalType.VisibleType != 0 {
333-
t.Errorf("unexpected visible type: got %d, expected 0", col.Type.InternalType.VisibleType)
334-
}
335331
if col.Type.Width() != 64 {
336332
t.Errorf("unexpected width: got %d, expected 64", col.Type.Width())
337333
}

‎pkg/sql/logictest/testdata/logic_test/dependencies‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,25 @@ query ITITTBTB colnames
2121
SELECT * FROM crdb_internal.table_columns WHERE descriptor_name LIKE 'test_%' ORDER BY descriptor_id, column_id
2222
----
2323
descriptor_id descriptor_name column_id column_name column_type nullable default_expr hidden
24-
106 test_kv 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false false NULL false
25-
106 test_kv 2 v family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
24+
106 test_kv 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false false NULL false
25+
106 test_kv 2 v family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
2626
106 test_kv 3 w family:DecimalFamily width:0 precision:0 locale:"" visible_type:0 oid:1700 time_precision_is_set:false true NULL false
27-
107 test_kvr1 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false false NULL false
28-
108 test_kvr2 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
29-
108 test_kvr2 2 v family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
30-
108 test_kvr2 3 rowid family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false false unique_rowid() true
31-
109 test_kvr3 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
32-
109 test_kvr3 2 v family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
33-
109 test_kvr3 3 rowid family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false false unique_rowid() true
34-
110 test_kvi1 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false false NULL false
35-
111 test_kvi2 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false false NULL false
36-
111 test_kvi2 2 v family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
37-
112 test_v1 1 v family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
38-
113 test_v2 1 v family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
39-
114 test_uwi_parent 1 a family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
40-
114 test_uwi_parent 2 rowid family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false false unique_rowid() true
41-
115 test_uwi_child 1 a family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false true NULL false
42-
115 test_uwi_child 2 rowid family:IntFamily width:64 precision:0 locale:"" visible_type:0 oid:20 time_precision_is_set:false false unique_rowid() true
27+
107 test_kvr1 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false false NULL false
28+
108 test_kvr2 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
29+
108 test_kvr2 2 v family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
30+
108 test_kvr2 3 rowid family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false false unique_rowid() true
31+
109 test_kvr3 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
32+
109 test_kvr3 2 v family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
33+
109 test_kvr3 3 rowid family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false false unique_rowid() true
34+
110 test_kvi1 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false false NULL false
35+
111 test_kvi2 1 k family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false false NULL false
36+
111 test_kvi2 2 v family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
37+
112 test_v1 1 v family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
38+
113 test_v2 1 v family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
39+
114 test_uwi_parent 1 a family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
40+
114 test_uwi_parent 2 rowid family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false false unique_rowid() true
41+
115 test_uwi_child 1 a family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false true NULL false
42+
115 test_uwi_child 2 rowid family:IntFamily width:64 precision:0 locale:"" visible_type:3 oid:20 time_precision_is_set:false false unique_rowid() true
4343

4444
query ITITTBBBI colnames
4545
SELECT descriptor_id, descriptor_name, index_id, index_name, index_type, is_unique, is_inverted, is_sharded, shard_bucket_count

‎pkg/sql/logictest/testdata/logic_test/mixed_version_char‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# LogicTest: cockroach-go-testserver-25.2
2+
3+
# This test verifies that the "char" type is handled correctly in a mixed
4+
# version cluster. The reason it's being tested is because during 25.4
5+
# development we began removing the legacy code for serializing types in the
6+
# pre-19.1 format. Doing this correctly required preserving some protobuf fields
7+
# until we drop support for 25.3, which is the last version that writes types in
8+
# the pre-19.1 format.
9+
# See https://github.com/cockroachdb/cockroach/issues/152629.
10+
11+
statement ok
12+
create table t (a "char"[], b "char", family (rowid,a,b));
13+
14+
statement ok
15+
insert into t values (ARRAY['cat':::STRING, '':::STRING], 'a');
16+
17+
upgrade 1
18+
19+
user root nodeidx=1
20+
21+
statement ok
22+
ALTER TABLE t ADD COLUMN c INT
23+
24+
user root nodeidx=2
25+
26+
statement ok
27+
insert into t values (ARRAY['abc'::STRING, ''::STRING], 'cat'::STRING);

‎pkg/sql/logictest/tests/cockroach-go-testserver-25.2/BUILD.bazel‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ go_test(
1111
"//pkg/sql/logictest:testdata", # keep
1212
],
1313
exec_properties = {"test.Pool": "heavy"},
14-
shard_count = 10,
14+
shard_count = 11,
1515
tags = ["cpu:3"],
1616
deps = [
1717
"//pkg/base",

‎pkg/sql/logictest/tests/cockroach-go-testserver-25.2/generated_test.go‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)