diff --git a/vortex-array/src/arrays/scalar_fn/vtable/mod.rs b/vortex-array/src/arrays/scalar_fn/vtable/mod.rs index fa98bdb9683..bb597bc6bb8 100644 --- a/vortex-array/src/arrays/scalar_fn/vtable/mod.rs +++ b/vortex-array/src/arrays/scalar_fn/vtable/mod.rs @@ -38,6 +38,7 @@ use crate::dtype::DType; use crate::executor::ExecutionCtx; use crate::executor::ExecutionResult; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::matcher::Matcher; use crate::scalar_fn; use crate::scalar_fn::Arity; @@ -309,7 +310,7 @@ impl scalar_fn::ScalarFnVTable for ArrayExpr { fn fmt_sql( &self, options: &Self::Options, - _expr: &Expression, + _expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "{}", options.0.encoding_id()) diff --git a/vortex-array/src/expr/bound_expression.rs b/vortex-array/src/expr/bound_expression.rs index 3a35757068a..26173171d91 100644 --- a/vortex-array/src/expr/bound_expression.rs +++ b/vortex-array/src/expr/bound_expression.rs @@ -228,7 +228,10 @@ impl BoundExpression { impl Display for BoundExpression { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - Display::fmt(&self.unbind(), f) + match self.kind() { + BoundKind::Scalar { scalar_fn, .. } => scalar_fn.fmt_sql(self, f), + BoundKind::Root => f.write_str("$"), + } } } @@ -333,9 +336,10 @@ mod tests { } #[test] - fn bound_tree_display_matches_unbound() -> VortexResult<()> { + fn bound_display_matches_unbound() -> VortexResult<()> { for expr in [root(), col("a"), eq(col("a"), lit(1_i32)), lit(true)] { let bound = expr.bind_scope(&scope())?; + assert_eq!(bound.to_string(), expr.to_string()); assert_eq!( bound.display_tree().to_string(), expr.display_tree().to_string() diff --git a/vortex-array/src/expr/display.rs b/vortex-array/src/expr/display.rs index 685af718134..d542f7d35b3 100644 --- a/vortex-array/src/expr/display.rs +++ b/vortex-array/src/expr/display.rs @@ -16,6 +16,38 @@ pub enum DisplayFormat { Tree, } +/// Read-only expression-tree interface used by scalar functions for SQL-style formatting. +/// +/// Both [`Expression`] and [`BoundExpression`] implement this interface, allowing scalar +/// functions to format either representation without converting between them. +pub trait ExprDisplay: Display { + /// Return the child at `index`. + fn display_child(&self, index: usize) -> &dyn ExprDisplay; + + /// Return the number of children in this node. + fn display_children_count(&self) -> usize; +} + +impl ExprDisplay for Expression { + fn display_child(&self, index: usize) -> &dyn ExprDisplay { + Expression::child(self, index) + } + + fn display_children_count(&self) -> usize { + self.children().len() + } +} + +impl ExprDisplay for BoundExpression { + fn display_child(&self, index: usize) -> &dyn ExprDisplay { + &self.children()[index] + } + + fn display_children_count(&self) -> usize { + self.children().len() + } +} + trait DisplayTreeNode: Sized { fn tree_children(&self) -> &[Self]; diff --git a/vortex-array/src/scalar_fn/erased.rs b/vortex-array/src/scalar_fn/erased.rs index 6e0011c297a..f4c8136ad26 100644 --- a/vortex-array/src/scalar_fn/erased.rs +++ b/vortex-array/src/scalar_fn/erased.rs @@ -20,6 +20,7 @@ use crate::ArrayRef; use crate::ExecutionCtx; use crate::dtype::DType; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar_fn::EmptyOptions; use crate::scalar_fn::ExecutionArgs; use crate::scalar_fn::ReduceCtx; @@ -160,8 +161,12 @@ impl ScalarFnRef { // Expression-taking methods — used by expr/ module via pub(crate) // ------------------------------------------------------------------ - /// Format this expression in SQL-style format. - pub(crate) fn fmt_sql(&self, expr: &Expression, f: &mut Formatter<'_>) -> std::fmt::Result { + /// Format an expression tree in SQL-style format. + pub(crate) fn fmt_sql( + &self, + expr: &dyn ExprDisplay, + f: &mut Formatter<'_>, + ) -> std::fmt::Result { self.0.fmt_sql(expr, f) } diff --git a/vortex-array/src/scalar_fn/fns/between/mod.rs b/vortex-array/src/scalar_fn/fns/between/mod.rs index 1a2e038b2fb..c3131dbd342 100644 --- a/vortex-array/src/scalar_fn/fns/between/mod.rs +++ b/vortex-array/src/scalar_fn/fns/between/mod.rs @@ -25,6 +25,7 @@ use crate::arrays::Primitive; use crate::builtins::ArrayBuiltins; use crate::dtype::DType; use crate::dtype::DType::Bool; +use crate::expr::display::ExprDisplay; use crate::expr::expression::Expression; use crate::scalar::Scalar; use crate::scalar_fn::Arity; @@ -224,7 +225,7 @@ impl ScalarFnVTable for Between { fn fmt_sql( &self, options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { let lower_op = if options.lower_strict.is_strict() { @@ -240,11 +241,11 @@ impl ScalarFnVTable for Between { write!( f, "({} {} {} {} {})", - expr.child(1), + expr.display_child(1), lower_op, - expr.child(0), + expr.display_child(0), upper_op, - expr.child(2) + expr.display_child(2) ) } diff --git a/vortex-array/src/scalar_fn/fns/binary/mod.rs b/vortex-array/src/scalar_fn/fns/binary/mod.rs index 8a986c079a0..a5b9fe70539 100644 --- a/vortex-array/src/scalar_fn/fns/binary/mod.rs +++ b/vortex-array/src/scalar_fn/fns/binary/mod.rs @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +use std::fmt::Display; use std::fmt::Formatter; #[expect(deprecated)] @@ -19,6 +20,7 @@ use crate::ExecutionCtx; use crate::dtype::DType; use crate::dtype::Nullability; use crate::expr::and; +use crate::expr::display::ExprDisplay; use crate::expr::expression::Expression; use crate::expr::lit; use crate::scalar_fn::Arity; @@ -89,13 +91,13 @@ impl ScalarFnVTable for Binary { fn fmt_sql( &self, operator: &Operator, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "(")?; - expr.child(0).fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; write!(f, " {} ", operator)?; - expr.child(1).fmt_sql(f)?; + Display::fmt(expr.display_child(1), f)?; write!(f, ")") } diff --git a/vortex-array/src/scalar_fn/fns/case_when.rs b/vortex-array/src/scalar_fn/fns/case_when.rs index 189556c1fdb..8779e0a179c 100644 --- a/vortex-array/src/scalar_fn/fns/case_when.rs +++ b/vortex-array/src/scalar_fn/fns/case_when.rs @@ -35,6 +35,7 @@ use crate::builders::builder_with_capacity; use crate::builtins::ArrayBuiltins; use crate::dtype::DType; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar::Scalar; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; @@ -136,7 +137,7 @@ impl ScalarFnVTable for CaseWhen { fn fmt_sql( &self, options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> fmt::Result { write!(f, "CASE")?; @@ -144,13 +145,13 @@ impl ScalarFnVTable for CaseWhen { write!( f, " WHEN {} THEN {}", - expr.child(i * 2), - expr.child(i * 2 + 1) + expr.display_child(i * 2), + expr.display_child(i * 2 + 1) )?; } if options.has_else { let else_idx = options.num_when_then_pairs as usize * 2; - write!(f, " ELSE {}", expr.child(else_idx))?; + write!(f, " ELSE {}", expr.display_child(else_idx))?; } write!(f, " END") } diff --git a/vortex-array/src/scalar_fn/fns/cast/mod.rs b/vortex-array/src/scalar_fn/fns/cast/mod.rs index 8df6accece3..7a9f1356647 100644 --- a/vortex-array/src/scalar_fn/fns/cast/mod.rs +++ b/vortex-array/src/scalar_fn/fns/cast/mod.rs @@ -3,6 +3,7 @@ mod kernel; +use std::fmt::Display; use std::fmt::Formatter; pub use kernel::*; @@ -33,6 +34,7 @@ use crate::arrays::VarBinView; use crate::arrays::struct_::compute::cast::struct_cast; use crate::builtins::ArrayBuiltins; use crate::dtype::DType; +use crate::expr::display::ExprDisplay; use crate::expr::expression::Expression; use crate::expr::lit; use crate::scalar_fn::Arity; @@ -91,9 +93,14 @@ impl ScalarFnVTable for Cast { } } - fn fmt_sql(&self, dtype: &DType, expr: &Expression, f: &mut Formatter<'_>) -> std::fmt::Result { + fn fmt_sql( + &self, + dtype: &DType, + expr: &dyn ExprDisplay, + f: &mut Formatter<'_>, + ) -> std::fmt::Result { write!(f, "cast(")?; - expr.children()[0].fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; write!(f, " as {}", dtype)?; write!(f, ")") } diff --git a/vortex-array/src/scalar_fn/fns/dynamic.rs b/vortex-array/src/scalar_fn/fns/dynamic.rs index a741fcd9217..00e59796e05 100644 --- a/vortex-array/src/scalar_fn/fns/dynamic.rs +++ b/vortex-array/src/scalar_fn/fns/dynamic.rs @@ -20,6 +20,7 @@ use crate::IntoArray; use crate::arrays::ConstantArray; use crate::dtype::DType; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::expr::traversal::NodeExt; use crate::expr::traversal::NodeVisitor; use crate::expr::traversal::TraversalOrder; @@ -64,10 +65,10 @@ impl ScalarFnVTable for DynamicComparison { fn fmt_sql( &self, dynamic: &DynamicComparisonExpr, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { - expr.child(0).fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; write!(f, " {} dynamic(", dynamic.operator)?; match dynamic.scalar() { None => write!(f, "scalar=")?, diff --git a/vortex-array/src/scalar_fn/fns/get_item.rs b/vortex-array/src/scalar_fn/fns/get_item.rs index af319ecf742..bbbc15de479 100644 --- a/vortex-array/src/scalar_fn/fns/get_item.rs +++ b/vortex-array/src/scalar_fn/fns/get_item.rs @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +use std::fmt::Display; use std::fmt::Formatter; use prost::Message; @@ -20,6 +21,7 @@ use crate::dtype::DType; use crate::dtype::FieldName; use crate::dtype::Nullability; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::expr::lit; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; @@ -78,10 +80,10 @@ impl ScalarFnVTable for GetItem { fn fmt_sql( &self, field_name: &FieldName, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { - expr.children()[0].fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; write!(f, ".{}", field_name) } diff --git a/vortex-array/src/scalar_fn/fns/is_not_null.rs b/vortex-array/src/scalar_fn/fns/is_not_null.rs index acb3b9e0f35..dcf00f62c08 100644 --- a/vortex-array/src/scalar_fn/fns/is_not_null.rs +++ b/vortex-array/src/scalar_fn/fns/is_not_null.rs @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +use std::fmt::Display; use std::fmt::Formatter; use vortex_error::VortexResult; @@ -13,7 +14,7 @@ use crate::IntoArray; use crate::arrays::ConstantArray; use crate::dtype::DType; use crate::dtype::Nullability; -use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; use crate::scalar_fn::EmptyOptions; @@ -60,11 +61,11 @@ impl ScalarFnVTable for IsNotNull { fn fmt_sql( &self, _options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "is_not_null(")?; - expr.child(0).fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; write!(f, ")") } diff --git a/vortex-array/src/scalar_fn/fns/like/mod.rs b/vortex-array/src/scalar_fn/fns/like/mod.rs index 3637e0384a3..c947ce461b2 100644 --- a/vortex-array/src/scalar_fn/fns/like/mod.rs +++ b/vortex-array/src/scalar_fn/fns/like/mod.rs @@ -31,6 +31,7 @@ use crate::dtype::DType; use crate::dtype::Nullability; use crate::expr::Expression; use crate::expr::and; +use crate::expr::display::ExprDisplay; use crate::scalar::Scalar; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; @@ -107,10 +108,10 @@ impl ScalarFnVTable for Like { fn fmt_sql( &self, options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { - expr.child(0).fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; if options.negated { write!(f, " not")?; } @@ -119,7 +120,7 @@ impl ScalarFnVTable for Like { } else { write!(f, " like ")?; } - expr.child(1).fmt_sql(f) + Display::fmt(expr.display_child(1), f) } fn return_dtype(&self, _options: &Self::Options, arg_dtypes: &[DType]) -> VortexResult { diff --git a/vortex-array/src/scalar_fn/fns/literal.rs b/vortex-array/src/scalar_fn/fns/literal.rs index 0449ae27e29..4c95fd3172d 100644 --- a/vortex-array/src/scalar_fn/fns/literal.rs +++ b/vortex-array/src/scalar_fn/fns/literal.rs @@ -16,6 +16,7 @@ use crate::IntoArray; use crate::arrays::ConstantArray; use crate::dtype::DType; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar::Scalar; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; @@ -74,7 +75,7 @@ impl ScalarFnVTable for Literal { fn fmt_sql( &self, scalar: &Scalar, - _expr: &Expression, + _expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "{}", scalar) diff --git a/vortex-array/src/scalar_fn/fns/pack.rs b/vortex-array/src/scalar_fn/fns/pack.rs index 84983d9d55e..60eb5b1c2c4 100644 --- a/vortex-array/src/scalar_fn/fns/pack.rs +++ b/vortex-array/src/scalar_fn/fns/pack.rs @@ -23,6 +23,7 @@ use crate::dtype::FieldNames; use crate::dtype::Nullability; use crate::dtype::StructFields; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::expr::lit; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; @@ -105,13 +106,13 @@ impl ScalarFnVTable for Pack { fn fmt_sql( &self, options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "pack(")?; - for (i, (name, child)) in options.names.iter().zip(expr.children().iter()).enumerate() { + for (i, name) in options.names.iter().enumerate() { write!(f, "{}: ", name)?; - child.fmt_sql(f)?; + Display::fmt(expr.display_child(i), f)?; if i + 1 < options.names.len() { write!(f, ", ")?; } diff --git a/vortex-array/src/scalar_fn/fns/root.rs b/vortex-array/src/scalar_fn/fns/root.rs index 646bed19a1b..0831e5b8585 100644 --- a/vortex-array/src/scalar_fn/fns/root.rs +++ b/vortex-array/src/scalar_fn/fns/root.rs @@ -11,7 +11,7 @@ use vortex_session::registry::CachedId; use crate::ArrayRef; use crate::ExecutionCtx; use crate::dtype::DType; -use crate::expr::expression::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; use crate::scalar_fn::EmptyOptions; @@ -58,7 +58,7 @@ impl ScalarFnVTable for Root { fn fmt_sql( &self, _options: &Self::Options, - _expr: &Expression, + _expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "$") diff --git a/vortex-array/src/scalar_fn/fns/select.rs b/vortex-array/src/scalar_fn/fns/select.rs index 2e0bbd02975..d6d30c5547b 100644 --- a/vortex-array/src/scalar_fn/fns/select.rs +++ b/vortex-array/src/scalar_fn/fns/select.rs @@ -24,6 +24,7 @@ use crate::arrays::struct_::StructArrayExt; use crate::dtype::DType; use crate::dtype::FieldName; use crate::dtype::FieldNames; +use crate::expr::display::ExprDisplay; use crate::expr::expression::Expression; use crate::expr::field::DisplayFieldNames; use crate::expr::get_item; @@ -104,10 +105,10 @@ impl ScalarFnVTable for Select { fn fmt_sql( &self, selection: &FieldSelection, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { - expr.child(0).fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; match selection { FieldSelection::Include(fields) => { write!(f, "{{{}}}", DisplayFieldNames(fields)) diff --git a/vortex-array/src/scalar_fn/fns/stat.rs b/vortex-array/src/scalar_fn/fns/stat.rs index 612298424e4..c961ac10d53 100644 --- a/vortex-array/src/scalar_fn/fns/stat.rs +++ b/vortex-array/src/scalar_fn/fns/stat.rs @@ -20,7 +20,7 @@ use crate::aggregate_fn::fns::all_non_null::AllNonNull; use crate::aggregate_fn::fns::all_null::AllNull; use crate::arrays::ConstantArray; use crate::dtype::DType; -use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::expr::stats::Precision; use crate::expr::stats::Stat; use crate::expr::stats::StatsProvider; @@ -101,11 +101,11 @@ impl ScalarFnVTable for StatFn { fn fmt_sql( &self, options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "stat(")?; - expr.child(0).fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; write!(f, ", {})", options.aggregate_fn()) } diff --git a/vortex-array/src/scalar_fn/fns/variant_get/mod.rs b/vortex-array/src/scalar_fn/fns/variant_get/mod.rs index d939235d24f..593ecb9c1db 100644 --- a/vortex-array/src/scalar_fn/fns/variant_get/mod.rs +++ b/vortex-array/src/scalar_fn/fns/variant_get/mod.rs @@ -25,7 +25,7 @@ use crate::builders::builder_with_capacity_in; use crate::dtype::DType; use crate::dtype::FieldName; use crate::dtype::Nullability; -use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar::Scalar; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; @@ -92,11 +92,11 @@ impl ScalarFnVTable for VariantGet { fn fmt_sql( &self, options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> fmt::Result { write!(f, "variant_get(")?; - expr.child(0).fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; let path = options.path().to_string(); write!(f, ", \"{}\"", StringEscape(&path))?; if let Some(dtype) = options.dtype() { diff --git a/vortex-array/src/scalar_fn/fns/zip/mod.rs b/vortex-array/src/scalar_fn/fns/zip/mod.rs index 5cb84bc737e..9b591cbdf0d 100644 --- a/vortex-array/src/scalar_fn/fns/zip/mod.rs +++ b/vortex-array/src/scalar_fn/fns/zip/mod.rs @@ -3,6 +3,7 @@ mod kernel; +use std::fmt::Display; use std::fmt::Formatter; pub use kernel::*; @@ -24,6 +25,7 @@ use crate::builders::builder_with_capacity; use crate::builtins::ArrayBuiltins; use crate::dtype::DType; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; use crate::scalar_fn::EmptyOptions; @@ -80,15 +82,15 @@ impl ScalarFnVTable for Zip { fn fmt_sql( &self, _options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "zip(")?; - expr.child(0).fmt_sql(f)?; + Display::fmt(expr.display_child(0), f)?; write!(f, ", ")?; - expr.child(1).fmt_sql(f)?; + Display::fmt(expr.display_child(1), f)?; write!(f, ", ")?; - expr.child(2).fmt_sql(f)?; + Display::fmt(expr.display_child(2), f)?; write!(f, ")") } diff --git a/vortex-array/src/scalar_fn/foreign.rs b/vortex-array/src/scalar_fn/foreign.rs index 94fd75b08f1..667b4756c1c 100644 --- a/vortex-array/src/scalar_fn/foreign.rs +++ b/vortex-array/src/scalar_fn/foreign.rs @@ -13,7 +13,7 @@ use crate::ArrayRef; use crate::ExecutionCtx; use crate::dtype::DType; use crate::dtype::Nullability; -use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; use crate::scalar_fn::ExecutionArgs; @@ -93,15 +93,15 @@ impl ScalarFnVTable for ForeignScalarFnVTable { fn fmt_sql( &self, _options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> fmt::Result { write!(f, "{}(", self.id)?; - for i in 0..expr.children().len() { + for i in 0..expr.display_children_count() { if i > 0 { write!(f, ", ")?; } - expr.child(i).fmt_sql(f)?; + Display::fmt(expr.display_child(i), f)?; } write!(f, ")") } diff --git a/vortex-array/src/scalar_fn/internal/row_count.rs b/vortex-array/src/scalar_fn/internal/row_count.rs index 7f78acbef64..290378c30a7 100644 --- a/vortex-array/src/scalar_fn/internal/row_count.rs +++ b/vortex-array/src/scalar_fn/internal/row_count.rs @@ -11,7 +11,7 @@ use vortex_array::arrays::scalar_fn::ScalarFnArrayExt; use vortex_array::dtype::DType; use vortex_array::dtype::Nullability; use vortex_array::dtype::PType; -use vortex_array::expr::Expression; +use vortex_array::expr::display::ExprDisplay; use vortex_array::scalar_fn::Arity; use vortex_array::scalar_fn::ChildName; use vortex_array::scalar_fn::EmptyOptions; @@ -64,7 +64,7 @@ impl ScalarFnVTable for RowCount { fn fmt_sql( &self, _options: &Self::Options, - _expr: &Expression, + _expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "row_count()") diff --git a/vortex-array/src/scalar_fn/typed.rs b/vortex-array/src/scalar_fn/typed.rs index a6620735ff2..9c9225c7737 100644 --- a/vortex-array/src/scalar_fn/typed.rs +++ b/vortex-array/src/scalar_fn/typed.rs @@ -24,6 +24,7 @@ use crate::ArrayRef; use crate::ExecutionCtx; use crate::dtype::DType; use crate::expr::Expression; +use crate::expr::display::ExprDisplay; use crate::scalar_fn::Arity; use crate::scalar_fn::ChildName; use crate::scalar_fn::ExecutionArgs; @@ -92,8 +93,8 @@ pub(super) trait DynScalarFn: 'static + Send + Sync + super::sealed::Sealed { fn is_strict(&self) -> bool; fn is_fallible(&self) -> bool; - // Expression methods — take &Expression for tree traversal - fn fmt_sql(&self, expression: &Expression, f: &mut Formatter<'_>) -> fmt::Result; + // Expression methods — take expressions for tree traversal + fn fmt_sql(&self, expression: &dyn ExprDisplay, f: &mut Formatter<'_>) -> fmt::Result; fn simplify( &self, expression: &Expression, @@ -192,7 +193,7 @@ impl DynScalarFn for TypedScalarFnInstance { V::is_fallible(&self.vtable, &self.options) } - fn fmt_sql(&self, expression: &Expression, f: &mut Formatter<'_>) -> fmt::Result { + fn fmt_sql(&self, expression: &dyn ExprDisplay, f: &mut Formatter<'_>) -> fmt::Result { V::fmt_sql(&self.vtable, &self.options, expression, f) } diff --git a/vortex-array/src/scalar_fn/vtable.rs b/vortex-array/src/scalar_fn/vtable.rs index ece52f6c4b4..55c897ce1a1 100644 --- a/vortex-array/src/scalar_fn/vtable.rs +++ b/vortex-array/src/scalar_fn/vtable.rs @@ -20,7 +20,7 @@ use crate::ArrayRef; use crate::ExecutionCtx; use crate::dtype::DType; use crate::expr::Expression; -use crate::expr::traversal::Node; +use crate::expr::display::ExprDisplay; use crate::scalar_fn::ScalarFnId; use crate::scalar_fn::ScalarFnRef; use crate::scalar_fn::TypedScalarFnInstance; @@ -67,20 +67,20 @@ pub trait ScalarFnVTable: 'static + Sized + Clone + Send + Sync { /// Returns the name of the nth child of the expr. fn child_name(&self, options: &Self::Options, child_idx: usize) -> ChildName; - /// Format this expression in a nice human-readable SQL-style format + /// Format an expression tree in a human-readable SQL-style format. /// - /// The implementation should recursively format child expressions by calling - /// `expr.child(i).fmt_sql(f)`. + /// The expression may be either an [`Expression`] or a + /// [`bound expression`](crate::expr::BoundExpression). fn fmt_sql( &self, options: &Self::Options, - expr: &Expression, + expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> fmt::Result { write!(f, "{}(", self.id())?; - let nchildren = expr.children_count(); - for (i, child) in expr.children().iter().enumerate() { - child.fmt_sql(f)?; + let nchildren = expr.display_children_count(); + for i in 0..nchildren { + Display::fmt(expr.display_child(i), f)?; if i + 1 < nchildren { write!(f, ", ")?; } diff --git a/vortex-layout/src/layouts/row_idx/expr.rs b/vortex-layout/src/layouts/row_idx/expr.rs index c0043542f93..31dbbd4d47e 100644 --- a/vortex-layout/src/layouts/row_idx/expr.rs +++ b/vortex-layout/src/layouts/row_idx/expr.rs @@ -8,6 +8,7 @@ use vortex_array::dtype::DType; use vortex_array::dtype::Nullability; use vortex_array::dtype::PType; use vortex_array::expr::Expression; +use vortex_array::expr::display::ExprDisplay; use vortex_array::scalar_fn::Arity; use vortex_array::scalar_fn::ChildName; use vortex_array::scalar_fn::EmptyOptions; @@ -41,7 +42,7 @@ impl ScalarFnVTable for RowIdx { fn fmt_sql( &self, _options: &Self::Options, - _expr: &Expression, + _expr: &dyn ExprDisplay, f: &mut Formatter<'_>, ) -> std::fmt::Result { write!(f, "#row_idx")