Skip to content
This repository was archived by the owner on Jan 7, 2025. It is now read-only.

Commit d77b3e3

Browse files
committed
fix(core): do not apply logical rule if iter budget is exausted (#265)
Signed-off-by: Alex Chi Z <[email protected]>
1 parent 6497fe7 commit d77b3e3

File tree

5 files changed

+26
-19
lines changed

5 files changed

+26
-19
lines changed

Cargo.lock

+4-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datafusion-optd-cli/tests/cli_integration.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use std::process::Command;
18+
use std::process::{Command, Stdio};
1919

2020
use assert_cmd::prelude::CommandCargoExt;
2121

@@ -55,11 +55,8 @@ fn init() {
5555
fn cli_test_tpch() {
5656
let mut cmd = Command::cargo_bin("datafusion-optd-cli").unwrap();
5757
cmd.current_dir(".."); // all paths in `test.sql` assume we're in the base dir of the repo
58-
cmd.args([
59-
"--enable-df-logical",
60-
"--file",
61-
"datafusion-optd-cli/tpch-sf0_01/test.sql",
62-
]);
58+
cmd.args(["--file", "datafusion-optd-cli/tpch-sf0_01/test.sql"]);
59+
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
6360
let status = cmd.status().unwrap();
6461
assert!(
6562
status.success(),

optd-core/src/cascades/optimizer.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ impl<T: NodeType, M: Memo<T>> CascadesOptimizer<T, M> {
297297
trace!(event = "fire_optimize_tasks", root_group_id = %group_id);
298298
let mut task = TaskContext::new(self);
299299
// 32MB stack for the optimization process, TODO: reduce memory footprint
300-
stacker::maybe_grow(32 * 1024 * 1024, 32 * 1024 * 1024, || {
300+
stacker::grow(32 * 1024 * 1024, || {
301301
let fut: Pin<Box<dyn Future<Output = ()>>> = Box::pin(task.fire_optimize(group_id));
302302
fut.block_on();
303303
});

optd-core/src/cascades/tasks2.rs

+17-8
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,14 @@ impl<'a, T: NodeType, M: Memo<T>> TaskContext<'a, T, M> {
160160
continue;
161161
}
162162
// Skip transformation rules when budget is used
163-
if self.optimizer.ctx.logical_budget_used && !rule.is_impl_rule() {
163+
if (self.optimizer.ctx.logical_budget_used || self.optimizer.ctx.all_budget_used)
164+
&& !rule.is_impl_rule()
165+
{
164166
continue;
165167
}
166-
if self.optimizer.ctx.all_budget_used {
168+
if self.optimizer.ctx.all_budget_used
169+
&& self.optimizer.get_group_winner(group_id).has_full_winner()
170+
{
167171
break;
168172
}
169173
if top_matches(rule.matcher(), expr.typ.clone()) {
@@ -246,7 +250,8 @@ impl<'a, T: NodeType, M: Memo<T>> TaskContext<'a, T, M> {
246250
let rule = self.optimizer.rules()[rule_id].clone();
247251

248252
let binding_exprs = match_and_pick_expr(rule.matcher(), expr_id, self.optimizer);
249-
if binding_exprs.len() >= 100 {
253+
const BINDING_EXPR_WARNING_THRESHOLD: usize = 200;
254+
if binding_exprs.len() >= BINDING_EXPR_WARNING_THRESHOLD {
250255
tracing::warn!(
251256
event = "rule_application",
252257
task = "apply_rule",
@@ -302,12 +307,16 @@ impl<'a, T: NodeType, M: Memo<T>> TaskContext<'a, T, M> {
302307
}
303308
}
304309

305-
if self.optimizer.ctx.all_budget_used {
306-
break;
307-
}
308-
if self.optimizer.ctx.logical_budget_used && !rule.is_impl_rule() {
310+
if (self.optimizer.ctx.logical_budget_used || self.optimizer.ctx.all_budget_used)
311+
&& !rule.is_impl_rule()
312+
{
309313
continue;
310314
}
315+
if self.optimizer.ctx.all_budget_used
316+
&& self.optimizer.get_group_winner(group_id).has_full_winner()
317+
{
318+
break;
319+
}
311320

312321
trace!(event = "before_apply_rule", task = "apply_rule", input_binding=%binding);
313322
let applied = rule.apply(self.optimizer, binding);
@@ -540,7 +549,7 @@ impl<'a, T: NodeType, M: Memo<T>> TaskContext<'a, T, M> {
540549

541550
fn on_task_start(&self) {
542551
if (self.optimizer.ctx.all_budget_used || self.optimizer.ctx.logical_budget_used)
543-
&& self.steps % 100 == 0
552+
&& self.steps % 100000 == 0
544553
{
545554
println!("out of budget, dumping info");
546555
println!("step={}", self.steps);

optd-sqlplannertest/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ repository = { workspace = true }
1313
[dependencies]
1414
clap = { version = "4.5.4", features = ["derive"] }
1515
anyhow = { version = "1", features = ["backtrace"] }
16-
sqlplannertest = "0.4"
16+
sqlplannertest = "0.4.1"
1717
async-trait = "0.1"
1818
datafusion-optd-cli = { path = "../datafusion-optd-cli", version = "43.0.0" }
1919
optd-datafusion-repr-adv-cost = { path = "../optd-datafusion-repr-adv-cost", version = "0.1" }

0 commit comments

Comments
 (0)