updated TPC-H benchmark snapshots

Author: kumarUjjawal

datafusion/core/src/physical_planner.rs

@@ -2402,6 +2402,7 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter(
     e: &Expr,
     name: Option<String>,
     human_display: Option<String>,
+    human_display_is_aliased: bool,
     logical_input_schema: &DFSchema,
     physical_input_schema: &Schema,
     execution_props: &ExecutionProps,
@@ -2450,6 +2451,7 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter(
                         .order_by(order_bys.clone())
                         .schema(Arc::new(physical_input_schema.to_owned()))
                         .alias(name)
+                        .with_aliased_human_display(human_display_is_aliased)
                         .with_ignore_nulls(ignore_nulls)
                         .with_distinct(*distinct);
                 if let Some(human_display) = human_display {
@@ -2477,30 +2479,37 @@ pub fn create_aggregate_expr_and_maybe_filter(
     // Physical explain prefers the lowered aggregate form, so unwrap all alias
     // layers to recover the underlying aggregate function and then re-attach
     // only the visible output alias.
-    let (name, human_display, e) = match e {
+    let (name, human_display, human_display_is_aliased, e) = match e {
         Expr::Alias(alias) => {
             let unaliased = e.clone().unalias_nested().data;
             let human_display = unaliased.human_display().to_string();
-            let human_display = if human_display.is_empty() || human_display == alias.name
-            {
-                alias.name.clone()
-            } else {
-                format!("{human_display} as {}", alias.name)
-            };
-            (Some(alias.name.clone()), Some(human_display), unaliased)
+            let (human_display, human_display_is_aliased) =
+                if human_display.is_empty() || human_display == alias.name {
+                    (alias.name.clone(), false)
+                } else {
+                    (format!("{human_display} as {}", alias.name), true)
+                };
+            (
+                Some(alias.name.clone()),
+                Some(human_display),
+                human_display_is_aliased,
+                unaliased,
+            )
         }
         Expr::AggregateFunction(_) => (
             Some(e.schema_name().to_string()),
             Some(e.human_display().to_string()),
+            false,
             e.clone(),
         ),
-        _ => (None, None, e.clone()),
+        _ => (None, None, false, e.clone()),
     };
 
     create_aggregate_expr_with_name_and_maybe_filter(
         &e,
         name,
         human_display,
+        human_display_is_aliased,
         logical_input_schema,
         physical_input_schema,
         execution_props,

datafusion/expr/src/expr.rs

@@ -1834,11 +1834,6 @@ impl Expr {
         Expr::Alias(Alias::new(self, relation, name.into()).with_metadata(metadata))
     }
 
-    #[doc(hidden)]
-    pub fn alias_with_existing(self, alias: &Alias) -> Expr {
-        Expr::Alias(alias.clone().with_expr(self))
-    }
-
     /// Remove an alias from an expression if one exists.
     ///
     /// If the expression is not an alias, the expression is returned unchanged.

datafusion/physical-expr/src/aggregate.rs

@@ -67,6 +67,8 @@ pub struct AggregateExprBuilder {
     alias: Option<String>,
     /// A human readable name
     human_display: Option<String>,
+    /// Whether `human_display` includes an explicit `... as <alias>` suffix.
+    human_display_is_aliased: bool,
     /// Arrow Schema for the aggregate function
     schema: SchemaRef,
     /// The physical order by expressions
@@ -86,6 +88,7 @@ impl AggregateExprBuilder {
             args,
             alias: None,
             human_display: None,
+            human_display_is_aliased: false,
             schema: Arc::new(Schema::empty()),
             order_bys: vec![],
             ignore_nulls: false,
@@ -190,6 +193,7 @@ impl AggregateExprBuilder {
             args,
             alias,
             human_display,
+            human_display_is_aliased,
             schema,
             order_bys,
             ignore_nulls,
@@ -239,6 +243,7 @@ impl AggregateExprBuilder {
             return_field,
             name,
             human_display,
+            human_display_is_aliased,
             schema: Arc::unwrap_or_clone(schema),
             order_bys,
             ignore_nulls,
@@ -261,6 +266,12 @@ impl AggregateExprBuilder {
         self
     }
 
+    #[doc(hidden)]
+    pub fn with_aliased_human_display(mut self, human_display_is_aliased: bool) -> Self {
+        self.human_display_is_aliased = human_display_is_aliased;
+        self
+    }
+
     pub fn schema(mut self, schema: SchemaRef) -> Self {
         self.schema = schema;
         self
@@ -317,6 +328,8 @@ pub struct AggregateFunctionExpr {
     name: String,
     /// Simplified name for `tree` explain.
     human_display: Option<String>,
+    /// Whether `human_display` includes an explicit `... as <alias>` suffix.
+    human_display_is_aliased: bool,
     schema: Schema,
     // The physical order by expressions
     order_bys: Vec<PhysicalSortExpr>,
@@ -352,6 +365,11 @@ impl AggregateFunctionExpr {
         self.human_display.as_deref()
     }
 
+    #[doc(hidden)]
+    pub fn has_aliased_human_display(&self) -> bool {
+        self.human_display_is_aliased
+    }
+
     /// Return if the aggregation is distinct
     pub fn is_distinct(&self) -> bool {
         self.is_distinct
@@ -462,6 +480,7 @@ impl AggregateFunctionExpr {
                 .order_by(self.order_bys.clone())
                 .schema(Arc::new(self.schema.clone()))
                 .alias(self.name().to_string())
+                .with_aliased_human_display(self.human_display_is_aliased)
                 .with_ignore_nulls(self.ignore_nulls)
                 .with_distinct(self.is_distinct)
                 .with_reversed(self.is_reversed);
@@ -586,15 +605,15 @@ impl AggregateFunctionExpr {
             ReversedUDAF::NotSupported => None,
             ReversedUDAF::Identical => Some(self.clone()),
             ReversedUDAF::Reversed(reverse_udf) => {
-                let aliased_human_display = self
-                    .human_display()
-                    .and_then(|human_display| {
-                        strip_alias_suffix(human_display, self.name())
-                    })
-                    .map(str::to_string);
-                let was_aliased = aliased_human_display.is_some();
+                let was_aliased = self.has_aliased_human_display();
                 let mut name = self.name().to_string();
                 let mut human_display = self.human_display().map(str::to_string);
+                // Reversing display follows two paths:
+                // - aliased display keeps the output `name` unchanged and rewrites only
+                //   the lowered expression in `human_display`, then re-attaches
+                //   `as <alias>`.
+                // - non-aliased display rewrites the canonical `name`, and rewrites
+                //   `human_display` only when present.
                 // If the function is changed, we need to reverse order_by clause as well
                 // i.e. First(a order by b asc null first) -> Last(a order by b desc null last)
                 if !was_aliased && self.fun().name() != reverse_udf.name() {
@@ -609,8 +628,19 @@ impl AggregateFunctionExpr {
                 }
 
                 if let Some(human_display) = human_display.as_mut() {
-                    if let Some(expr_display) = aliased_human_display {
-                        *human_display = expr_display;
+                    if was_aliased {
+                        let stripped = match strip_alias_suffix(
+                            human_display,
+                            self.name(),
+                        ) {
+                            Some(stripped) => stripped.to_string(),
+                            None => panic!(
+                                "invariant violated: aliased aggregate human_display must end with ` as {}`: {}",
+                                self.name(),
+                                human_display
+                            ),
+                        };
+                        *human_display = stripped;
                     }
 
                     if self.fun().name() != reverse_udf.name() {
@@ -632,6 +662,7 @@ impl AggregateFunctionExpr {
                         .order_by(self.order_bys.iter().map(|e| e.reverse()).collect())
                         .schema(Arc::new(self.schema.clone()))
                         .alias(name)
+                        .with_aliased_human_display(was_aliased)
                         .with_ignore_nulls(self.ignore_nulls)
                         .with_distinct(self.is_distinct)
                         .with_reversed(!self.is_reversed);
@@ -693,6 +724,7 @@ impl AggregateFunctionExpr {
             name: self.name.clone(),
             // TODO: Human name should be updated after re-write to not mislead
             human_display: self.human_display.clone(),
+            human_display_is_aliased: self.human_display_is_aliased,
             schema: self.schema.clone(),
             order_bys: new_order_bys,
             ignore_nulls: self.ignore_nulls,

datafusion/physical-plan/src/aggregates/mod.rs

@@ -1447,7 +1447,7 @@ impl DisplayAs for AggregateExec {
 }
 
 fn format_aggregate_exec_expr(agg: &AggregateFunctionExpr) -> &str {
-    if has_aliased_human_display(agg) {
+    if agg.has_aliased_human_display() {
         agg.human_display().unwrap_or_else(|| agg.name())
     } else {
         agg.name()
@@ -1458,12 +1458,6 @@ fn format_tree_aggregate_expr(agg: &AggregateFunctionExpr) -> &str {
     agg.human_display().unwrap_or_else(|| agg.name())
 }
 
-fn has_aliased_human_display(agg: &AggregateFunctionExpr) -> bool {
-    agg.human_display()
-        .and_then(|human_display| human_display.strip_suffix(agg.name()))
-        .is_some_and(|prefix| prefix.ends_with(" as "))
-}
-
 impl ExecutionPlan for AggregateExec {
     fn name(&self) -> &'static str {
         "AggregateExec"
@@ -3053,6 +3047,7 @@ mod tests {
             .human_display(
                 "first_value(b) ORDER BY [b ASC NULLS LAST] as agg".to_string(),
             )
+            .with_aliased_human_display(true)
             .build()?;
 
         let reversed = agg.reverse_expr().expect("expected reverse expr");
@@ -3087,6 +3082,7 @@ mod tests {
             "first_value(first_value_col) ORDER BY [first_value_col ASC NULLS LAST] as agg"
                 .to_string(),
         )
+        .with_aliased_human_display(true)
         .build()?;
 
         let reversed = agg.reverse_expr().expect("expected reverse expr");
@@ -3121,6 +3117,29 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn test_reverse_expr_preserves_non_aliased_display_path() -> Result<()> {
+        let schema = create_test_schema()?;
+        let agg = AggregateExprBuilder::new(first_value_udaf(), vec![col("b", &schema)?])
+            .order_by(vec![PhysicalSortExpr {
+                expr: col("b", &schema)?,
+                options: SortOptions::new(false, false),
+            }])
+            .schema(Arc::clone(&schema))
+            .alias("first_value(b) ORDER BY [b ASC NULLS LAST]")
+            .build()?;
+
+        let reversed = agg.reverse_expr().expect("expected reverse expr");
+
+        assert_eq!(
+            reversed.name(),
+            "last_value(b) ORDER BY [b DESC NULLS FIRST]"
+        );
+        assert_eq!(reversed.human_display(), None);
+
+        Ok(())
+    }
+
     // This function constructs the physical plan below,
     //
     // "AggregateExec: mode=Final, gby=[a@0 as a], aggr=[FIRST_VALUE(b)]",

datafusion/proto/proto/datafusion.proto

@@ -943,6 +943,7 @@ message PhysicalAggregateExprNode {
   bool ignore_nulls = 6;
   optional bytes fun_definition = 7;
   string human_display = 8;
+  bool human_display_is_aliased = 9;
 }
 
 message PhysicalWindowExprNode {

datafusion/proto/src/generated/pbjson.rs

@@ -1264,7 +1264,10 @@ impl protobuf::PhysicalPlanNode {
                                     .with_ignore_nulls(agg_node.ignore_nulls)
                                     .with_distinct(agg_node.distinct)
                                     .order_by(order_bys)
-                                    .human_display(agg_node.human_display.clone());
+                                    .human_display(agg_node.human_display.clone())
+                                    .with_aliased_human_display(
+                                        agg_node.human_display_is_aliased,
+                                    );
                                     builder.build().map(Arc::new)
                                 }
                             })

datafusion/proto/src/generated/prost.rs

@@ -84,6 +84,7 @@ pub fn serialize_physical_aggr_expr(
                     .human_display()
                     .unwrap_or_default()
                     .to_string(),
+                human_display_is_aliased: aggr_expr.has_aliased_human_display(),
             },
         )),
     })