Skip to content

Commit 06d4e1b

Browse files
authored
chore(query): Optimize Optimizer Performance by Reducing Redundant Computations (#18979)
1 parent b6bb253 commit 06d4e1b

File tree

3 files changed

+67
-41
lines changed

3 files changed

+67
-41
lines changed

src/query/sql/src/planner/optimizer/optimizer_context.rs

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
use std::collections::HashMap;
16+
use std::collections::HashSet;
1617
use std::sync::Arc;
1718

1819
use databend_common_catalog::table_context::TableContext;
@@ -38,6 +39,10 @@ pub struct OptimizerContext {
3839
enable_dphyp: RwLock<bool>,
3940
max_push_down_limit: RwLock<usize>,
4041
planning_agg_index: RwLock<bool>,
42+
skip_list: HashSet<String>,
43+
skip_list_str: String,
44+
grouping_sets_to_union: bool,
45+
4146
#[educe(Debug(ignore))]
4247
sample_executor: RwLock<Option<Arc<dyn QueryExecutor>>>,
4348

@@ -52,6 +57,20 @@ pub struct OptimizerContext {
5257

5358
impl OptimizerContext {
5459
pub fn new(table_ctx: Arc<dyn TableContext>, metadata: MetadataRef) -> Arc<Self> {
60+
let settings = table_ctx.get_settings();
61+
let grouping_sets_to_union = settings.get_grouping_sets_to_union().unwrap_or_default();
62+
63+
let (skip_list_str, skip_list) = match settings.get_optimizer_skip_list() {
64+
Ok(skip_list_str) if !skip_list_str.is_empty() => {
65+
let skip_list = skip_list_str
66+
.split(',')
67+
.map(|item| item.trim().to_lowercase())
68+
.collect::<HashSet<_>>();
69+
(skip_list_str.to_string(), skip_list)
70+
}
71+
_ => ("".to_string(), HashSet::new()),
72+
};
73+
5574
Arc::new(Self {
5675
table_ctx,
5776
metadata,
@@ -62,6 +81,9 @@ impl OptimizerContext {
6281
max_push_down_limit: RwLock::new(10000),
6382
sample_executor: RwLock::new(None),
6483
planning_agg_index: RwLock::new(false),
84+
skip_list,
85+
skip_list_str,
86+
grouping_sets_to_union,
6587
flags: RwLock::new(HashMap::new()),
6688
enable_trace: RwLock::new(false),
6789
})
@@ -148,33 +170,22 @@ impl OptimizerContext {
148170

149171
/// Check if an optimizer or rule is disabled based on optimizer_skip_list setting
150172
pub fn is_optimizer_disabled(&self, name: &str) -> bool {
151-
let settings = self.get_table_ctx().get_settings();
152-
153-
if !settings.get_grouping_sets_to_union().unwrap_or_default()
173+
if !self.grouping_sets_to_union
154174
&& (name == RuleID::GroupingSetsToUnion.to_string()
155175
|| name == RuleID::HierarchicalGroupingSetsToUnion.to_string())
156176
{
157177
return true;
158178
}
159179

160-
match settings.get_optimizer_skip_list() {
161-
Ok(skip_list) if !skip_list.is_empty() => {
162-
let name_lower = name.to_lowercase();
163-
let is_disabled = skip_list
164-
.split(',')
165-
.map(str::trim)
166-
.any(|item| item.to_lowercase() == name_lower);
167-
168-
if is_disabled {
169-
log::warn!(
170-
"Skipping optimizer component: {} (found in optimizer_skip_list: {})",
171-
name,
172-
skip_list
173-
);
174-
}
175-
is_disabled
176-
}
177-
_ => false,
180+
let name_lower = name.to_lowercase();
181+
let is_disabled = self.skip_list.contains(&name_lower);
182+
if is_disabled {
183+
log::warn!(
184+
"Skipping optimizer component: {} (found in optimizer_skip_list: {})",
185+
name,
186+
self.skip_list_str
187+
);
178188
}
189+
is_disabled
179190
}
180191
}

src/query/sql/src/planner/optimizer/optimizers/recursive/recursive.rs

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -51,23 +51,30 @@ impl RecursiveRuleOptimizer {
5151

5252
#[recursive::recursive]
5353
fn optimize_expression(&self, s_expr: &SExpr) -> Result<SExpr> {
54-
let mut optimized_children = Vec::with_capacity(s_expr.arity());
55-
let mut children_changed = false;
56-
for expr in s_expr.children() {
57-
let optimized_child = self.optimize_sync(expr)?;
58-
if !optimized_child.eq(expr) {
59-
children_changed = true;
54+
let mut current = s_expr.clone();
55+
56+
loop {
57+
let mut optimized_children = Vec::with_capacity(current.arity());
58+
let mut children_changed = false;
59+
for expr in current.children() {
60+
let optimized_child = self.optimize_sync(expr)?;
61+
if !optimized_child.eq(expr) {
62+
children_changed = true;
63+
}
64+
optimized_children.push(Arc::new(optimized_child));
6065
}
61-
optimized_children.push(Arc::new(optimized_child));
62-
}
63-
let mut optimized_expr = s_expr.clone();
64-
if children_changed {
65-
optimized_expr = s_expr.replace_children(optimized_children);
66-
}
6766

68-
let result = self.apply_transform_rules(&optimized_expr, self.rules)?;
67+
if children_changed {
68+
current = current.replace_children(optimized_children);
69+
}
6970

70-
Ok(result)
71+
match self.apply_transform_rules(&current, self.rules)? {
72+
Some(new_expr) => {
73+
current = new_expr;
74+
}
75+
None => return Ok(current),
76+
}
77+
}
7178
}
7279

7380
/// Trace rule execution, regardless of whether the rule had an effect
@@ -104,7 +111,7 @@ impl RecursiveRuleOptimizer {
104111
Ok(())
105112
}
106113

107-
fn apply_transform_rules(&self, s_expr: &SExpr, rules: &[RuleID]) -> Result<SExpr> {
114+
fn apply_transform_rules(&self, s_expr: &SExpr, rules: &[RuleID]) -> Result<Option<SExpr>> {
108115
let mut s_expr = s_expr.clone();
109116
for rule_id in rules {
110117
let rule = RuleFactory::create_rule(*rule_id, self.ctx.clone())?;
@@ -129,17 +136,16 @@ impl RecursiveRuleOptimizer {
129136
{
130137
s_expr.set_applied_rule(&rule.id());
131138
rule.apply(&s_expr, &mut state)?;
132-
if !state.results().is_empty() {
133-
let result = &state.results()[0];
139+
if let Some(result) = state.results().first() {
140+
let result = result.clone();
134141

135142
// For tracing only
136143
if trace_enabled {
137144
let duration = start_time.elapsed();
138145
self.trace_rule_execution(rule.name(), duration, &before_expr, &state)?;
139146
}
140147

141-
let optimized_result = self.optimize_expression(result)?;
142-
return Ok(optimized_result);
148+
return Ok(Some(result));
143149
}
144150
}
145151

@@ -150,7 +156,7 @@ impl RecursiveRuleOptimizer {
150156
}
151157
}
152158

153-
Ok(s_expr)
159+
Ok(None)
154160
}
155161
}
156162

src/query/sql/src/planner/semantic/type_check.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4939,6 +4939,7 @@ impl<'a> TypeChecker<'a> {
49394939
Some(Vec::with_capacity(exprs.len()));
49404940
let mut element_type: Option<DataType> = None;
49414941

4942+
let mut data_type_set = HashSet::with_capacity(2);
49424943
for expr in exprs {
49434944
let box (arg, data_type) = self.resolve(expr)?;
49444945
if let Some(values) = constant_values.as_mut() {
@@ -4948,6 +4949,13 @@ impl<'a> TypeChecker<'a> {
49484949
_ => None,
49494950
};
49504951
if let Some(value) = maybe_constant {
4952+
// If the data type has already been computed,
4953+
// we don't need to compute the common type again.
4954+
if data_type_set.contains(&data_type) {
4955+
elems.push(arg);
4956+
values.push((value, data_type));
4957+
continue;
4958+
}
49514959
element_type = if let Some(current_ty) = element_type.clone() {
49524960
common_super_type(
49534961
current_ty.clone(),
@@ -4959,6 +4967,7 @@ impl<'a> TypeChecker<'a> {
49594967
};
49604968

49614969
if element_type.is_some() {
4970+
data_type_set.insert(data_type.clone());
49624971
values.push((value, data_type));
49634972
} else {
49644973
constant_values = None;

0 commit comments

Comments
 (0)