alamb commented on code in PR #5521:
URL: https://github.com/apache/arrow-datafusion/pull/5521#discussion_r1131407108


##########
datafusion/expr/src/logical_plan/extension.rs:
##########
@@ -17,16 +17,15 @@
 
 //! This module defines the interface for logical nodes
 use crate::{Expr, LogicalPlan};
-use datafusion_common::DFSchemaRef;
+use datafusion_common::{DFSchema, DFSchemaRef};
 use std::hash::{Hash, Hasher};
 use std::{any::Any, cmp::Eq, collections::HashSet, fmt, sync::Arc};
 
-/// This defines the interface for `LogicalPlan` nodes that can be
+/// This defines the interface for [`LogicalPlan`] nodes that can be
 /// used to extend DataFusion with custom relational operators.
 ///
-/// See the example in
-/// [user_defined_plan.rs](../../tests/user_defined_plan.rs) for an
-/// example of how to use this extension API.
+/// Trait [`UserDefinedLogicalNodeCore`] is *the recommended way to implement*

Review Comment:
   👍 



##########
datafusion/expr/src/logical_plan/extension.rs:
##########
@@ -178,3 +173,109 @@ impl std::cmp::PartialEq for dyn UserDefinedLogicalNode {
 }
 
 impl Eq for dyn UserDefinedLogicalNode {}
+
+/// This trait facilitates implementation of the [`UserDefinedLogicalNode`].
+///
+/// See the example in
+/// [user_defined_plan.rs](../../tests/user_defined_plan.rs) for an
+/// example of how to use this extension API.
+pub trait UserDefinedLogicalNodeCore:
+    fmt::Debug + Eq + Hash + Send + Sync + 'static
+{
+    /// Return the plan's name.
+    fn name(&self) -> &str;
+
+    /// Return the logical plan's inputs.
+    fn inputs(&self) -> Vec<&LogicalPlan>;
+
+    /// Return the output schema of this logical plan node.
+    fn schema(&self) -> &DFSchemaRef;
+
+    /// Returns all expressions in the current logical plan node. This
+    /// should not include expressions of any inputs (aka
+    /// non-recursively). These expressions are used for optimizer
+    /// passes and rewrites.
+    fn expressions(&self) -> Vec<Expr>;
+
+    /// A list of output columns (e.g. the names of columns in
+    /// self.schema()) for which predicates can not be pushed below
+    /// this node without changing the output.
+    ///
+    /// By default, this returns all columns and thus prevents any
+    /// predicates from being pushed below this node.
+    fn prevent_predicate_push_down_columns(&self) -> HashSet<String> {
+        // default (safe) is all columns in the schema.
+        get_all_columns_from_schema(self.schema())
+    }
+
+    /// Write a single line, human readable string to `f` for use in explain 
plan.
+    ///
+    /// For example: `TopK: k=10`
+    fn fmt_for_explain(&self, f: &mut fmt::Formatter) -> fmt::Result;
+
+    /// Create a new `ExtensionPlanNode` with the specified children
+    /// and expressions. This function is used during optimization
+    /// when the plan is being rewritten and a new instance of the
+    /// `ExtensionPlanNode` must be created.
+    ///
+    /// Note that exprs and inputs are in the same order as the result
+    /// of self.inputs and self.exprs.
+    ///
+    /// So, `self.from_template(exprs, ..).expressions() == exprs
+    #[allow(clippy::wrong_self_convention)]
+    fn from_template(&self, exprs: &[Expr], inputs: &[LogicalPlan]) -> Self;
+}
+
+impl<T: UserDefinedLogicalNodeCore> UserDefinedLogicalNode for T {

Review Comment:
   ```suggestion
   /// Automatically derive UserDefinedLogicalNode to `UserDefinedLogicalNode` 
   /// to avoid boiler plate for implementing `as_any`, `Hash` and `PartialEq` 
   impl<T: UserDefinedLogicalNodeCore> UserDefinedLogicalNode for T {
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to