From 926bba77bc82f48dbf64c9386bc96846a119173d Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Fri, 17 May 2024 15:10:11 -0400 Subject: [PATCH 1/3] Minor: Fix names in ArrayFunctionRewriter --- datafusion/core/src/physical_planner.rs | 6 +++--- datafusion/functions-array/src/rewrite.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/datafusion/core/src/physical_planner.rs b/datafusion/core/src/physical_planner.rs index 406196a59146..9c6ebf665706 100644 --- a/datafusion/core/src/physical_planner.rs +++ b/datafusion/core/src/physical_planner.rs @@ -220,12 +220,12 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result { match field { GetFieldAccess::NamedStructField { name: _ } => { unreachable!( - "NamedStructField should have been rewritten in OperatorToFunction" + "NamedStructField should have been rewritten in ArrayFunctionRewriter" ) } GetFieldAccess::ListIndex { key: _ } => { unreachable!( - "ListIndex should have been rewritten in OperatorToFunction" + "ListIndex should have been rewritten in ArrayFunctionRewriter" ) } GetFieldAccess::ListRange { @@ -234,7 +234,7 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result { stride: _, } => { unreachable!( - "ListRange should have been rewritten in OperatorToFunction" + "ListRange should have been rewritten in ArrayFunctionRewriter" ) } }; diff --git a/datafusion/functions-array/src/rewrite.rs b/datafusion/functions-array/src/rewrite.rs index a7aba78c1dbe..d18f1f8a3cbb 100644 --- a/datafusion/functions-array/src/rewrite.rs +++ b/datafusion/functions-array/src/rewrite.rs @@ -33,7 +33,7 @@ pub(crate) struct ArrayFunctionRewriter {} impl FunctionRewrite for ArrayFunctionRewriter { fn name(&self) -> &str { - "FunctionRewrite" + "ArrayFunctionRewriter" } fn rewrite( From 7dcf0cb8dd490d787d3a3ddcf1a22562c05fefde Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Fri, 17 May 2024 15:24:49 -0400 Subject: [PATCH 2/3] Change panic to internal error --- datafusion/core/src/physical_planner.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/datafusion/core/src/physical_planner.rs b/datafusion/core/src/physical_planner.rs index 9c6ebf665706..b67f2cdbbe76 100644 --- a/datafusion/core/src/physical_planner.rs +++ b/datafusion/core/src/physical_planner.rs @@ -219,13 +219,13 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result { Expr::GetIndexedField(GetIndexedField { expr: _, field }) => { match field { GetFieldAccess::NamedStructField { name: _ } => { - unreachable!( - "NamedStructField should have been rewritten in ArrayFunctionRewriter" + return internal_err!( + "NamedStructField is no longer supported. Should use the get_field function instead", ) } GetFieldAccess::ListIndex { key: _ } => { - unreachable!( - "ListIndex should have been rewritten in ArrayFunctionRewriter" + return internal_err!( + "ListIndex is no longer supported. Should use the get_field function instead", ) } GetFieldAccess::ListRange { @@ -233,8 +233,8 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result { stop: _, stride: _, } => { - unreachable!( - "ListRange should have been rewritten in ArrayFunctionRewriter" + return internal_err!( + "ListRange is no longer supported. Should use the get_field function instead", ) } }; From 9dbf5a4ca02e479282b6de5c0cbf3ae292016c1f Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Fri, 17 May 2024 15:53:52 -0400 Subject: [PATCH 3/3] fix --- datafusion/core/src/physical_planner.rs | 38 ++++++++++++------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/datafusion/core/src/physical_planner.rs b/datafusion/core/src/physical_planner.rs index b67f2cdbbe76..5349a9b65d69 100644 --- a/datafusion/core/src/physical_planner.rs +++ b/datafusion/core/src/physical_planner.rs @@ -216,29 +216,27 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result { let expr = create_physical_name(expr, false)?; Ok(format!("{expr} IS NOT UNKNOWN")) } - Expr::GetIndexedField(GetIndexedField { expr: _, field }) => { - match field { - GetFieldAccess::NamedStructField { name: _ } => { - return internal_err!( - "NamedStructField is no longer supported. Should use the get_field function instead", + Expr::GetIndexedField(GetIndexedField { expr: _, field }) => match field { + GetFieldAccess::NamedStructField { name: _ } => { + internal_err!( + "NamedStructField is no longer supported. Should use the get_field function instead" ) - } - GetFieldAccess::ListIndex { key: _ } => { - return internal_err!( - "ListIndex is no longer supported. Should use the get_field function instead", + } + GetFieldAccess::ListIndex { key: _ } => { + internal_err!( + "ListIndex is no longer supported. Should use the get_field function instead" ) - } - GetFieldAccess::ListRange { - start: _, - stop: _, - stride: _, - } => { - return internal_err!( - "ListRange is no longer supported. Should use the get_field function instead", + } + GetFieldAccess::ListRange { + start: _, + stop: _, + stride: _, + } => { + internal_err!( + "ListRange is no longer supported. Should use the get_field function instead" ) - } - }; - } + } + }, Expr::ScalarFunction(fun) => fun.func.display_name(&fun.args), Expr::WindowFunction(WindowFunction { fun,