diff --git a/optd-datafusion-repr/src/plan_nodes.rs b/optd-datafusion-repr/src/plan_nodes.rs index 91c3bd16..cfd29f0a 100644 --- a/optd-datafusion-repr/src/plan_nodes.rs +++ b/optd-datafusion-repr/src/plan_nodes.rs @@ -208,8 +208,17 @@ pub trait OptRelNode: 'static + Clone { } } -pub trait ExplainData: OptRelNode { - fn explain_data(data: &Value) -> Vec<(&'static str, Pretty<'static>)>; +/// Plan nodes that are defined through `define_plan_node` macro with data +/// field should implement this trait. Since data is stored as `Value` in +/// `RelNode`, we need to now how to convert between `Value` and the actual +/// data type. +/// +/// For reasons why `explain_data`` needs to be explicitly implemented, see +/// `define_plan_node`. +pub trait ExplainData: OptRelNode { + fn data_to_value(data: &T) -> Value; + fn value_to_data(value: &Value) -> T; + fn explain_data(data: &T) -> Vec<(&'static str, Pretty<'static>)>; } #[derive(Clone, Debug)] diff --git a/optd-datafusion-repr/src/plan_nodes/macros.rs b/optd-datafusion-repr/src/plan_nodes/macros.rs index 13a43436..df394932 100644 --- a/optd-datafusion-repr/src/plan_nodes/macros.rs +++ b/optd-datafusion-repr/src/plan_nodes/macros.rs @@ -1,21 +1,11 @@ -/// Plan nodes with data fields must implement `ExplainData` trait. An example: +/// Plan nodes with data fields must implement `ExplainData` trait. /// -/// ```ignore -/// #[derive(Clone, Debug)] -/// struct PhysicalDummy(PlanNode); -/// -/// // Implement `OptRelNode` using `define_plan_node!`... -/// -/// impl ExplainData for PhysicalDummy { -/// fn explain_data(data: &Value) -> Vec<(&'static str, Pretty<'static>)> { -/// if let Value::Int32(i) = data { -/// vec![("primitive_data", i.to_string().into())] -/// } else { -/// unreachable!() -/// } -/// } -/// } -/// ``` +/// The generated `dispatch_explain` method delegates explaining data to +/// rel node implementations of `ExplainData` trait instead of just debug +/// printing it, because for complex data type (struct), derived debug printing +/// displays struct name which should be hidden from the user. It also wraps +/// the fields in braces, unlike the rest of the fields as children. + macro_rules! define_plan_node { ( $struct_name:ident : $meta_typ:tt, @@ -23,7 +13,7 @@ macro_rules! define_plan_node { [ $({ $child_id:literal, $child_name:ident : $child_meta_typ:ty }),* ] , [ $({ $attr_id:literal, $attr_name:ident : $attr_meta_typ:ty }),* ] $(, { $inner_name:ident : $inner_typ:ty })? - $(, $data_name:ident)? + $(, $data_name:ident: $data_typ: ty )? ) => { impl OptRelNode for $struct_name { fn into_rel_node(self) -> OptRelNodeRef { @@ -49,7 +39,7 @@ macro_rules! define_plan_node { if let Some(meta_map) = meta_map { fields = fields.with_meta(self.0.get_meta(meta_map)); }; - define_plan_node!(@expand_fields self, $struct_name, fields $(, $data_name)?); + define_plan_node!(@expand_data_fields self, $struct_name, fields $(, $data_name)?); pretty_xmlish::Pretty::simple_record( stringify!($struct_name), @@ -65,13 +55,13 @@ macro_rules! define_plan_node { pub fn new( $($child_name : $child_meta_typ,)* $($attr_name : $attr_meta_typ),* - $($data_name: Value)? + $($data_name: $data_typ)? $(, $inner_name : $inner_typ)? ) -> $struct_name { #[allow(unused_mut, unused)] let mut data = None; $( - data = Some($data_name); + data = Some($struct_name::data_to_value(&$data_name)); )* $struct_name($meta_typ( optd_core::rel_node::RelNode { @@ -111,11 +101,11 @@ macro_rules! define_plan_node { } }; // Dummy branch that does nothing when data is `None`. - (@expand_fields $self:ident, $struct_name:ident, $fields:ident) => {}; + (@expand_data_fields $self:ident, $struct_name:ident, $fields:ident) => {}; // Expand explain fields with data. - (@expand_fields $self:ident, $struct_name:ident, $fields:ident, $data_name:ident) => { - let data = $self.0 .0.data.as_ref().unwrap(); - $fields.extend($struct_name::explain_data(data)); + (@expand_data_fields $self:ident, $struct_name:ident, $fields:ident, $data_name:ident) => { + let value = $self.0 .0.data.as_ref().unwrap(); + $fields.extend($struct_name::explain_data(&$struct_name::value_to_data(&value))); }; } @@ -151,40 +141,42 @@ mod test { #[derive(Clone, Debug)] struct PhysicalComplexDummy(PlanNode); - impl ExplainData for PhysicalComplexDummy { - fn explain_data(data: &Value) -> Vec<(&'static str, Pretty<'static>)> { - if let Value::Serialized(serialized_data) = data { - let data: ComplexData = bincode::deserialize(serialized_data).unwrap(); - vec![ - ("a", data.a.to_string().into()), - ("b", data.b.to_string().into()), - ] - } else { - unreachable!() - } - } - } - define_plan_node!( PhysicalComplexDummy: PlanNode, PhysicalScan, [ { 0, child: PlanNode } ], [ ], - complex_data + complex_data: ComplexData ); + impl ExplainData for PhysicalComplexDummy { + fn data_to_value(data: &ComplexData) -> Value { + Value::Serialized(bincode::serialize(data).unwrap().into_iter().collect()) + } + + fn value_to_data(value: &Value) -> ComplexData { + if let Value::Serialized(serialized_data) = value { + bincode::deserialize(serialized_data).unwrap() + } else { + unreachable!() + } + } + + fn explain_data(data: &ComplexData) -> Vec<(&'static str, Pretty<'static>)> { + vec![ + ("a", data.a.to_string().into()), + ("b", data.b.to_string().into()), + ] + } + } + let node = PhysicalComplexDummy::new( LogicalScan::new("a".to_string()).0, - Value::Serialized( - bincode::serialize(&ComplexData { - a: 1, - b: "a".to_string(), - }) - .unwrap() - .into_iter() - .collect(), - ), + ComplexData { + a: 1, + b: "a".to_string(), + }, ); let pretty = node.dispatch_explain(None); println!("{}", get_explain_str(&pretty));