refine docs

Signed-off-by: Bugen Zhao <[email protected]>

src/frontend/src/optimizer/plan_node/mod.rs

@@ -62,7 +62,6 @@ pub trait ConventionMarker: 'static + Sized {
 
     fn value() -> Convention;
 }
-pub trait PhysicalConventionMarker: ConventionMarker {}
 
 pub struct Logical;
 impl ConventionMarker for Logical {
@@ -81,7 +80,6 @@ impl ConventionMarker for Batch {
         Convention::Batch
     }
 }
-impl PhysicalConventionMarker for Batch {}
 
 pub struct Stream;
 impl ConventionMarker for Stream {
@@ -91,7 +89,6 @@ impl ConventionMarker for Stream {
         Convention::Stream
     }
 }
-impl PhysicalConventionMarker for Stream {}
 
 pub trait StaticPlanNodeMeta {
     type Convention: ConventionMarker;
@@ -104,7 +101,7 @@ pub trait StaticPlanNodeMeta {
 
 impl<P> PlanNodeMeta for P
 where
-    P: StaticPlanNodeMeta + 'static,
+    P: StaticPlanNodeMeta,
 {
     fn node_type(&self) -> PlanNodeType {
         P::NODE_TYPE

src/frontend/src/optimizer/plan_node/plan_base.rs

@@ -24,19 +24,27 @@ use crate::optimizer::property::{Distribution, FunctionalDependencySet, Order};
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct NoExtra;
 
-/// Common extra fields for physical plan nodes.
-#[derive(Clone, Debug, PartialEq, Eq, Hash)]
-struct PhysicalCommonExtra {
-    /// The distribution property of the PlanNode's output, store an `Distribution::any()` here
-    /// will not affect correctness, but insert unnecessary exchange in plan
-    dist: Distribution,
-}
+// Make them public types in a private module to allow using them as public trait bounds,
+// while still keeping them private to the super module.
+mod physical_common {
+    use super::*;
+
+    /// Common extra fields for physical plan nodes.
+    #[derive(Clone, Debug, PartialEq, Eq, Hash)]
+    pub struct PhysicalCommonExtra {
+        /// The distribution property of the PlanNode's output, store an `Distribution::any()` here
+        /// will not affect correctness, but insert unnecessary exchange in plan
+        pub dist: Distribution,
+    }
 
-trait GetPhysicalCommon {
-    fn physical(&self) -> &PhysicalCommonExtra;
-    fn physical_mut(&mut self) -> &mut PhysicalCommonExtra;
+    pub trait GetPhysicalCommon {
+        fn physical(&self) -> &PhysicalCommonExtra;
+        fn physical_mut(&mut self) -> &mut PhysicalCommonExtra;
+    }
 }
 
+use physical_common::*;
+
 /// Extra fields for stream plan nodes.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct StreamExtra {
@@ -277,25 +285,16 @@ impl PlanBase<Batch> {
     }
 }
 
-impl PlanBase<Batch> {
-    /// Clone the plan node with a new distribution.
-    ///
-    /// Panics if the plan node is not physical.
-    pub fn clone_with_new_distribution(&self, dist: Distribution) -> Self {
-        let mut new = self.clone();
-        new.extra.physical.dist = dist;
-        new
-    }
-}
-
-// TODO: unify the impls for `PlanBase<Stream>` and `PlanBase<Batch>`.
-impl PlanBase<Stream> {
+impl<C: ConventionMarker> PlanBase<C>
+where
+    C::Extra: GetPhysicalCommon,
+{
     /// Clone the plan node with a new distribution.
     ///
     /// Panics if the plan node is not physical.
     pub fn clone_with_new_distribution(&self, dist: Distribution) -> Self {
         let mut new = self.clone();
-        new.extra.physical.dist = dist;
+        new.extra.physical_mut().dist = dist;
         new
     }
 }
@@ -315,18 +314,38 @@ pub enum PlanBaseRef<'a> {
     Batch(&'a PlanBase<Batch>),
 }
 
+impl PlanBaseRef<'_> {
+    pub fn convention(self) -> Convention {
+        match self {
+            PlanBaseRef::Logical(_) => Convention::Logical,
+            PlanBaseRef::Stream(_) => Convention::Stream,
+            PlanBaseRef::Batch(_) => Convention::Batch,
+        }
+    }
+}
+
+/// Dispatch a method call to the corresponding plan base type.
 macro_rules! dispatch_plan_base {
     ($self:ident, [$($convention:ident),+ $(,)?], $method:expr) => {
         match $self {
             $(
                 PlanBaseRef::$convention(plan) => $method(plan),
             )+
+
             #[allow(unreachable_patterns)]
-            _ => panic!() // TODO
+            _ => unreachable!("calling `{}` on a plan node of `{:?}`", stringify!($method), $self.convention()),
         }
     }
 }
 
+/// Workaround for getters returning references.
+///
+/// For example, callers writing `GenericPlanRef::schema(&foo.plan_base())` will lead to a
+/// borrow checker error, as it borrows [`PlanBaseRef`] again, which is already a reference.
+///
+/// As a workaround, we directly let the getters below take the ownership of [`PlanBaseRef`], when
+/// callers write `foo.plan_base().schema()`, the compiler will prefer these ones over the ones
+/// defined in traits like [`GenericPlanRef`].
 impl<'a> PlanBaseRef<'a> {
     pub(super) fn schema(self) -> &'a Schema {
         dispatch_plan_base!(self, [Logical, Stream, Batch], GenericPlanRef::schema)