Skip to content

Commit c4dd5e0

Browse files
authored
adapter: apply missing optimizer feature overrides (#35181)
For indexes, MVs, and CTs, optimizer features can be overridden on a per-cluster basis. This PR fixes two places where we forgot to apply these overrides. This fixes a bug where dataflows would be incorrectly re-planned after an envd restart because we'd drop their cluster overrides.
1 parent 6cf273f commit c4dd5e0

3 files changed

Lines changed: 69 additions & 6 deletions

File tree

src/adapter/src/catalog/state.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ use mz_repr::namespaces::{
5555
UNSTABLE_SCHEMAS,
5656
};
5757
use mz_repr::network_policy_id::NetworkPolicyId;
58-
use mz_repr::optimize::OptimizerFeatures;
58+
use mz_repr::optimize::{OptimizerFeatures, OverrideFrom};
5959
use mz_repr::role_id::RoleId;
6060
use mz_repr::{
6161
CatalogItemId, GlobalId, RelationDesc, RelationVersion, RelationVersionSelector,
@@ -1328,8 +1328,13 @@ impl CatalogState {
13281328
.collect();
13291329

13301330
// Collect optimizer parameters.
1331+
let system_vars = session_catalog.system_vars();
1332+
let overrides = self
1333+
.get_cluster(materialized_view.cluster_id)
1334+
.config
1335+
.features();
13311336
let optimizer_config =
1332-
optimize::OptimizerConfig::from(session_catalog.system_vars());
1337+
optimize::OptimizerConfig::from(system_vars).override_from(&overrides);
13331338
let previous_exprs = previous_item.map(|item| match item {
13341339
CatalogItem::MaterializedView(materialized_view) => {
13351340
(materialized_view.raw_expr, materialized_view.optimized_expr)

src/adapter/src/coord.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ use mz_persist_client::usage::{ShardsUsageReferenced, StorageUsageClient};
134134
use mz_repr::adt::numeric::Numeric;
135135
use mz_repr::explain::{ExplainConfig, ExplainFormat};
136136
use mz_repr::global_id::TransientIdGen;
137-
use mz_repr::optimize::OptimizerFeatures;
137+
use mz_repr::optimize::{OptimizerFeatures, OverrideFrom};
138138
use mz_repr::role_id::RoleId;
139139
use mz_repr::{CatalogItemId, Diff, GlobalId, RelationDesc, SqlRelationType, Timestamp};
140140
use mz_secrets::cache::CachingSecretsReader;
@@ -3050,7 +3050,11 @@ impl Coordinator {
30503050
let mut instance_snapshots = BTreeMap::new();
30513051
let mut uncached_expressions = BTreeMap::new();
30523052

3053-
let optimizer_config = OptimizerConfig::from(self.catalog().system_config());
3053+
let optimizer_config = |catalog: &Catalog, cluster_id| {
3054+
let system_config = catalog.system_config();
3055+
let overrides = catalog.get_cluster(cluster_id).config.features();
3056+
OptimizerConfig::from(system_config).override_from(&overrides)
3057+
};
30543058

30553059
for entry in ordered_catalog_entries {
30563060
match entry.item() {
@@ -3069,6 +3073,8 @@ impl Coordinator {
30693073
continue;
30703074
}
30713075

3076+
let optimizer_config = optimizer_config(&self.catalog, idx.cluster_id);
3077+
30723078
let (optimized_plan, physical_plan, metainfo) =
30733079
match cached_global_exprs.remove(&global_id) {
30743080
Some(global_expressions)
@@ -3089,7 +3095,7 @@ impl Coordinator {
30893095
self.owned_catalog(),
30903096
compute_instance.clone(),
30913097
global_id,
3092-
optimizer_config.clone(),
3098+
optimizer_config,
30933099
self.optimizer_metrics(),
30943100
);
30953101

@@ -3154,6 +3160,8 @@ impl Coordinator {
31543160
});
31553161
let global_id = mv.global_id_writes();
31563162

3163+
let optimizer_config = optimizer_config(&self.catalog, mv.cluster_id);
3164+
31573165
let (optimized_plan, physical_plan, metainfo) =
31583166
match cached_global_exprs.remove(&global_id) {
31593167
Some(global_expressions)
@@ -3186,7 +3194,7 @@ impl Coordinator {
31863194
mv.non_null_assertions.clone(),
31873195
mv.refresh_schedule.clone(),
31883196
debug_name,
3189-
optimizer_config.clone(),
3197+
optimizer_config,
31903198
self.optimizer_metrics(),
31913199
force_non_monotonic,
31923200
);
@@ -3252,6 +3260,8 @@ impl Coordinator {
32523260
});
32533261
let global_id = ct.global_id();
32543262

3263+
let optimizer_config = optimizer_config(&self.catalog, ct.cluster_id);
3264+
32553265
let (optimized_plan, physical_plan, metainfo) =
32563266
match cached_global_exprs.remove(&global_id) {
32573267
Some(global_expressions)

test/cluster/mzcompose.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6467,3 +6467,51 @@ def workflow_github_10086(c: Composition) -> None:
64676467
"""
64686468
)[0][0]
64696469
assert not upper_empty
6470+
6471+
6472+
def workflow_test_optimizer_feature_override_after_restart(c: Composition) -> None:
6473+
"""
6474+
Test that verifies that optimizer feature overrides survive envd restarts.
6475+
"""
6476+
6477+
with c.override(
6478+
Materialized(
6479+
additional_system_parameter_defaults={
6480+
"enable_eager_delta_joins": "false",
6481+
},
6482+
),
6483+
):
6484+
c.up("materialized")
6485+
6486+
c.sql(
6487+
"""
6488+
CREATE CLUSTER test (SIZE 'scale=1,workers=1')
6489+
FEATURES (ENABLE EAGER DELTA JOINS = true);
6490+
GRANT ALL ON CLUSTER test TO materialize;
6491+
""",
6492+
port=6877,
6493+
user="mz_system",
6494+
)
6495+
6496+
c.sql(
6497+
"""
6498+
SET cluster = test;
6499+
6500+
CREATE TABLE t1 (x int, y int);
6501+
CREATE TABLE t2 (x int, y int);
6502+
CREATE TABLE t3 (x int, y int);
6503+
CREATE MATERIALIZED VIEW mv2 IN CLUSTER test AS
6504+
SELECT t1.y as f1, t2.y as f2, t3.y as f3
6505+
FROM t1, t2, t3
6506+
WHERE t1.x = t2.x AND t2.y = t3.y;
6507+
"""
6508+
)
6509+
6510+
plan1 = c.sql_query("EXPLAIN MATERIALIZED VIEW mv2")[0][0]
6511+
6512+
c.kill("materialized")
6513+
c.up("materialized")
6514+
6515+
plan2 = c.sql_query("EXPLAIN MATERIALIZED VIEW mv2")[0][0]
6516+
6517+
assert plan1 == plan2, f"before={plan1}\nafter={plan2}"

0 commit comments

Comments
 (0)