refactor(core): drop children_cost on the cost model, doc property bu…

…ilders (#235) As discussed today :) Signed-off-by: Alex Chi <[email protected]>
cmu-db · Nov 13, 2024 · 91cd0a2 · 91cd0a2
1 parent 342f2fb
commit 91cd0a2
Show file tree

Hide file tree

Showing 8 changed files with 16 additions and 22 deletions.
diff --git a/optd-core/src/cascades/tasks/optimize_inputs.rs b/optd-core/src/cascades/tasks/optimize_inputs.rs
@@ -197,7 +197,6 @@ impl<T: NodeType, M: Memo<T>> Task<T, M> for OptimizeInputsTask {
                 &expr.typ,
                 &preds,
                 &input_statistics_ref,
-                &input_cost,
                 Some(context.clone()),
                 Some(optimizer),
             );

diff --git a/optd-core/src/cost.rs b/optd-core/src/cost.rs
@@ -23,7 +23,6 @@ pub trait CostModel<T: NodeType, M: Memo<T>>: 'static + Send + Sync {
         node: &T,
         predicates: &[ArcPredNode<T>],
         children_stats: &[Option<&Statistics>],
-        children_costs: &[Cost],
         context: Option<RelNodeContext>,
         optimizer: Option<&CascadesOptimizer<T, M>>,
     ) -> Cost;

diff --git a/optd-core/src/logical_property.rs b/optd-core/src/logical_property.rs
@@ -8,10 +8,15 @@ use std::fmt::{Debug, Display};
 
 use crate::nodes::{ArcPredNode, NodeType};
 
+/// The trait enables we store any logical property in the memo table by erasing the concrete type.
+/// In the future, we can implement `serialize`/`deserialize` on this trait so that we can serialize
+/// the logical properties.
 pub trait LogicalProperty: 'static + Any + Send + Sync + Debug + Display {
     fn as_any(&self) -> &dyn Any;
 }
 
+/// A wrapper around the `LogicalPropertyBuilder` so that we can erase the concrete type and store
+/// it safely in the memo table.
 pub trait LogicalPropertyBuilderAny<T: NodeType>: 'static + Send + Sync {
     fn derive_any(
         &self,
@@ -22,6 +27,7 @@ pub trait LogicalPropertyBuilderAny<T: NodeType>: 'static + Send + Sync {
     fn property_name(&self) -> &'static str;
 }
 
+/// The trait for building logical properties for a plan node.
 pub trait LogicalPropertyBuilder<T: NodeType>: 'static + Send + Sync + Sized {
     type Prop: LogicalProperty + Sized + Clone;
 

diff --git a/optd-core/src/physical_property.rs b/optd-core/src/physical_property.rs
@@ -12,11 +12,16 @@ use itertools::Itertools;
 
 use crate::nodes::{ArcPredNode, NodeType, PlanNode, PlanNodeOrGroup};
 
+/// The trait enables we store any physical property in the memo table by erasing the concrete type.
+/// In the future, we can implement `serialize`/`deserialize` on this trait so that we can serialize
+/// the physical properties.
 pub trait PhysicalProperty: 'static + Any + Send + Sync + Debug + Display {
     fn as_any(&self) -> &dyn Any;
     fn to_boxed(&self) -> Box<dyn PhysicalProperty>;
 }
 
+/// A wrapper around the `PhysicalPropertyBuilder` so that we can erase the concrete type and store
+/// it safely in the memo table.
 pub trait PhysicalPropertyBuilderAny<T: NodeType>: 'static + Send + Sync {
     fn derive_any(
         &self,
@@ -41,6 +46,7 @@ pub trait PhysicalPropertyBuilderAny<T: NodeType>: 'static + Send + Sync {
     fn property_name(&self) -> &'static str;
 }
 
+/// The trait for building physical properties for a plan node.
 pub trait PhysicalPropertyBuilder<T: NodeType>: 'static + Send + Sync + Sized {
     type Prop: PhysicalProperty + Clone + Sized;
 

diff --git a/optd-datafusion-repr-adv-cost/src/lib.rs b/optd-datafusion-repr-adv-cost/src/lib.rs
@@ -61,18 +61,11 @@ impl CostModel<DfNodeType, NaiveMemo<DfNodeType>> for AdvancedCostModel {
         node: &DfNodeType,
         predicates: &[ArcDfPredNode],
         children_stats: &[Option<&Statistics>],
-        children_costs: &[Cost],
         context: Option<RelNodeContext>,
         optimizer: Option<&CascadesOptimizer<DfNodeType>>,
     ) -> Cost {
-        self.base_model.compute_operation_cost(
-            node,
-            predicates,
-            children_stats,
-            children_costs,
-            context,
-            optimizer,
-        )
+        self.base_model
+            .compute_operation_cost(node, predicates, children_stats, context, optimizer)
     }
 
     fn derive_statistics(

diff --git a/optd-datafusion-repr/src/cost/adaptive_cost.rs b/optd-datafusion-repr/src/cost/adaptive_cost.rs
@@ -67,22 +67,15 @@ impl CostModel<DfNodeType, NaiveMemo<DfNodeType>> for AdaptiveCostModel {
         node: &DfNodeType,
         predicates: &[ArcDfPredNode],
         children: &[Option<&Statistics>],
-        children_cost: &[Cost],
         context: Option<RelNodeContext>,
         optimizer: Option<&CascadesOptimizer<DfNodeType>>,
     ) -> Cost {
         if let DfNodeType::PhysicalScan = node {
             let row_cnt = self.get_row_cnt(&context);
             return DfCostModel::cost(0.0, row_cnt);
         }
-        self.base_model.compute_operation_cost(
-            node,
-            predicates,
-            children,
-            children_cost,
-            context,
-            optimizer,
-        )
+        self.base_model
+            .compute_operation_cost(node, predicates, children, context, optimizer)
     }
 
     fn derive_statistics(

diff --git a/optd-datafusion-repr/src/cost/base_cost.rs b/optd-datafusion-repr/src/cost/base_cost.rs
@@ -132,7 +132,6 @@ impl CostModel<DfNodeType, NaiveMemo<DfNodeType>> for DfCostModel {
         node: &DfNodeType,
         predicates: &[ArcDfPredNode],
         children: &[Option<&Statistics>],
-        _: &[Cost],
         _context: Option<RelNodeContext>,
         _optimizer: Option<&CascadesOptimizer<DfNodeType>>,
     ) -> Cost {

diff --git a/optd-datafusion-repr/src/testing/dummy_cost.rs b/optd-datafusion-repr/src/testing/dummy_cost.rs
@@ -19,7 +19,6 @@ impl CostModel<DfNodeType, NaiveMemo<DfNodeType>> for DummyCostModel {
         _: &DfNodeType,
         _: &[ArcDfPredNode],
         _: &[Option<&Statistics>],
-        _: &[Cost],
         _: Option<RelNodeContext>,
         _: Option<&CascadesOptimizer<DfNodeType>>,
     ) -> Cost {