Skip to content

Commit

Permalink
sql: allow old enum value '1' for sql.defaults.vectorize
Browse files Browse the repository at this point in the history
Fixes: cockroachdb#133278

Release note (bug fix): This commit fixes a bug which causes new
connections to fail with the following error after upgrading to v24.2:

```
ERROR: invalid value for parameter "vectorize": "unknown(1)"
SQLSTATE: 22023
HINT: Available values: off,on,experimental_always
```

In order to hit this bug, the cluster must have:
1. been on version v21.1 at some point in the past
2. run `SET CLUSTER SETTING sql.defaults.vectorize = 'on';` on v21.1
3. not set sql.defaults.vectorize after upgrading past v21.1
4. upgraded all the way to v24.2

The conditions required for this bug can be detected using:

```
SELECT * FROM system.settings WHERE name = 'sql.defaults.vectorize';
```

If the value is '1', the following statement should be run to fix it
before upgrading to v24.2:

```
RESET CLUSTER SETTING sql.defaults.vectorize;
```

This commit fixes the bug by making '1' a legal value for
sql.defaults.vectorize again (mapping to 'on').
  • Loading branch information
michae2 committed Oct 23, 2024
1 parent b7b76a1 commit 42e3840
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 3 deletions.
6 changes: 5 additions & 1 deletion pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,11 @@ var VectorizeClusterMode = settings.RegisterEnumSetting(
func() map[sessiondatapb.VectorizeExecMode]string {
m := make(map[sessiondatapb.VectorizeExecMode]string, len(sessiondatapb.VectorizeExecMode_name))
for k := range sessiondatapb.VectorizeExecMode_name {
// Note that VectorizeExecMode.String() remaps "unset" to "on".
// Note that for historical reasons, VectorizeExecMode.String() remaps
// both "unset" and "201auto" to "on", so we end up with a map like:
// 0: on, 1: on, 2: on, 3: experimental_always, 4: off. This means that
// after SET CLUSTER SETTING sql.defaults.vectorize = 'on'; we could have
// 0, 1, or 2 in system.settings and must handle all three cases as 'on'.
m[sessiondatapb.VectorizeExecMode(k)] = sessiondatapb.VectorizeExecMode(k).String()
}
return m
Expand Down
54 changes: 54 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/system
Original file line number Diff line number Diff line change
Expand Up @@ -1428,3 +1428,57 @@ ALTER TABLE system.public.tenant_usage CONFIGURE ZONE USING
num_replicas = 5,
constraints = '[]',
lease_preferences = '[]'

# Regression test for 133278, that clusters with sql.defaults.vectorize set to
# old value '1' can still start new connections.

statement ok
UPSERT INTO system.settings (name, value, "valueType") VALUES ('sql.defaults.vectorize', '1', 'e')

query TT
SELECT name, value FROM system.settings WHERE name = 'sql.defaults.vectorize'
----
sql.defaults.vectorize 1

query T
SHOW CLUSTER SETTING sql.defaults.vectorize
----
on

statement ok
SET vectorize = DEFAULT

query T
SHOW vectorize
----
on

# Make sure we can open a new connection.
user testuser newsession

user root

statement ok
RESET CLUSTER SETTING sql.defaults.vectorize

statement ok
RESET vectorize

query TT
SELECT name, value FROM system.settings WHERE name = 'sql.defaults.vectorize'
----

query T
SHOW CLUSTER SETTING sql.defaults.vectorize
----
on

query T
SHOW vectorize
----
on

user testuser

statement ok
RESET vectorize
5 changes: 4 additions & 1 deletion pkg/sql/sessiondatapb/session_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (c DataConversionConfig) GetFloatPrec(typ *types.T) int {
}

func (m VectorizeExecMode) String() string {
if m == VectorizeUnset {
if m == VectorizeUnset || m == DeprecatedVectorize201Auto {
m = VectorizeOn
}
name, ok := VectorizeExecMode_name[int32(m)]
Expand All @@ -72,6 +72,9 @@ func VectorizeExecModeFromString(val string) (VectorizeExecMode, bool) {
if m == VectorizeUnset {
return 0, false
}
if m == DeprecatedVectorize201Auto {
m = VectorizeOn
}
return m, true
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/sessiondatapb/session_data.proto
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ enum VectorizeExecMode {
// the first enum value as zero is required by proto3. This is mapped to
// VectorizeOn.
unset = 0 [(gogoproto.enumvalue_customname) = "VectorizeUnset"];
reserved 1;
// DeprecatedVectorized201Auto is only possible for clusters that have been
// upgraded from v21.1 or older and had the old setting of "201auto" which
// was mapped to "on" in v21.1. It is now an alias for "on".
deprecated201auto = 1 [(gogoproto.enumvalue_customname) = "DeprecatedVectorize201Auto"];
// VectorizeOn means that any supported queries will be run using the
// columnar execution.
on = 2 [(gogoproto.enumvalue_customname) = "VectorizeOn"];
Expand Down

0 comments on commit 42e3840

Please sign in to comment.