Skip to content

Commit 2b99ef8

Browse files
Remove feature flag for #30974 (#32031)
Remove the defensive feature flag for #30974, which seems to have landed without incident. ### Motivation <!-- Which of the following best describes the motivation behind this PR? * This PR fixes a recognized bug. [Ensure issue is linked somewhere.] * This PR adds a known-desirable feature. [Ensure issue is linked somewhere.] * This PR fixes a previously unreported bug. [Describe the bug in detail, as if you were filing a bug report.] * This PR adds a feature that has not yet been specified. [Write a brief specification for the feature, including justification for its inclusion in Materialize, as if you were writing the original feature specification.] * This PR refactors existing code. [Describe what was wrong with the existing code, if it is not obvious.] --> ### Tips for reviewer <!-- Leave some tips for your reviewer, like: * The diff is much smaller if viewed with whitespace hidden. * [Some function/module/file] deserves extra attention. * [Some function/module/file] is pure code movement and only needs a skim. Delete this section if no tips. --> ### Checklist - [ ] This PR has adequate test coverage / QA involvement has been duly considered. ([trigger-ci for additional test/nightly runs](https://trigger-ci.dev.materialize.com/)) - [ ] This PR has an associated up-to-date [design doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md), is a design doc ([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)), or is sufficiently small to not require a design. <!-- Reference the design in the description. --> - [ ] If this PR evolves [an existing `$T ⇔ Proto$T` mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md) (possibly in a backwards-incompatible way), then it is tagged with a `T-proto` label. - [ ] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label ([example](MaterializeInc/cloud#5021)). <!-- Ask in #team-cloud on Slack if you need help preparing the cloud PR. --> - [ ] If this PR includes major [user-facing behavior changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note), I have pinged the relevant PM to schedule a changelog post.
1 parent fa1621e commit 2b99ef8

File tree

6 files changed

+12
-52
lines changed

6 files changed

+12
-52
lines changed

src/repr/src/optimize.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,6 @@ optimizer_feature_flags!({
123123
// See the feature flag of the same name.
124124
enable_projection_pushdown_after_relation_cse: bool,
125125
// See the feature flag of the same name.
126-
enable_let_prefix_extraction: bool,
127-
// See the feature flag of the same name.
128126
enable_less_reduce_in_eqprop: bool,
129127
});
130128

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4650,7 +4650,6 @@ pub fn unplan_create_cluster(
46504650
enable_reduce_reduction: _,
46514651
enable_join_prioritize_arranged,
46524652
enable_projection_pushdown_after_relation_cse,
4653-
enable_let_prefix_extraction: _,
46544653
enable_less_reduce_in_eqprop: _,
46554654
} = optimizer_feature_overrides;
46564655
// The ones from above that don't occur below are not wired up to cluster features.

src/sql/src/plan/statement/dml.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,6 @@ impl TryFrom<ExplainPlanOptionExtracted> for ExplainConfig {
434434
enable_join_prioritize_arranged: v.enable_join_prioritize_arranged,
435435
enable_projection_pushdown_after_relation_cse: v
436436
.enable_projection_pushdown_after_relation_cse,
437-
enable_let_prefix_extraction: Default::default(),
438437
enable_less_reduce_in_eqprop: Default::default(),
439438
},
440439
})

src/sql/src/session/vars/definitions.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1785,12 +1785,6 @@ macro_rules! feature_flags {
17851785
}
17861786

17871787
feature_flags!(
1788-
{
1789-
name: enable_let_prefix_extraction,
1790-
desc: "Enables hoisting of loop-invariant CTE bindindgs",
1791-
default: true,
1792-
enable_for_item_parsing: false,
1793-
},
17941788
// Gates for other feature flags
17951789
{
17961790
name: allow_real_time_recency,
@@ -2233,7 +2227,6 @@ impl From<&super::SystemVars> for OptimizerFeatures {
22332227
enable_join_prioritize_arranged: vars.enable_join_prioritize_arranged(),
22342228
enable_projection_pushdown_after_relation_cse: vars
22352229
.enable_projection_pushdown_after_relation_cse(),
2236-
enable_let_prefix_extraction: vars.enable_let_prefix_extraction(),
22372230
enable_less_reduce_in_eqprop: vars.enable_less_reduce_in_eqprop(),
22382231
}
22392232
}

src/transform/src/normalize_lets.rs

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -132,32 +132,6 @@ impl NormalizeLets {
132132

133133
// A final bottom-up traversal to normalize the shape of nested LetRec blocks
134134
relation.try_visit_mut_post(&mut |relation| -> Result<(), RecursionLimitError> {
135-
if !features.enable_let_prefix_extraction {
136-
// Disassemble `LetRec` into a `Let` stack if possible.
137-
// If a `LetRec` remains, return the would-be `Let` bindings to it.
138-
// This is to maintain `LetRec`-freedom for `LetRec`-free expressions.
139-
let mut bindings = let_motion::harvest_non_recursive(relation);
140-
if let MirRelationExpr::LetRec {
141-
ids,
142-
values,
143-
limits,
144-
body: _,
145-
} = relation
146-
{
147-
bindings.extend(ids.drain(..).zip(values.drain(..).zip(limits.drain(..))));
148-
support::replace_bindings_from_map(bindings, ids, values, limits);
149-
} else {
150-
for (id, (value, max_iter)) in bindings.into_iter().rev() {
151-
assert_none!(max_iter);
152-
*relation = MirRelationExpr::Let {
153-
id,
154-
value: Box::new(value),
155-
body: Box::new(relation.take_dangerous()),
156-
};
157-
}
158-
}
159-
}
160-
161135
// Move a non-recursive suffix of bindings from the end of the LetRec
162136
// to the LetRec body.
163137
// This is unsafe when applied to expressions which contain `ArrangeBy`,
@@ -189,20 +163,18 @@ impl NormalizeLets {
189163
}
190164
}
191165

192-
if features.enable_let_prefix_extraction {
193-
// Extract `Let` prefixes from `LetRec`, to reveal their non-recursive nature.
194-
// This assists with hoisting e.g. arrangements out of `LetRec` blocks, a thing
195-
// we don't promise to do, but it can be helpful to do. This also exposes more
196-
// AST nodes to non-`LetRec` analyses, which don't always have parity with `LetRec`.
197-
let bindings = let_motion::harvest_non_recursive(relation);
198-
for (id, (value, max_iter)) in bindings.into_iter().rev() {
199-
assert_none!(max_iter);
200-
*relation = MirRelationExpr::Let {
201-
id,
202-
value: Box::new(value),
203-
body: Box::new(relation.take_dangerous()),
204-
};
205-
}
166+
// Extract `Let` prefixes from `LetRec`, to reveal their non-recursive nature.
167+
// This assists with hoisting e.g. arrangements out of `LetRec` blocks, a thing
168+
// we don't promise to do, but it can be helpful to do. This also exposes more
169+
// AST nodes to non-`LetRec` analyses, which don't always have parity with `LetRec`.
170+
let bindings = let_motion::harvest_non_recursive(relation);
171+
for (id, (value, max_iter)) in bindings.into_iter().rev() {
172+
assert_none!(max_iter);
173+
*relation = MirRelationExpr::Let {
174+
id,
175+
value: Box::new(value),
176+
body: Box::new(relation.take_dangerous()),
177+
};
206178
}
207179

208180
Ok(())

src/transform/tests/test_transforms.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,6 @@ fn apply_transform<T: mz_transform::Transform>(
260260
let mut features = mz_repr::optimize::OptimizerFeatures::default();
261261
// Apply a non-default feature flag to test the right implementation.
262262
features.enable_letrec_fixpoint_analysis = true;
263-
features.enable_let_prefix_extraction = true;
264263
let typecheck_ctx = mz_transform::typecheck::empty_context();
265264
let mut df_meta = DataflowMetainfo::default();
266265
let mut transform_ctx =

0 commit comments

Comments
 (0)