Box wildcard options

apache · Dec 14, 2024 · 1e8d072 · 1e8d072
1 parent ef93bce
commit 1e8d072
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 48 deletions.
diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs
@@ -222,8 +222,6 @@ use sqlparser::ast::{
 /// // to 42 = 5 AND b = 6
 /// assert_eq!(rewritten.data, lit(42).eq(lit(5)).and(col("b").eq(lit(6))));
 #[derive(Clone, PartialEq, Eq, PartialOrd, Hash, Debug)]
-// TODO make the enum smaller with more boxing (looks like Wildcard is now bigger)
-#[allow(clippy::large_enum_variant)]
 pub enum Expr {
     /// An expression with a specific name.
     Alias(Alias),
@@ -315,7 +313,7 @@ pub enum Expr {
     /// plan into physical plan.
     Wildcard {
         qualifier: Option<TableReference>,
-        options: WildcardOptions,
+        options: Box<WildcardOptions>,
     },
     /// List of grouping set expressions. Only valid in the context of an aggregate
     /// GROUP BY expression list

diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs
@@ -123,15 +123,15 @@ pub fn placeholder(id: impl Into<String>) -> Expr {
 pub fn wildcard() -> Expr {
     Expr::Wildcard {
         qualifier: None,
-        options: WildcardOptions::default(),
+        options: Box::new(WildcardOptions::default()),
     }
 }
 
 /// Create an '*' [`Expr::Wildcard`] expression with the wildcard options
 pub fn wildcard_with_options(options: WildcardOptions) -> Expr {
     Expr::Wildcard {
         qualifier: None,
-        options,
+        options: Box::new(options),
     }
 }
 
@@ -148,7 +148,7 @@ pub fn wildcard_with_options(options: WildcardOptions) -> Expr {
 pub fn qualified_wildcard(qualifier: impl Into<TableReference>) -> Expr {
     Expr::Wildcard {
         qualifier: Some(qualifier.into()),
-        options: WildcardOptions::default(),
+        options: Box::new(WildcardOptions::default()),
     }
 }
 
@@ -159,7 +159,7 @@ pub fn qualified_wildcard_with_options(
 ) -> Expr {
     Expr::Wildcard {
         qualifier: Some(qualifier.into()),
-        options,
+        options: Box::new(options),
     }
 }
 

diff --git a/datafusion/optimizer/src/analyzer/inline_table_scan.rs b/datafusion/optimizer/src/analyzer/inline_table_scan.rs
@@ -23,8 +23,7 @@ use crate::analyzer::AnalyzerRule;
 use datafusion_common::config::ConfigOptions;
 use datafusion_common::tree_node::{Transformed, TransformedResult, TreeNode};
 use datafusion_common::{Column, Result};
-use datafusion_expr::expr::WildcardOptions;
-use datafusion_expr::{logical_plan::LogicalPlan, Expr, LogicalPlanBuilder};
+use datafusion_expr::{logical_plan::LogicalPlan, wildcard, Expr, LogicalPlanBuilder};
 
 /// Analyzed rule that inlines TableScan that provide a [`LogicalPlan`]
 /// (DataFrame / ViewTable)
@@ -93,10 +92,7 @@ fn generate_projection_expr(
             )));
         }
     } else {
-        exprs.push(Expr::Wildcard {
-            qualifier: None,
-            options: WildcardOptions::default(),
-        });
+        exprs.push(wildcard());
     }
     Ok(exprs)
 }

diff --git a/datafusion/proto/src/logical_plan/from_proto.rs b/datafusion/proto/src/logical_plan/from_proto.rs
@@ -513,7 +513,7 @@ pub fn parse_expr(
             let qualifier = qualifier.to_owned().map(|x| x.try_into()).transpose()?;
             Ok(Expr::Wildcard {
                 qualifier,
-                options: WildcardOptions::default(),
+                options: Box::new(WildcardOptions::default()),
             })
         }
         ExprType::ScalarUdfExpr(protobuf::ScalarUdfExprNode {

diff --git a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs
@@ -67,7 +67,7 @@ use datafusion_common::{
 use datafusion_expr::dml::CopyTo;
 use datafusion_expr::expr::{
     self, Between, BinaryExpr, Case, Cast, GroupingSet, InList, Like, ScalarFunction,
-    Unnest, WildcardOptions,
+    Unnest,
 };
 use datafusion_expr::logical_plan::{Extension, UserDefinedLogicalNodeCore};
 use datafusion_expr::{
@@ -2061,21 +2061,15 @@ fn roundtrip_unnest() {
 
 #[test]
 fn roundtrip_wildcard() {
-    let test_expr = Expr::Wildcard {
-        qualifier: None,
-        options: WildcardOptions::default(),
-    };
+    let test_expr = wildcard();
 
     let ctx = SessionContext::new();
     roundtrip_expr_test(test_expr, ctx);
 }
 
 #[test]
 fn roundtrip_qualified_wildcard() {
-    let test_expr = Expr::Wildcard {
-        qualifier: Some("foo".into()),
-        options: WildcardOptions::default(),
-    };
+    let test_expr = qualified_wildcard("foo");
 
     let ctx = SessionContext::new();
     roundtrip_expr_test(test_expr, ctx);

diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs
@@ -22,11 +22,11 @@ use datafusion_common::{
     internal_datafusion_err, internal_err, not_impl_err, plan_datafusion_err, plan_err,
     DFSchema, Dependency, Result,
 };
-use datafusion_expr::expr::WildcardOptions;
 use datafusion_expr::expr::{ScalarFunction, Unnest};
 use datafusion_expr::planner::PlannerResult;
 use datafusion_expr::{
-    expr, Expr, ExprFunctionExt, ExprSchemable, WindowFrame, WindowFunctionDefinition,
+    expr, qualified_wildcard, wildcard, Expr, ExprFunctionExt, ExprSchemable,
+    WindowFrame, WindowFunctionDefinition,
 };
 use sqlparser::ast::{
     DuplicateTreatment, Expr as SQLExpr, Function as SQLFunction, FunctionArg,
@@ -418,28 +418,19 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
                 name: _,
                 arg: FunctionArgExpr::Wildcard,
                 operator: _,
-            } => Ok(Expr::Wildcard {
-                qualifier: None,
-                options: WildcardOptions::default(),
-            }),
+            } => Ok(wildcard()),
             FunctionArg::Unnamed(FunctionArgExpr::Expr(arg)) => {
                 self.sql_expr_to_logical_expr(arg, schema, planner_context)
             }
-            FunctionArg::Unnamed(FunctionArgExpr::Wildcard) => Ok(Expr::Wildcard {
-                qualifier: None,
-                options: WildcardOptions::default(),
-            }),
+            FunctionArg::Unnamed(FunctionArgExpr::Wildcard) => Ok(wildcard()),
             FunctionArg::Unnamed(FunctionArgExpr::QualifiedWildcard(object_name)) => {
                 let qualifier = self.object_name_to_table_reference(object_name)?;
                 // Sanity check on qualifier with schema
                 let qualified_indices = schema.fields_indices_with_qualified(&qualifier);
                 if qualified_indices.is_empty() {
                     return plan_err!("Invalid qualifier {qualifier}");
                 }
-                Ok(Expr::Wildcard {
-                    qualifier: Some(qualifier),
-                    options: WildcardOptions::default(),
-                })
+                Ok(qualified_wildcard(qualifier))
             }
             _ => not_impl_err!("Unsupported qualified wildcard argument: {sql:?}"),
         }

diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs
@@ -595,11 +595,11 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
             }
             SQLExpr::Wildcard(_token) => Ok(Expr::Wildcard {
                 qualifier: None,
-                options: WildcardOptions::default(),
+                options: Box::new(WildcardOptions::default()),
             }),
             SQLExpr::QualifiedWildcard(object_name, _token) => Ok(Expr::Wildcard {
                 qualifier: Some(self.object_name_to_table_reference(object_name)?),
-                options: WildcardOptions::default(),
+                options: Box::new(WildcardOptions::default()),
             }),
             SQLExpr::Tuple(values) => self.parse_tuple(schema, planner_context, values),
             _ => not_impl_err!("Unsupported ast node in sqltorel: {sql:?}"),

diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs
@@ -1688,7 +1688,7 @@ mod tests {
         let dummy_logical_plan = table_scan(Some("t"), &dummy_schema, None)?
             .project(vec![Expr::Wildcard {
                 qualifier: None,
-                options: WildcardOptions::default(),
+                options: Box::new(WildcardOptions::default()),
             }])?
             .filter(col("a").eq(lit(1)))?
             .build()?;
@@ -1880,21 +1880,15 @@ mod tests {
             (sum(col("a")), r#"sum(a)"#),
             (
                 count_udaf()
-                    .call(vec![Expr::Wildcard {
-                        qualifier: None,
-                        options: WildcardOptions::default(),
-                    }])
+                    .call(vec![wildcard()])
                     .distinct()
                     .build()
                     .unwrap(),
                 "count(DISTINCT *)",
             ),
             (
                 count_udaf()
-                    .call(vec![Expr::Wildcard {
-                        qualifier: None,
-                        options: WildcardOptions::default(),
-                    }])
+                    .call(vec![wildcard()])
                     .filter(lit(true))
                     .build()
                     .unwrap(),