Skip to content

Commit 3815d52

Browse files
beneschParkMyCar
authored andcommitted
sql: fix regression in ALTER CLUSTER ... SET (SIZE ...)
Due to a bug in #24748, `ALTER CLUSTER ... SET (SIZE ...)` was inadvertently enabling the `DISK` option, even when not explicitly specified, when resizing between non-v2 sizes. Huge thanks to @bobbyiliev for catching this ahead of the release.
1 parent 65de0dd commit 3815d52

2 files changed

Lines changed: 33 additions & 4 deletions

File tree

src/sql/src/plan/statement/ddl.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4514,11 +4514,14 @@ pub fn plan_alter_cluster(
45144514
// The long term plan is to phase out the v1 cluster sizes, at
45154515
// which point we'll be able to remove the `DISK` option
45164516
// entirely and simply always enable disk.
4517-
if is_cluster_size_v2(size) && disk.is_some() {
4518-
sql_bail!("DISK option not supported for cluster sizes ending in cc or C because disk is always enabled");
4517+
if is_cluster_size_v2(size) {
4518+
if disk.is_some() {
4519+
sql_bail!("DISK option not supported for cluster sizes ending in cc or C because disk is always enabled");
4520+
} else {
4521+
options.disk = AlterOptionParameter::Set(true);
4522+
}
45194523
}
45204524
options.size = AlterOptionParameter::Set(size.clone());
4521-
options.disk = AlterOptionParameter::Set(true);
45224525
}
45234526
if let Some(availability_zones) = availability_zones {
45244527
options.availability_zones = AlterOptionParameter::Set(availability_zones);

test/testdrive/cc_cluster_sizes.td

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
# by the Apache License, Version 2.0.
99

1010
$ postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr}
11-
ALTER SYSTEM SET enable_cc_cluster_sizes = false
11+
ALTER SYSTEM SET enable_cc_cluster_sizes = false;
12+
ALTER SYSTEM SET disk_cluster_replicas_default = false;
1213

1314
# Cannot create clusters with cc cluster size naming schemes
1415
! CREATE CLUSTER c SIZE '1cc';
@@ -39,13 +40,19 @@ $ postgres-execute connection=postgres://mz_system:materialize@${testdrive.mater
3940
ALTER SYSTEM SET enable_cc_cluster_sizes = true
4041

4142
> CREATE CLUSTER c SIZE '1cc';
43+
> SELECT disk FROM mz_clusters WHERE name = 'c'
44+
true
4245
> DROP CLUSTER c
4346

4447
> CREATE CLUSTER c SIZE '1C';
48+
> SELECT disk FROM mz_clusters WHERE name = 'c'
49+
true
4550
> DROP CLUSTER c
4651

4752
# Create a cluster with a legacy size with disk enabled.
4853
> CREATE CLUSTER c SIZE '1', DISK = true
54+
> SELECT disk FROM mz_clusters WHERE name = 'c'
55+
true
4956

5057
# Altering to a cc size with disk explicitly toggled is not allowed.
5158
! ALTER CLUSTER c SET (SIZE = '1cc', DISK = true)
@@ -57,18 +64,26 @@ contains:DISK option not supported for cluster sizes ending in cc or C
5764
# even though the cluster's initial creation specified disk explicitly. The
5865
# DISK value is just forced to true.
5966
> ALTER CLUSTER c SET (SIZE = '1cc')
67+
> SELECT disk FROM mz_clusters WHERE name = 'c'
68+
true
6069
> DROP CLUSTER c
6170

6271
# Same test as before, except the legacy size cluster has disk explicitly
6372
# disabled.
6473
> CREATE CLUSTER c SIZE '1', DISK = false
6574
> ALTER CLUSTER c SET (SIZE = '1cc')
75+
> SELECT disk FROM mz_clusters WHERE name = 'c'
76+
true
6677
> DROP CLUSTER c
6778

6879
# Same test as before, except the legacy size cluster has no disk explicitly
6980
# configured.
7081
> CREATE CLUSTER c SIZE = '1'
82+
> SELECT disk FROM mz_clusters WHERE name = 'c'
83+
false
7184
> ALTER CLUSTER c SET (SIZE = '1cc')
85+
> SELECT disk FROM mz_clusters WHERE name = 'c'
86+
true
7287

7388
# Cannot explicitly alter DISK option for new sizes.
7489
! ALTER CLUSTER c SET (DISK = false)
@@ -78,6 +93,17 @@ contains:DISK option not supported for cluster sizes ending in cc or C
7893

7994
# But it's okay if you're going back to a legacy size.
8095
> ALTER CLUSTER c SET (DISK = true, SIZE = '1')
96+
> SELECT disk FROM mz_clusters WHERE name = 'c'
97+
true
98+
> DROP CLUSTER c
99+
100+
# Ensure that altering from a legacy size to a legacy size does not enable disk.
101+
> CREATE CLUSTER c SIZE = '1'
102+
> SELECT disk FROM mz_clusters WHERE name = 'c'
103+
false
104+
> ALTER CLUSTER c SET (SIZE = '2')
105+
> SELECT disk FROM mz_clusters WHERE name = 'c'
106+
false
81107
> DROP CLUSTER c
82108

83109
# Ensure that disk isn't configurable for the new sizes (as it's force enabled).

0 commit comments

Comments
 (0)