Fix: Heuristic wrapper fix (#150)

The flow of the optimization is: OptimizeGroup(root) --> exploreGroup(children)(can only apply logical->logical) --> after exploration, there are physical exprs in the root --> optimizeInput --> optimizeGroup(children)(can apply logical to physical) While marking the rules as deadend, we cannot mark the impl rules as fired as we might need them to start the optimizeGroup task of the children node after merging of groups even if we know the expr can never be the winner. --------- Signed-off-by: AveryQi115 <[email protected]>
cmu-db · Apr 22, 2024 · 7574267 · 7574267
1 parent ac76856
commit 7574267
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 36 deletions.
diff --git a/optd-core/src/cascades/memo.rs b/optd-core/src/cascades/memo.rs
@@ -297,6 +297,7 @@ impl<T: RelNodeTyp> Memo<T> {
                 }
                 let group_id = self.get_group_id_of_expr_id(new_expr_id);
                 let group_id = self.get_reduced_group_id(group_id);
+
                 self.merge_group_inner(replace_group_id, group_id);
                 return false;
             }

diff --git a/optd-core/src/cascades/optimizer.rs b/optd-core/src/cascades/optimizer.rs
@@ -292,17 +292,19 @@ impl<T: RelNodeTyp> CascadesOptimizer<T> {
         let replaced = self.memo.replace_group_expr(expr_id, group_id, expr);
         if replaced {
             // the old expr is replaced, so we clear the fired rules for old expr
-            self.fired_rules
-                .entry(expr_id)
-                .and_modify(|fired_rules| fired_rules.clear());
+            self.fired_rules.entry(expr_id).or_default().clear();
             return;
         }
-        // new expr merged with old expr, we mark old expr as a dead end
-        self.fired_rules.entry(expr_id).and_modify(|fired_rules| {
-            for i in 0..self.rules.len() {
-                fired_rules.insert(i);
+
+        // We can mark the expr as a deadend
+        // However, even some of the exprs cannot be the winner for the group
+        // We still need the physical form of those expr to start the optimizeInput task
+        // So we don't mark the impl rules as fired
+        for i in 0..self.rules.len() {
+            if !self.rules[i].rule().is_impl_rule() {
+                self.fired_rules.entry(expr_id).or_default().insert(i);
             }
-        });
+        }
     }
 
     pub(super) fn get_group_info(&self, group_id: GroupId) -> GroupInfo {

diff --git a/optd-core/src/cascades/tasks/apply_rule.rs b/optd-core/src/cascades/tasks/apply_rule.rs
@@ -233,11 +233,10 @@ impl<T: RelNodeTyp> Task<T> for ApplyRuleTask {
 
                     trace!(event = "apply_rule replace", expr_id = %self.expr_id, rule_id = %self.rule_id);
 
-                    // the expr returned by heuristic rule is a brand new one
-                    // so there's no optimizeExpressionTask for it in the original task list
-                    // we should set exploring as false to both envoke tranform rule and impl rule for it
-                    tasks.push(Box::new(OptimizeExpressionTask::new(self.expr_id, false))
-                        as Box<dyn Task<T>>);
+                    tasks.push(
+                        Box::new(OptimizeExpressionTask::new(self.expr_id, self.exploring))
+                            as Box<dyn Task<T>>,
+                    );
                 }
                 continue;
             }

diff --git a/optd-sqlplannertest/tests/tpch.planner.sql b/optd-sqlplannertest/tests/tpch.planner.sql
@@ -627,29 +627,29 @@ PhysicalSort
         │           ├── Cast { cast_to: Decimal128(20, 0), expr: 1 }
         │           └── #23
         ├── groups: [ #41 ]
-        └── PhysicalHashJoin { join_type: Inner, left_keys: [ #36 ], right_keys: [ #0 ] }
-            ├── PhysicalHashJoin { join_type: Inner, left_keys: [ #19, #3 ], right_keys: [ #0, #3 ] }
-            │   ├── PhysicalHashJoin { join_type: Inner, left_keys: [ #0 ], right_keys: [ #1 ] }
-            │   │   ├── PhysicalScan { table: customer }
-            │   │   └── PhysicalHashJoin { join_type: Inner, left_keys: [ #0 ], right_keys: [ #0 ] }
-            │   │       ├── PhysicalFilter
-            │   │       │   ├── cond:And
-            │   │       │   │   ├── Geq
-            │   │       │   │   │   ├── #4
-            │   │       │   │   │   └── Cast { cast_to: Date32, expr: "2023-01-01" }
-            │   │       │   │   └── Lt
-            │   │       │   │       ├── #4
-            │   │       │   │       └── Cast { cast_to: Date32, expr: "2024-01-01" }
-            │   │       │   └── PhysicalScan { table: orders }
-            │   │       └── PhysicalScan { table: lineitem }
-            │   └── PhysicalScan { table: supplier }
-            └── PhysicalHashJoin { join_type: Inner, left_keys: [ #2 ], right_keys: [ #0 ] }
-                ├── PhysicalScan { table: nation }
-                └── PhysicalFilter
-                    ├── cond:Eq
-                    │   ├── #1
-                    │   └── "Asia"
-                    └── PhysicalScan { table: region }
+        └── PhysicalHashJoin { join_type: Inner, left_keys: [ #19, #3 ], right_keys: [ #0, #3 ] }
+            ├── PhysicalHashJoin { join_type: Inner, left_keys: [ #0 ], right_keys: [ #1 ] }
+            │   ├── PhysicalScan { table: customer }
+            │   └── PhysicalHashJoin { join_type: Inner, left_keys: [ #0 ], right_keys: [ #0 ] }
+            │       ├── PhysicalFilter
+            │       │   ├── cond:And
+            │       │   │   ├── Geq
+            │       │   │   │   ├── #4
+            │       │   │   │   └── Cast { cast_to: Date32, expr: "2023-01-01" }
+            │       │   │   └── Lt
+            │       │   │       ├── #4
+            │       │   │       └── Cast { cast_to: Date32, expr: "2024-01-01" }
+            │       │   └── PhysicalScan { table: orders }
+            │       └── PhysicalScan { table: lineitem }
+            └── PhysicalHashJoin { join_type: Inner, left_keys: [ #3 ], right_keys: [ #0 ] }
+                ├── PhysicalScan { table: supplier }
+                └── PhysicalHashJoin { join_type: Inner, left_keys: [ #2 ], right_keys: [ #0 ] }
+                    ├── PhysicalScan { table: nation }
+                    └── PhysicalFilter
+                        ├── cond:Eq
+                        │   ├── #1
+                        │   └── "Asia"
+                        └── PhysicalScan { table: region }
 */
 
 -- TPC-H Q6