diff --git a/datafusion/core/src/datasource/listing/helpers.rs b/datafusion/core/src/datasource/listing/helpers.rs index 427940d4b7ad..6032c49885c7 100644 --- a/datafusion/core/src/datasource/listing/helpers.rs +++ b/datafusion/core/src/datasource/listing/helpers.rs @@ -127,7 +127,7 @@ pub fn expr_applicable_for_cols(col_names: &[String], expr: &Expr) -> bool { | Expr::WindowFunction { .. } | Expr::Wildcard | Expr::QualifiedWildcard { .. } - | Expr::Placeholder { .. } => { + | Expr::Placeholder(_) => { is_applicable = false; VisitRecursion::Stop } diff --git a/datafusion/core/src/datasource/listing_table_factory.rs b/datafusion/core/src/datasource/listing_table_factory.rs index d61235445a3e..44595e5122c1 100644 --- a/datafusion/core/src/datasource/listing_table_factory.rs +++ b/datafusion/core/src/datasource/listing_table_factory.rs @@ -169,7 +169,7 @@ impl TableProviderFactory for ListingTableFactory { fn get_extension(path: &str) -> String { let res = Path::new(path).extension().and_then(|ext| ext.to_str()); match res { - Some(ext) => format!(".{}", ext), + Some(ext) => format!(".{ext}"), None => "".to_string(), } } diff --git a/datafusion/core/src/physical_plan/planner.rs b/datafusion/core/src/physical_plan/planner.rs index 7f99b4e57462..cc52739f6acb 100644 --- a/datafusion/core/src/physical_plan/planner.rs +++ b/datafusion/core/src/physical_plan/planner.rs @@ -345,7 +345,7 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result { Expr::QualifiedWildcard { .. } => Err(DataFusionError::Internal( "Create physical name does not support qualified wildcard".to_string(), )), - Expr::Placeholder { .. } => Err(DataFusionError::Internal( + Expr::Placeholder(_) => Err(DataFusionError::Internal( "Create physical name does not support placeholder".to_string(), )), Expr::OuterReferenceColumn(_, _) => Err(DataFusionError::Internal( diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 8b6562285b57..55bc71f1ab61 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -177,12 +177,7 @@ pub enum Expr { GroupingSet(GroupingSet), /// A place holder for parameters in a prepared statement /// (e.g. `$foo` or `$1`) - Placeholder { - /// The identifier of the parameter (e.g, $1 or $foo) - id: String, - /// The type the parameter will be filled in with - data_type: Option, - }, + Placeholder(Placeholder), /// A place holder which hold a reference to a qualified field /// in the outer query, used for correlated sub queries. OuterReferenceColumn(DataType, Column), @@ -562,6 +557,22 @@ impl InSubquery { } } +/// Placeholder +#[derive(Clone, PartialEq, Eq, Hash)] +pub struct Placeholder { + /// The identifier of the parameter (e.g, $1 or $foo) + pub id: String, + /// The type the parameter will be filled in with + pub data_type: Option, +} + +impl Placeholder { + /// Create a new Placeholder expression + pub fn new(id: String, data_type: Option) -> Self { + Self { id, data_type } + } +} + /// Grouping sets /// See /// for Postgres definition. @@ -666,7 +677,7 @@ impl Expr { Expr::Literal(..) => "Literal", Expr::Negative(..) => "Negative", Expr::Not(..) => "Not", - Expr::Placeholder { .. } => "Placeholder", + Expr::Placeholder(_) => "Placeholder", Expr::QualifiedWildcard { .. } => "QualifiedWildcard", Expr::ScalarFunction(..) => "ScalarFunction", Expr::ScalarSubquery { .. } => "ScalarSubquery", @@ -1172,7 +1183,7 @@ impl fmt::Debug for Expr { ) } }, - Expr::Placeholder { id, .. } => write!(f, "{id}"), + Expr::Placeholder(Placeholder { id, .. }) => write!(f, "{id}"), } } } @@ -1457,7 +1468,7 @@ fn create_name(e: &Expr) -> Result { Expr::QualifiedWildcard { .. } => Err(DataFusionError::Internal( "Create name does not support qualified wildcard".to_string(), )), - Expr::Placeholder { id, .. } => Ok((*id).to_string()), + Expr::Placeholder(Placeholder { id, .. }) => Ok((*id).to_string()), } } diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index 8b6afdec94aa..680a5cb7846b 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -18,7 +18,7 @@ use super::{Between, Expr, Like}; use crate::expr::{ AggregateFunction, AggregateUDF, BinaryExpr, Cast, GetIndexedField, InList, - InSubquery, ScalarFunction, ScalarUDF, Sort, TryCast, WindowFunction, + InSubquery, Placeholder, ScalarFunction, ScalarUDF, Sort, TryCast, WindowFunction, }; use crate::field_util::get_indexed_field; use crate::type_coercion::binary::get_result_type; @@ -62,7 +62,7 @@ impl ExprSchemable for Expr { fn get_type(&self, schema: &S) -> Result { match self { Expr::Alias(expr, name) => match &**expr { - Expr::Placeholder { data_type, .. } => match &data_type { + Expr::Placeholder(Placeholder { data_type, .. }) => match &data_type { None => schema.data_type(&Column::from_name(name)).cloned(), Some(dt) => Ok(dt.clone()), }, @@ -154,9 +154,13 @@ impl ExprSchemable for Expr { Expr::Like { .. } | Expr::ILike { .. } | Expr::SimilarTo { .. } => { Ok(DataType::Boolean) } - Expr::Placeholder { data_type, .. } => data_type.clone().ok_or_else(|| { - DataFusionError::Plan("Placeholder type could not be resolved".to_owned()) - }), + Expr::Placeholder(Placeholder { data_type, .. }) => { + data_type.clone().ok_or_else(|| { + DataFusionError::Plan( + "Placeholder type could not be resolved".to_owned(), + ) + }) + } Expr::Wildcard => { // Wildcard do not really have a type and do not appear in projections Ok(DataType::Null) @@ -231,7 +235,7 @@ impl ExprSchemable for Expr { | Expr::IsNotFalse(_) | Expr::IsNotUnknown(_) | Expr::Exists { .. } - | Expr::Placeholder { .. } => Ok(true), + | Expr::Placeholder(_) => Ok(true), Expr::InSubquery(InSubquery { expr, .. }) => expr.nullable(input_schema), Expr::ScalarSubquery(subquery) => { Ok(subquery.subquery.schema().field(0).is_nullable()) diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index fdd875ca84de..d95edb7e7c3a 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -15,8 +15,8 @@ // specific language governing permissions and limitations // under the License. -use crate::expr::Exists; use crate::expr::InSubquery; +use crate::expr::{Exists, Placeholder}; ///! Logical plan types use crate::logical_plan::display::{GraphvizVisitor, IndentVisitor}; use crate::logical_plan::extension::UserDefinedLogicalNode; @@ -620,7 +620,7 @@ impl LogicalPlan { self.apply(&mut |plan| { plan.inspect_expressions(|expr| { expr.apply(&mut |expr| { - if let Expr::Placeholder { id, data_type } = expr { + if let Expr::Placeholder(Placeholder { id, data_type }) = expr { let prev = param_types.get(id); match (prev, data_type) { (Some(Some(prev)), Some(dt)) => { @@ -654,7 +654,7 @@ impl LogicalPlan { ) -> Result { expr.transform(&|expr| { match &expr { - Expr::Placeholder { id, data_type } => { + Expr::Placeholder(Placeholder { id, data_type }) => { if id.is_empty() || id == "$0" { return Err(DataFusionError::Plan( "Empty placeholder id".to_string(), @@ -2264,10 +2264,10 @@ mod tests { let plan = table_scan(TableReference::none(), &schema, None) .unwrap() - .filter(col("id").eq(Expr::Placeholder { - id: "".into(), - data_type: Some(DataType::Int32), - })) + .filter(col("id").eq(Expr::Placeholder(Placeholder::new( + "".into(), + Some(DataType::Int32), + )))) .unwrap() .build() .unwrap(); @@ -2280,10 +2280,10 @@ mod tests { let plan = table_scan(TableReference::none(), &schema, None) .unwrap() - .filter(col("id").eq(Expr::Placeholder { - id: "$0".into(), - data_type: Some(DataType::Int32), - })) + .filter(col("id").eq(Expr::Placeholder(Placeholder::new( + "$0".into(), + Some(DataType::Int32), + )))) .unwrap() .build() .unwrap(); diff --git a/datafusion/expr/src/tree_node/expr.rs b/datafusion/expr/src/tree_node/expr.rs index 7bc9edf4c28a..3b8df59dab7b 100644 --- a/datafusion/expr/src/tree_node/expr.rs +++ b/datafusion/expr/src/tree_node/expr.rs @@ -19,8 +19,8 @@ use crate::expr::{ AggregateFunction, AggregateUDF, Between, BinaryExpr, Case, Cast, GetIndexedField, - GroupingSet, InList, InSubquery, Like, ScalarFunction, ScalarUDF, Sort, TryCast, - WindowFunction, + GroupingSet, InList, InSubquery, Like, Placeholder, ScalarFunction, ScalarUDF, Sort, + TryCast, WindowFunction, }; use crate::Expr; use datafusion_common::tree_node::VisitRecursion; @@ -67,7 +67,7 @@ impl TreeNode for Expr { | Expr::ScalarSubquery(_) | Expr::Wildcard | Expr::QualifiedWildcard { .. } - | Expr::Placeholder { .. } => vec![], + | Expr::Placeholder (_) => vec![], Expr::BinaryExpr(BinaryExpr { left, right, .. }) => { vec![left.as_ref().clone(), right.as_ref().clone()] } @@ -340,7 +340,9 @@ impl TreeNode for Expr { key, )) } - Expr::Placeholder { id, data_type } => Expr::Placeholder { id, data_type }, + Expr::Placeholder(Placeholder { id, data_type }) => { + Expr::Placeholder(Placeholder { id, data_type }) + } }) } } diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs index 06f9ccd11cc2..1d7aa536f910 100644 --- a/datafusion/expr/src/utils.rs +++ b/datafusion/expr/src/utils.rs @@ -308,7 +308,7 @@ pub fn expr_to_columns(expr: &Expr, accum: &mut HashSet) -> Result<()> { | Expr::Wildcard | Expr::QualifiedWildcard { .. } | Expr::GetIndexedField { .. } - | Expr::Placeholder { .. } + | Expr::Placeholder(_) | Expr::OuterReferenceColumn { .. } => {} } Ok(()) diff --git a/datafusion/optimizer/src/push_down_filter.rs b/datafusion/optimizer/src/push_down_filter.rs index d724c59d0dd7..175c6a118c7c 100644 --- a/datafusion/optimizer/src/push_down_filter.rs +++ b/datafusion/optimizer/src/push_down_filter.rs @@ -154,7 +154,7 @@ fn can_evaluate_as_join_condition(predicate: &Expr) -> Result { predicate.apply(&mut |expr| match expr { Expr::Column(_) | Expr::Literal(_) - | Expr::Placeholder { .. } + | Expr::Placeholder(_) | Expr::ScalarVariable(_, _) => Ok(VisitRecursion::Skip), Expr::Exists { .. } | Expr::InSubquery(_) diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs index 07391a2d7779..d077c9f83a04 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -265,7 +265,7 @@ impl<'a> ConstEvaluator<'a> { | Expr::GroupingSet(_) | Expr::Wildcard | Expr::QualifiedWildcard { .. } - | Expr::Placeholder { .. } => false, + | Expr::Placeholder(_) => false, Expr::ScalarFunction(ScalarFunction { fun, .. }) => { Self::volatility_ok(fun.volatility()) } diff --git a/datafusion/proto/src/logical_plan/from_proto.rs b/datafusion/proto/src/logical_plan/from_proto.rs index 890baba7575b..5afd94b5a7e4 100644 --- a/datafusion/proto/src/logical_plan/from_proto.rs +++ b/datafusion/proto/src/logical_plan/from_proto.rs @@ -34,6 +34,7 @@ use datafusion_common::{ Column, DFField, DFSchema, DFSchemaRef, DataFusionError, OwnedTableReference, Result, ScalarValue, }; +use datafusion_expr::expr::Placeholder; use datafusion_expr::{ abs, acos, acosh, array, ascii, asin, asinh, atan, atan2, atanh, bit_length, btrim, cbrt, ceil, character_length, chr, coalesce, concat_expr, concat_ws_expr, cos, cosh, @@ -1413,14 +1414,11 @@ pub fn parse_expr( ))) } ExprType::Placeholder(PlaceholderNode { id, data_type }) => match data_type { - None => Ok(Expr::Placeholder { - id: id.clone(), - data_type: None, - }), - Some(data_type) => Ok(Expr::Placeholder { - id: id.clone(), - data_type: Some(data_type.try_into()?), - }), + None => Ok(Expr::Placeholder(Placeholder::new(id.clone(), None))), + Some(data_type) => Ok(Expr::Placeholder(Placeholder::new( + id.clone(), + Some(data_type.try_into()?), + ))), }, } } diff --git a/datafusion/proto/src/logical_plan/to_proto.rs b/datafusion/proto/src/logical_plan/to_proto.rs index 263acd58dc47..e881051f273b 100644 --- a/datafusion/proto/src/logical_plan/to_proto.rs +++ b/datafusion/proto/src/logical_plan/to_proto.rs @@ -37,7 +37,7 @@ use arrow::datatypes::{ use datafusion_common::{Column, DFField, DFSchemaRef, OwnedTableReference, ScalarValue}; use datafusion_expr::expr::{ self, Between, BinaryExpr, Cast, GetIndexedField, GroupingSet, InList, Like, - ScalarFunction, ScalarUDF, Sort, + Placeholder, ScalarFunction, ScalarUDF, Sort, }; use datafusion_expr::{ logical_plan::PlanType, logical_plan::StringifiedPlan, AggregateFunction, @@ -944,7 +944,7 @@ impl TryFrom<&Expr> for protobuf::LogicalExprNode { .collect::, Self::Error>>()?, })), }, - Expr::Placeholder { id, data_type } => { + Expr::Placeholder(Placeholder { id, data_type }) => { let data_type = match data_type { Some(data_type) => Some(data_type.try_into()?), None => None, diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index a281e8a98534..a120b3400ad8 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -30,8 +30,8 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel}; use arrow_schema::DataType; use datafusion_common::tree_node::{Transformed, TreeNode}; use datafusion_common::{Column, DFSchema, DataFusionError, Result, ScalarValue}; -use datafusion_expr::expr::InList; use datafusion_expr::expr::ScalarFunction; +use datafusion_expr::expr::{InList, Placeholder}; use datafusion_expr::{ col, expr, lit, AggregateFunction, Between, BinaryExpr, BuiltinScalarFunction, Cast, Expr, ExprSchemable, GetIndexedField, Like, Operator, TryCast, @@ -500,7 +500,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { // modifies expr if it is a placeholder with datatype of right fn rewrite_placeholder(expr: &mut Expr, other: &Expr, schema: &DFSchema) -> Result<()> { - if let Expr::Placeholder { id: _, data_type } = expr { + if let Expr::Placeholder(Placeholder { id: _, data_type }) = expr { if data_type.is_none() { let other_dt = other.get_type(schema); match other_dt { diff --git a/datafusion/sql/src/expr/value.rs b/datafusion/sql/src/expr/value.rs index e747bf1b2a8a..3d959f17ce0c 100644 --- a/datafusion/sql/src/expr/value.rs +++ b/datafusion/sql/src/expr/value.rs @@ -19,7 +19,7 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel}; use arrow::compute::kernels::cast_utils::parse_interval_month_day_nano; use arrow_schema::DataType; use datafusion_common::{DFSchema, DataFusionError, Result, ScalarValue}; -use datafusion_expr::expr::BinaryExpr; +use datafusion_expr::expr::{BinaryExpr, Placeholder}; use datafusion_expr::{lit, Expr, Operator}; use log::debug; use sqlparser::ast::{BinaryOperator, DateTimeField, Expr as SQLExpr, Value}; @@ -114,10 +114,10 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { param, param_type ); - Ok(Expr::Placeholder { - id: param, - data_type: param_type.cloned(), - }) + Ok(Expr::Placeholder(Placeholder::new( + param, + param_type.cloned(), + ))) } pub(super) fn sql_array_literal( diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 779a79e15534..16d3c6c79309 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -48,6 +48,7 @@ use sqlparser::ast::{ TableConstraint, TableFactor, TableWithJoins, TransactionMode, UnaryOperator, Value, }; +use datafusion_expr::expr::Placeholder; use sqlparser::parser::ParserError::ParserError; use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::Arc; @@ -902,16 +903,16 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { for (col_name, expr) in values.into_iter() { let expr = self.sql_to_expr(expr, &table_schema, &mut planner_context)?; let expr = match expr { - datafusion_expr::Expr::Placeholder { + datafusion_expr::Expr::Placeholder(Placeholder { ref id, ref data_type, - } => match data_type { + }) => match data_type { None => { let dt = table_schema.data_type(&Column::from_name(&col_name))?; - datafusion_expr::Expr::Placeholder { - id: id.clone(), - data_type: Some(dt.clone()), - } + datafusion_expr::Expr::Placeholder(Placeholder::new( + id.clone(), + Some(dt.clone()), + )) } Some(_) => expr, }, diff --git a/datafusion/sql/src/utils.rs b/datafusion/sql/src/utils.rs index 1bfff37b48bd..160543360963 100644 --- a/datafusion/sql/src/utils.rs +++ b/datafusion/sql/src/utils.rs @@ -23,7 +23,8 @@ use sqlparser::ast::Ident; use datafusion_common::{DataFusionError, Result, ScalarValue}; use datafusion_expr::expr::{ AggregateFunction, AggregateUDF, Between, BinaryExpr, Case, GetIndexedField, - GroupingSet, InList, InSubquery, Like, ScalarFunction, ScalarUDF, WindowFunction, + GroupingSet, InList, InSubquery, Like, Placeholder, ScalarFunction, ScalarUDF, + WindowFunction, }; use datafusion_expr::expr::{Cast, Sort}; use datafusion_expr::utils::{expr_as_column_expr, find_column_exprs}; @@ -413,10 +414,9 @@ where ))) } }, - Expr::Placeholder { id, data_type } => Ok(Expr::Placeholder { - id: id.clone(), - data_type: data_type.clone(), - }), + Expr::Placeholder(Placeholder { id, data_type }) => Ok(Expr::Placeholder( + Placeholder::new(id.clone(), data_type.clone()), + )), }, } }