alamb commented on code in PR #5111:
URL: https://github.com/apache/arrow-datafusion/pull/5111#discussion_r1091046569
##########
datafusion/core/src/physical_plan/common.rs:
##########
@@ -368,7 +370,152 @@ mod tests {
datatypes::{DataType, Field, Schema},
record_batch::RecordBatch,
};
- use datafusion_physical_expr::expressions::col;
+ use datafusion_physical_expr::expressions::{col, Column};
+
+ #[test]
+ fn get_meet_of_orderings_helper_common_prefix_test() -> Result<()> {
+ let input1: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("c", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input2: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("y", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input3: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("x", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("y", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let expected = vec![PhysicalSortExpr {
Review Comment:
👍
##########
datafusion/core/src/physical_plan/common.rs:
##########
@@ -368,7 +370,152 @@ mod tests {
datatypes::{DataType, Field, Schema},
record_batch::RecordBatch,
};
- use datafusion_physical_expr::expressions::col;
+ use datafusion_physical_expr::expressions::{col, Column};
+
+ #[test]
+ fn get_meet_of_orderings_helper_common_prefix_test() -> Result<()> {
+ let input1: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("c", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input2: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("y", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input3: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("x", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("y", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let expected = vec![PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ }];
+
+ let result = get_meet_of_orderings_helper(vec![&input1, &input2,
&input3]);
+ assert_eq!(result.unwrap(), expected);
+ Ok(())
+ }
+
+ #[test]
+ fn get_meet_of_orderings_helper_subset_test() -> Result<()> {
+ let input1: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input2: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("c", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input3: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("d", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let result = get_meet_of_orderings_helper(vec![&input1, &input2,
&input3]);
+ assert_eq!(result.unwrap(), input1);
+ Ok(())
+ }
+
+ #[test]
+ fn get_meet_of_orderings_helper_no_overlap_test() -> Result<()> {
+ let input1: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input2: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("x", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 1)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input3: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("y", 1)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let result = get_meet_of_orderings_helper(vec![&input1, &input2,
&input3]);
+ assert!(result.is_none());
Review Comment:
👍
##########
datafusion/core/src/physical_plan/common.rs:
##########
@@ -368,7 +370,152 @@ mod tests {
datatypes::{DataType, Field, Schema},
record_batch::RecordBatch,
};
- use datafusion_physical_expr::expressions::col;
+ use datafusion_physical_expr::expressions::{col, Column};
+
+ #[test]
+ fn get_meet_of_orderings_helper_common_prefix_test() -> Result<()> {
+ let input1: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("c", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input2: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("y", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input3: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("x", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("y", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let expected = vec![PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ }];
+
+ let result = get_meet_of_orderings_helper(vec![&input1, &input2,
&input3]);
+ assert_eq!(result.unwrap(), expected);
+ Ok(())
+ }
+
+ #[test]
+ fn get_meet_of_orderings_helper_subset_test() -> Result<()> {
+ let input1: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input2: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("c", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let input3: Vec<PhysicalSortExpr> = vec![
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("a", 0)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("b", 1)),
+ options: SortOptions::default(),
+ },
+ PhysicalSortExpr {
+ expr: Arc::new(Column::new("d", 2)),
+ options: SortOptions::default(),
+ },
+ ];
+
+ let result = get_meet_of_orderings_helper(vec![&input1, &input2,
&input3]);
+ assert_eq!(result.unwrap(), input1);
Review Comment:
👍
##########
datafusion/core/src/physical_plan/union.rs:
##########
@@ -240,14 +239,20 @@ impl ExecutionPlan for UnionExec {
// which is the "meet" of all input orderings. In this example, this
// function will return vec![false, true, true], indicating that we
// preserve the orderings for the 2nd and the 3rd children.
- self.inputs()
- .iter()
- .map(|child| {
- ordering_satisfy(self.output_ordering(),
child.output_ordering(), || {
- child.equivalence_properties()
+ if let Some(output_ordering) = self.output_ordering() {
+ self.inputs()
+ .iter()
+ .map(|child| {
+ if let Some(child_ordering) = child.output_ordering() {
Review Comment:
I don't understand why we no longer need to check that the output order of
the child is compatible with the output order of the input. Shouldn't this be
calling `get_meet_of_orderings` to check rather than checking the length?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
