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: #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 bebf89e commit 5dc0e71
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 5 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ This session variable default should now be configured using ALTER ROLE... SET:
sql.defaults.use_declarative_schema_changer enumeration on "default value for use_declarative_schema_changer session setting;disables new schema changer by default [off = 0, on = 1, unsafe = 2, unsafe_always = 3]
This cluster setting is being kept to preserve backwards-compatibility.
This session variable default should now be configured using ALTER ROLE... SET: https://www.cockroachlabs.com/docs/stable/alter-role.html" application
sql.defaults.vectorize enumeration on "default vectorize mode [on = 0, on = 2, experimental_always = 3, off = 4]
sql.defaults.vectorize enumeration on "default vectorize mode [on = 0, on = 1, on = 2, experimental_always = 3, off = 4]
This cluster setting is being kept to preserve backwards-compatibility.
This session variable default should now be configured using ALTER ROLE... SET: https://www.cockroachlabs.com/docs/stable/alter-role.html" application
sql.defaults.zigzag_join.enabled boolean false "default value for enable_zigzag_join session setting; disallows use of zig-zag join by default
Expand Down
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
<tr><td><div id="setting-sql-defaults-transaction-rows-written-err" class="anchored"><code>sql.defaults.transaction_rows_written_err</code></div></td><td>integer</td><td><code>0</code></td><td>the limit for the number of rows written by a SQL transaction which - once exceeded - will fail the transaction (or will trigger a logging event to SQL_INTERNAL_PERF for internal transactions); use 0 to disable<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-defaults-transaction-rows-written-log" class="anchored"><code>sql.defaults.transaction_rows_written_log</code></div></td><td>integer</td><td><code>0</code></td><td>the threshold for the number of rows written by a SQL transaction which - once exceeded - will trigger a logging event to SQL_PERF (or SQL_INTERNAL_PERF for internal transactions); use 0 to disable<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-defaults-use-declarative-schema-changer" class="anchored"><code>sql.defaults.use_declarative_schema_changer</code></div></td><td>enumeration</td><td><code>on</code></td><td>default value for use_declarative_schema_changer session setting;disables new schema changer by default [off = 0, on = 1, unsafe = 2, unsafe_always = 3]<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-defaults-vectorize" class="anchored"><code>sql.defaults.vectorize</code></div></td><td>enumeration</td><td><code>on</code></td><td>default vectorize mode [on = 0, on = 2, experimental_always = 3, off = 4]<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-defaults-vectorize" class="anchored"><code>sql.defaults.vectorize</code></div></td><td>enumeration</td><td><code>on</code></td><td>default vectorize mode [on = 0, on = 1, on = 2, experimental_always = 3, off = 4]<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-defaults-zigzag-join-enabled" class="anchored"><code>sql.defaults.zigzag_join.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>default value for enable_zigzag_join session setting; disallows use of zig-zag join by default<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-distsql-temp-storage-workmem" class="anchored"><code>sql.distsql.temp_storage.workmem</code></div></td><td>byte size</td><td><code>64 MiB</code></td><td>maximum amount of memory in bytes a processor can use before falling back to temp storage</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-guardrails-max-row-size-err" class="anchored"><code>sql.guardrails.max_row_size_err</code></div></td><td>byte size</td><td><code>512 MiB</code></td><td>maximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an error is returned; use 0 to disable</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,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 @@ -1404,3 +1404,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 @@ -167,7 +167,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 5dc0e71

Please sign in to comment.