alamb commented on code in PR #11977:
URL: https://github.com/apache/datafusion/pull/11977#discussion_r1717580437
##########
datafusion/expr/src/expr.rs:
##########
@@ -1104,6 +1103,7 @@ impl Expr {
}
/// Returns a full and complete string representation of this expression.
+ #[deprecated(note = "use format! instead")]
Review Comment:
👍
##########
datafusion/expr/src/expr.rs:
##########
@@ -2386,263 +2386,13 @@ fn fmt_function(
write!(f, "{}({}{})", fun, distinct_str, args.join(", "))
}
-pub fn create_function_physical_name(
- fun: &str,
- distinct: bool,
- args: &[Expr],
- order_by: Option<&Vec<Expr>>,
-) -> Result<String> {
- let names: Vec<String> = args
- .iter()
- .map(|e| create_physical_name(e, false))
- .collect::<Result<_>>()?;
-
- let distinct_str = match distinct {
- true => "DISTINCT ",
- false => "",
- };
-
- let phys_name = format!("{}({}{})", fun, distinct_str, names.join(","));
-
- Ok(order_by
- .map(|order_by| format!("{} ORDER BY [{}]", phys_name,
expr_vec_fmt!(order_by)))
- .unwrap_or(phys_name))
-}
-
-pub fn physical_name(e: &Expr) -> Result<String> {
- create_physical_name(e, true)
-}
-
-fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result<String> {
- match e {
- Expr::Unnest(_) => {
- internal_err!(
- "Expr::Unnest should have been converted to
LogicalPlan::Unnest"
- )
- }
- Expr::Column(c) => {
- if is_first_expr {
- Ok(c.name.clone())
- } else {
- Ok(c.flat_name())
- }
- }
- Expr::Alias(Alias { name, .. }) => Ok(name.clone()),
- Expr::ScalarVariable(_, variable_names) =>
Ok(variable_names.join(".")),
- Expr::Literal(value) => Ok(format!("{value:?}")),
- Expr::BinaryExpr(BinaryExpr { left, op, right }) => {
- let left = create_physical_name(left, false)?;
- let right = create_physical_name(right, false)?;
- Ok(format!("{left} {op} {right}"))
- }
- Expr::Case(case) => {
- let mut name = "CASE ".to_string();
- if let Some(e) = &case.expr {
- let _ = write!(name, "{} ", create_physical_name(e, false)?);
- }
- for (w, t) in &case.when_then_expr {
- let _ = write!(
- name,
- "WHEN {} THEN {} ",
- create_physical_name(w, false)?,
- create_physical_name(t, false)?
- );
- }
- if let Some(e) = &case.else_expr {
- let _ = write!(name, "ELSE {} ", create_physical_name(e,
false)?);
- }
- name += "END";
- Ok(name)
- }
- Expr::Cast(Cast { expr, .. }) => {
- // CAST does not change the expression name
- create_physical_name(expr, false)
- }
- Expr::TryCast(TryCast { expr, .. }) => {
- // CAST does not change the expression name
- create_physical_name(expr, false)
- }
- Expr::Not(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("NOT {expr}"))
- }
- Expr::Negative(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("(- {expr})"))
- }
- Expr::IsNull(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("{expr} IS NULL"))
- }
- Expr::IsNotNull(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("{expr} IS NOT NULL"))
- }
- Expr::IsTrue(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("{expr} IS TRUE"))
- }
- Expr::IsFalse(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("{expr} IS FALSE"))
- }
- Expr::IsUnknown(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("{expr} IS UNKNOWN"))
- }
- Expr::IsNotTrue(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("{expr} IS NOT TRUE"))
- }
- Expr::IsNotFalse(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("{expr} IS NOT FALSE"))
- }
- Expr::IsNotUnknown(expr) => {
- let expr = create_physical_name(expr, false)?;
- Ok(format!("{expr} IS NOT UNKNOWN"))
- }
- Expr::ScalarFunction(fun) => fun.func.schema_name(&fun.args),
- Expr::WindowFunction(WindowFunction {
- fun,
- args,
- order_by,
- ..
- }) => {
- create_function_physical_name(&fun.to_string(), false, args,
Some(order_by))
- }
- Expr::AggregateFunction(AggregateFunction {
- func,
- distinct,
- args,
- filter: _,
- order_by,
- null_treatment: _,
- }) => {
- create_function_physical_name(func.name(), *distinct, args,
order_by.as_ref())
- }
- Expr::GroupingSet(grouping_set) => match grouping_set {
- GroupingSet::Rollup(exprs) => Ok(format!(
- "ROLLUP ({})",
- exprs
- .iter()
- .map(|e| create_physical_name(e, false))
- .collect::<Result<Vec<_>>>()?
- .join(", ")
- )),
- GroupingSet::Cube(exprs) => Ok(format!(
- "CUBE ({})",
- exprs
- .iter()
- .map(|e| create_physical_name(e, false))
- .collect::<Result<Vec<_>>>()?
- .join(", ")
- )),
- GroupingSet::GroupingSets(lists_of_exprs) => {
- let mut strings = vec![];
- for exprs in lists_of_exprs {
- let exprs_str = exprs
- .iter()
- .map(|e| create_physical_name(e, false))
- .collect::<Result<Vec<_>>>()?
- .join(", ");
- strings.push(format!("({exprs_str})"));
- }
- Ok(format!("GROUPING SETS ({})", strings.join(", ")))
- }
- },
-
- Expr::InList(InList {
- expr,
- list,
- negated,
- }) => {
- let expr = create_physical_name(expr, false)?;
- let list = list.iter().map(|expr| create_physical_name(expr,
false));
- if *negated {
- Ok(format!("{expr} NOT IN ({list:?})"))
- } else {
- Ok(format!("{expr} IN ({list:?})"))
- }
- }
- Expr::Exists { .. } => {
- not_impl_err!("EXISTS is not yet supported in the physical plan")
- }
- Expr::InSubquery(_) => {
- not_impl_err!("IN subquery is not yet supported in the physical
plan")
- }
- Expr::ScalarSubquery(_) => {
- not_impl_err!("Scalar subqueries are not yet supported in the
physical plan")
- }
- Expr::Between(Between {
- expr,
- negated,
- low,
- high,
- }) => {
- let expr = create_physical_name(expr, false)?;
- let low = create_physical_name(low, false)?;
- let high = create_physical_name(high, false)?;
- if *negated {
- Ok(format!("{expr} NOT BETWEEN {low} AND {high}"))
- } else {
- Ok(format!("{expr} BETWEEN {low} AND {high}"))
- }
- }
- Expr::Like(Like {
- negated,
- expr,
- pattern,
- escape_char,
- case_insensitive,
- }) => {
- let expr = create_physical_name(expr, false)?;
- let pattern = create_physical_name(pattern, false)?;
- let op_name = if *case_insensitive { "ILIKE" } else { "LIKE" };
- let escape = if let Some(char) = escape_char {
- format!("CHAR '{char}'")
- } else {
- "".to_string()
- };
- if *negated {
- Ok(format!("{expr} NOT {op_name} {pattern}{escape}"))
- } else {
- Ok(format!("{expr} {op_name} {pattern}{escape}"))
- }
- }
- Expr::SimilarTo(Like {
- negated,
- expr,
- pattern,
- escape_char,
- case_insensitive: _,
- }) => {
- let expr = create_physical_name(expr, false)?;
- let pattern = create_physical_name(pattern, false)?;
- let escape = if let Some(char) = escape_char {
- format!("CHAR '{char}'")
- } else {
- "".to_string()
- };
- if *negated {
- Ok(format!("{expr} NOT SIMILAR TO {pattern}{escape}"))
- } else {
- Ok(format!("{expr} SIMILAR TO {pattern}{escape}"))
- }
- }
- Expr::Sort { .. } => {
- internal_err!("Create physical name does not support sort
expression")
- }
- Expr::Wildcard { qualifier, options } => match qualifier {
- Some(qualifier) => Ok(format!("{}.*{}", qualifier, options)),
- None => Ok(format!("*{}", options)),
- },
- Expr::Placeholder(_) => {
- internal_err!("Create physical name does not support placeholder")
- }
- Expr::OuterReferenceColumn(_, _) => {
- internal_err!("Create physical name does not support
OuterReferenceColumn")
- }
+/// The name of the column (field) that this `Expr` will produce in the
physical plan.
+/// The difference from [Expr::schema_name] is that top-level columns are
unqualified.
+pub fn physical_name(expr: &Expr) -> Result<String> {
+ if let Expr::Column(col) = expr {
Review Comment:
😍
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -2179,6 +2179,7 @@ mod tests {
.map(|order_by_expr| {
let ordering_req = order_by_expr.unwrap_or_default();
AggregateExprBuilder::new(array_agg_udaf(),
vec![Arc::clone(col_a)])
+ .alias("a")
Review Comment:
why is this change needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
