gokselk commented on code in PR #13805:
URL: https://github.com/apache/datafusion/pull/13805#discussion_r1889210445
##########
datafusion/physical-expr/src/equivalence/properties.rs:
##########
@@ -3651,4 +3661,38 @@ mod tests {
sort_expr
}
+
+ #[test]
+ fn test_union_constant_value_preservation() -> Result<()> {
+ let schema = Arc::new(Schema::new(vec![
+ Field::new("a", DataType::Int32, true),
+ Field::new("b", DataType::Int32, true),
+ ]));
+
+ let col_a = col("a", &schema)?;
+ let literal_10 = ScalarValue::Int32(Some(10));
+
+ // Create first input with a=10
+ let const_expr1 =
+ ConstExpr::new(Arc::clone(&col_a)).with_value(literal_10.clone());
+ let input1 = EquivalenceProperties::new(Arc::clone(&schema))
+ .with_constants(vec![const_expr1]);
+
+ // Create second input with a=10
+ let const_expr2 =
+ ConstExpr::new(Arc::clone(&col_a)).with_value(literal_10.clone());
+ let input2 = EquivalenceProperties::new(Arc::clone(&schema))
+ .with_constants(vec![const_expr2]);
+
+ // Calculate union properties
+ let union_props = calculate_union(vec![input1, input2], schema)?;
+
+ // Verify column 'a' remains constant with value 10
+ let const_a = &union_props.constants()[0];
+ assert!(const_a.expr().eq(&col_a));
+ assert!(const_a.across_partitions());
+ assert_eq!(const_a.value(), Some(&literal_10));
+
+ Ok(())
+ }
}
Review Comment:
> Can you take a look at the first check @gokselk? It should take a few line
changes in plan_with_order_preserving_variants() function. It should first look
the order requirements, and if they are matched, then it would try to convert
CoalescePartitionExec to SortPreservingMergeExec. But before that conversion,
you can check across_partitions flag of the input constants, and if it is true,
you can left the CoalescePartitionsExec as is.
I looked into this optimization but it's not currently possible with just a
few lines in `plan_with_order_preserving_variants()`. The challenge is that
`PhysicalSortExpr` doesn't track value information, which prevents us from
safely using `CoalescePartitionsExec` instead of `SortPreservingMergeExec`.
We would need to add value tracking to `PhysicalSortExpr` to implement this
optimization.
--
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
