From c98c99ab8fa9c51ef3736abbe7cc056422613ce6 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 8 Jul 2024 15:44:06 -0600 Subject: [PATCH] chore: Rename UserDefinedSQLPlanner to ExprPlanner (#11338) * rename UserDefinedSQLPlanner to ExprPlanner * Clippy --------- Co-authored-by: Andrew Lamb --- datafusion/core/src/execution/context/mod.rs | 12 +++++------ .../core/src/execution/session_state.rs | 21 +++++++++---------- ...defined_sql_planner.rs => expr_planner.rs} | 6 +++--- datafusion/core/tests/user_defined/mod.rs | 4 ++-- .../user_defined_scalar_functions.rs | 4 ++-- datafusion/execution/src/task.rs | 4 ++-- datafusion/expr/src/planner.rs | 12 +++++------ datafusion/expr/src/registry.rs | 16 +++++++------- datafusion/functions-array/src/planner.rs | 6 +++--- datafusion/functions/src/core/planner.rs | 4 ++-- datafusion/functions/src/planner.rs | 4 ++-- datafusion/proto/src/bytes/mod.rs | 4 ++-- datafusion/proto/src/bytes/registry.rs | 4 ++-- datafusion/sql/src/planner.rs | 9 +++----- 14 files changed, 52 insertions(+), 58 deletions(-) rename datafusion/core/tests/user_defined/{user_defined_sql_planner.rs => expr_planner.rs} (95%) diff --git a/datafusion/core/src/execution/context/mod.rs b/datafusion/core/src/execution/context/mod.rs index 04debf498aa9..4b9e3e843341 100644 --- a/datafusion/core/src/execution/context/mod.rs +++ b/datafusion/core/src/execution/context/mod.rs @@ -60,7 +60,7 @@ use datafusion_execution::registry::SerializerRegistry; use datafusion_expr::{ expr_rewriter::FunctionRewrite, logical_plan::{DdlStatement, Statement}, - planner::UserDefinedSQLPlanner, + planner::ExprPlanner, Expr, UserDefinedLogicalNode, WindowUDF, }; @@ -1392,17 +1392,15 @@ impl FunctionRegistry for SessionContext { self.state.write().register_function_rewrite(rewrite) } - fn expr_planners(&self) -> Vec> { + fn expr_planners(&self) -> Vec> { self.state.read().expr_planners() } - fn register_user_defined_sql_planner( + fn register_expr_planner( &mut self, - user_defined_sql_planner: Arc, + expr_planner: Arc, ) -> Result<()> { - self.state - .write() - .register_user_defined_sql_planner(user_defined_sql_planner) + self.state.write().register_expr_planner(expr_planner) } } diff --git a/datafusion/core/src/execution/session_state.rs b/datafusion/core/src/execution/session_state.rs index f8a90230b38f..af4144b32776 100644 --- a/datafusion/core/src/execution/session_state.rs +++ b/datafusion/core/src/execution/session_state.rs @@ -60,7 +60,7 @@ use datafusion_execution::runtime_env::RuntimeEnv; use datafusion_execution::TaskContext; use datafusion_expr::execution_props::ExecutionProps; use datafusion_expr::expr_rewriter::FunctionRewrite; -use datafusion_expr::planner::UserDefinedSQLPlanner; +use datafusion_expr::planner::ExprPlanner; use datafusion_expr::registry::{FunctionRegistry, SerializerRegistry}; use datafusion_expr::simplify::SimplifyInfo; use datafusion_expr::var_provider::{is_system_variables, VarType}; @@ -101,7 +101,7 @@ pub struct SessionState { /// Responsible for analyzing and rewrite a logical plan before optimization analyzer: Analyzer, /// Provides support for customising the SQL planner, e.g. to add support for custom operators like `->>` or `?` - user_defined_sql_planners: Vec>, + expr_planners: Vec>, /// Responsible for optimizing a logical plan optimizer: Optimizer, /// Responsible for optimizing a physical execution plan @@ -231,7 +231,7 @@ impl SessionState { ); } - let user_defined_sql_planners: Vec> = vec![ + let expr_planners: Vec> = vec![ Arc::new(functions::core::planner::CoreFunctionPlanner::default()), // register crate of array expressions (if enabled) #[cfg(feature = "array_expressions")] @@ -248,7 +248,7 @@ impl SessionState { let mut new_self = SessionState { session_id, analyzer: Analyzer::new(), - user_defined_sql_planners, + expr_planners, optimizer: Optimizer::new(), physical_optimizers: PhysicalOptimizer::new(), query_planner: Arc::new(DefaultQueryPlanner {}), @@ -970,7 +970,7 @@ impl SessionState { let mut query = SqlToRel::new_with_options(provider, self.get_parser_options()); // custom planners are registered first, so they're run first and take precedence over built-in planners - for planner in self.user_defined_sql_planners.iter() { + for planner in self.expr_planners.iter() { query = query.with_user_defined_planner(planner.clone()); } @@ -1186,16 +1186,15 @@ impl FunctionRegistry for SessionState { Ok(()) } - fn expr_planners(&self) -> Vec> { - self.user_defined_sql_planners.clone() + fn expr_planners(&self) -> Vec> { + self.expr_planners.clone() } - fn register_user_defined_sql_planner( + fn register_expr_planner( &mut self, - user_defined_sql_planner: Arc, + expr_planner: Arc, ) -> datafusion_common::Result<()> { - self.user_defined_sql_planners - .push(user_defined_sql_planner); + self.expr_planners.push(expr_planner); Ok(()) } } diff --git a/datafusion/core/tests/user_defined/user_defined_sql_planner.rs b/datafusion/core/tests/user_defined/expr_planner.rs similarity index 95% rename from datafusion/core/tests/user_defined/user_defined_sql_planner.rs rename to datafusion/core/tests/user_defined/expr_planner.rs index 6b660e15a197..1b23bf9ab2ef 100644 --- a/datafusion/core/tests/user_defined/user_defined_sql_planner.rs +++ b/datafusion/core/tests/user_defined/expr_planner.rs @@ -26,12 +26,12 @@ use datafusion::prelude::*; use datafusion::sql::sqlparser::ast::BinaryOperator; use datafusion_common::ScalarValue; use datafusion_expr::expr::Alias; -use datafusion_expr::planner::{PlannerResult, RawBinaryExpr, UserDefinedSQLPlanner}; +use datafusion_expr::planner::{ExprPlanner, PlannerResult, RawBinaryExpr}; use datafusion_expr::BinaryExpr; struct MyCustomPlanner; -impl UserDefinedSQLPlanner for MyCustomPlanner { +impl ExprPlanner for MyCustomPlanner { fn plan_binary_op( &self, expr: RawBinaryExpr, @@ -68,7 +68,7 @@ async fn plan_and_collect(sql: &str) -> Result> { let config = SessionConfig::new().set_str("datafusion.sql_parser.dialect", "postgres"); let mut ctx = SessionContext::new_with_config(config); - ctx.register_user_defined_sql_planner(Arc::new(MyCustomPlanner))?; + ctx.register_expr_planner(Arc::new(MyCustomPlanner))?; ctx.sql(sql).await?.collect().await } diff --git a/datafusion/core/tests/user_defined/mod.rs b/datafusion/core/tests/user_defined/mod.rs index 9b83a9fdd408..56cec8df468b 100644 --- a/datafusion/core/tests/user_defined/mod.rs +++ b/datafusion/core/tests/user_defined/mod.rs @@ -30,5 +30,5 @@ mod user_defined_window_functions; /// Tests for User Defined Table Functions mod user_defined_table_functions; -/// Tests for User Defined SQL Planner -mod user_defined_sql_planner; +/// Tests for Expression Planner +mod expr_planner; diff --git a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs index ae8a009c6292..1733068debb9 100644 --- a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs +++ b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs @@ -575,9 +575,9 @@ async fn test_user_defined_functions_cast_to_i64() -> Result<()> { async fn test_user_defined_sql_functions() -> Result<()> { let ctx = SessionContext::new(); - let sql_planners = ctx.expr_planners(); + let expr_planners = ctx.expr_planners(); - assert!(!sql_planners.is_empty()); + assert!(!expr_planners.is_empty()); Ok(()) } diff --git a/datafusion/execution/src/task.rs b/datafusion/execution/src/task.rs index 24d61e6a8b72..df7fd0dbd92c 100644 --- a/datafusion/execution/src/task.rs +++ b/datafusion/execution/src/task.rs @@ -27,7 +27,7 @@ use crate::{ runtime_env::{RuntimeConfig, RuntimeEnv}, }; use datafusion_common::{plan_datafusion_err, DataFusionError, Result}; -use datafusion_expr::planner::UserDefinedSQLPlanner; +use datafusion_expr::planner::ExprPlanner; use datafusion_expr::{AggregateUDF, ScalarUDF, WindowUDF}; /// Task Execution Context @@ -192,7 +192,7 @@ impl FunctionRegistry for TaskContext { Ok(self.scalar_functions.insert(udf.name().into(), udf)) } - fn expr_planners(&self) -> Vec> { + fn expr_planners(&self) -> Vec> { vec![] } } diff --git a/datafusion/expr/src/planner.rs b/datafusion/expr/src/planner.rs index c255edbea5ae..aeb8ed8372b7 100644 --- a/datafusion/expr/src/planner.rs +++ b/datafusion/expr/src/planner.rs @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -//! [`ContextProvider`] and [`UserDefinedSQLPlanner`] APIs to customize SQL query planning +//! [`ContextProvider`] and [`ExprPlanner`] APIs to customize SQL query planning use std::sync::Arc; @@ -83,7 +83,7 @@ pub trait ContextProvider { } /// This trait allows users to customize the behavior of the SQL planner -pub trait UserDefinedSQLPlanner: Send + Sync { +pub trait ExprPlanner: Send + Sync { /// Plan the binary operation between two expressions, returns original /// BinaryExpr if not possible fn plan_binary_op( @@ -168,7 +168,7 @@ pub trait UserDefinedSQLPlanner: Send + Sync { /// Note `left` and `right` are DataFusion [`Expr`]s but the `op` is the SQL AST /// operator. /// -/// This structure is used by [`UserDefinedSQLPlanner`] to plan operators with +/// This structure is used by [`ExprPlanner`] to plan operators with /// custom expressions. #[derive(Debug, Clone)] pub struct RawBinaryExpr { @@ -179,7 +179,7 @@ pub struct RawBinaryExpr { /// An expression with GetFieldAccess to plan /// -/// This structure is used by [`UserDefinedSQLPlanner`] to plan operators with +/// This structure is used by [`ExprPlanner`] to plan operators with /// custom expressions. #[derive(Debug, Clone)] pub struct RawFieldAccessExpr { @@ -189,7 +189,7 @@ pub struct RawFieldAccessExpr { /// A Dictionary literal expression `{ key: value, ...}` /// -/// This structure is used by [`UserDefinedSQLPlanner`] to plan operators with +/// This structure is used by [`ExprPlanner`] to plan operators with /// custom expressions. #[derive(Debug, Clone)] pub struct RawDictionaryExpr { @@ -197,7 +197,7 @@ pub struct RawDictionaryExpr { pub values: Vec, } -/// Result of planning a raw expr with [`UserDefinedSQLPlanner`] +/// Result of planning a raw expr with [`ExprPlanner`] #[derive(Debug, Clone)] pub enum PlannerResult { /// The raw expression was successfully planned as a new [`Expr`] diff --git a/datafusion/expr/src/registry.rs b/datafusion/expr/src/registry.rs index 6a27c05bb451..988dc0f5aeda 100644 --- a/datafusion/expr/src/registry.rs +++ b/datafusion/expr/src/registry.rs @@ -18,7 +18,7 @@ //! FunctionRegistry trait use crate::expr_rewriter::FunctionRewrite; -use crate::planner::UserDefinedSQLPlanner; +use crate::planner::ExprPlanner; use crate::{AggregateUDF, ScalarUDF, UserDefinedLogicalNode, WindowUDF}; use datafusion_common::{not_impl_err, plan_datafusion_err, Result}; use std::collections::HashMap; @@ -110,15 +110,15 @@ pub trait FunctionRegistry { not_impl_err!("Registering FunctionRewrite") } - /// Set of all registered [`UserDefinedSQLPlanner`]s - fn expr_planners(&self) -> Vec>; + /// Set of all registered [`ExprPlanner`]s + fn expr_planners(&self) -> Vec>; - /// Registers a new [`UserDefinedSQLPlanner`] with the registry. - fn register_user_defined_sql_planner( + /// Registers a new [`ExprPlanner`] with the registry. + fn register_expr_planner( &mut self, - _user_defined_sql_planner: Arc, + _expr_planner: Arc, ) -> Result<()> { - not_impl_err!("Registering UserDefinedSQLPlanner") + not_impl_err!("Registering ExprPlanner") } } @@ -196,7 +196,7 @@ impl FunctionRegistry for MemoryFunctionRegistry { Ok(self.udwfs.insert(udaf.name().into(), udaf)) } - fn expr_planners(&self) -> Vec> { + fn expr_planners(&self) -> Vec> { vec![] } } diff --git a/datafusion/functions-array/src/planner.rs b/datafusion/functions-array/src/planner.rs index 01853fb56908..cfbe99b4b7fd 100644 --- a/datafusion/functions-array/src/planner.rs +++ b/datafusion/functions-array/src/planner.rs @@ -19,7 +19,7 @@ use datafusion_common::{utils::list_ndims, DFSchema, Result}; use datafusion_expr::{ - planner::{PlannerResult, RawBinaryExpr, RawFieldAccessExpr, UserDefinedSQLPlanner}, + planner::{ExprPlanner, PlannerResult, RawBinaryExpr, RawFieldAccessExpr}, sqlparser, AggregateFunction, Expr, ExprSchemable, GetFieldAccess, }; use datafusion_functions::expr_fn::get_field; @@ -34,7 +34,7 @@ use crate::{ pub struct ArrayFunctionPlanner; -impl UserDefinedSQLPlanner for ArrayFunctionPlanner { +impl ExprPlanner for ArrayFunctionPlanner { fn plan_binary_op( &self, expr: RawBinaryExpr, @@ -101,7 +101,7 @@ impl UserDefinedSQLPlanner for ArrayFunctionPlanner { pub struct FieldAccessPlanner; -impl UserDefinedSQLPlanner for FieldAccessPlanner { +impl ExprPlanner for FieldAccessPlanner { fn plan_field_access( &self, expr: RawFieldAccessExpr, diff --git a/datafusion/functions/src/core/planner.rs b/datafusion/functions/src/core/planner.rs index e803c92dd0b3..748b598d292f 100644 --- a/datafusion/functions/src/core/planner.rs +++ b/datafusion/functions/src/core/planner.rs @@ -18,7 +18,7 @@ use datafusion_common::DFSchema; use datafusion_common::Result; use datafusion_expr::expr::ScalarFunction; -use datafusion_expr::planner::{PlannerResult, RawDictionaryExpr, UserDefinedSQLPlanner}; +use datafusion_expr::planner::{ExprPlanner, PlannerResult, RawDictionaryExpr}; use datafusion_expr::Expr; use super::named_struct; @@ -26,7 +26,7 @@ use super::named_struct; #[derive(Default)] pub struct CoreFunctionPlanner {} -impl UserDefinedSQLPlanner for CoreFunctionPlanner { +impl ExprPlanner for CoreFunctionPlanner { fn plan_dictionary_literal( &self, expr: RawDictionaryExpr, diff --git a/datafusion/functions/src/planner.rs b/datafusion/functions/src/planner.rs index 41ff92f26111..ad42c5edd6e6 100644 --- a/datafusion/functions/src/planner.rs +++ b/datafusion/functions/src/planner.rs @@ -20,14 +20,14 @@ use datafusion_common::Result; use datafusion_expr::{ expr::ScalarFunction, - planner::{PlannerResult, UserDefinedSQLPlanner}, + planner::{ExprPlanner, PlannerResult}, Expr, }; #[derive(Default)] pub struct UserDefinedFunctionPlanner; -impl UserDefinedSQLPlanner for UserDefinedFunctionPlanner { +impl ExprPlanner for UserDefinedFunctionPlanner { #[cfg(feature = "datetime_expressions")] fn plan_extract(&self, args: Vec) -> Result>> { Ok(PlannerResult::Planned(Expr::ScalarFunction( diff --git a/datafusion/proto/src/bytes/mod.rs b/datafusion/proto/src/bytes/mod.rs index 83210cb4e41f..9188480431aa 100644 --- a/datafusion/proto/src/bytes/mod.rs +++ b/datafusion/proto/src/bytes/mod.rs @@ -39,7 +39,7 @@ use std::sync::Arc; use datafusion::execution::registry::FunctionRegistry; use datafusion::physical_plan::ExecutionPlan; use datafusion::prelude::SessionContext; -use datafusion_expr::planner::UserDefinedSQLPlanner; +use datafusion_expr::planner::ExprPlanner; mod registry; @@ -167,7 +167,7 @@ impl Serializeable for Expr { ) } - fn expr_planners(&self) -> Vec> { + fn expr_planners(&self) -> Vec> { vec![] } } diff --git a/datafusion/proto/src/bytes/registry.rs b/datafusion/proto/src/bytes/registry.rs index 075993e2ba76..eae2425f8ac1 100644 --- a/datafusion/proto/src/bytes/registry.rs +++ b/datafusion/proto/src/bytes/registry.rs @@ -20,7 +20,7 @@ use std::{collections::HashSet, sync::Arc}; use datafusion::execution::registry::FunctionRegistry; use datafusion_common::plan_err; use datafusion_common::Result; -use datafusion_expr::planner::UserDefinedSQLPlanner; +use datafusion_expr::planner::ExprPlanner; use datafusion_expr::{AggregateUDF, ScalarUDF, WindowUDF}; /// A default [`FunctionRegistry`] registry that does not resolve any @@ -56,7 +56,7 @@ impl FunctionRegistry for NoRegistry { plan_err!("No function registry provided to deserialize, so can not deserialize User Defined Window Function '{}'", udwf.inner().name()) } - fn expr_planners(&self) -> Vec> { + fn expr_planners(&self) -> Vec> { vec![] } } diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs index 1d7225a08a43..bb8c2b30c216 100644 --- a/datafusion/sql/src/planner.rs +++ b/datafusion/sql/src/planner.rs @@ -24,7 +24,7 @@ use arrow_schema::*; use datafusion_common::{ field_not_found, internal_err, plan_datafusion_err, DFSchemaRef, SchemaError, }; -use datafusion_expr::planner::UserDefinedSQLPlanner; +use datafusion_expr::planner::ExprPlanner; use sqlparser::ast::{ArrayElemTypeDef, ExactNumberInfo}; use sqlparser::ast::{ColumnDef as SQLColumnDef, ColumnOption}; use sqlparser::ast::{DataType as SQLDataType, Ident, ObjectName, TableAlias}; @@ -216,7 +216,7 @@ pub struct SqlToRel<'a, S: ContextProvider> { pub(crate) ident_normalizer: IdentNormalizer, pub(crate) value_normalizer: ValueNormalizer, /// user defined planner extensions - pub(crate) planners: Vec>, + pub(crate) planners: Vec>, } impl<'a, S: ContextProvider> SqlToRel<'a, S> { @@ -226,10 +226,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { } /// add an user defined planner - pub fn with_user_defined_planner( - mut self, - planner: Arc, - ) -> Self { + pub fn with_user_defined_planner(mut self, planner: Arc) -> Self { self.planners.push(planner); self }