gene-bordegaray commented on code in PR #23623:
URL: https://github.com/apache/datafusion/pull/23623#discussion_r3595667807


##########
datafusion/sqllogictest/test_files/range_partitioning.slt:
##########
@@ -792,7 +792,7 @@ UNION ALL
 SELECT range_key, value FROM range_partitioned;
 ----
 physical_plan
-01)UnionExec
+01)InterleaveExec

Review Comment:
   could we also add:
   1. a sub set test
   2. negative test (fallback to union)
   3. union into another operator test to ensure propagation



##########
datafusion/physical-plan/src/union.rs:
##########
@@ -1288,6 +1295,66 @@ mod tests {
         );
     }
 
+    fn make_range_exec(
+        schema: &SchemaRef,
+        split_values: Vec<i32>,
+        descending: bool,
+    ) -> Result<Arc<dyn ExecutionPlan>> {
+        let sort_expr = PhysicalSortExpr::new(
+            col("a", schema)?,
+            SortOptions {
+                descending,
+                nulls_first: false,
+            },
+        );
+        let ordering = LexOrdering::new(vec![sort_expr]).unwrap();
+        let split_points = split_values
+            .into_iter()
+            .map(|v| SplitPoint::new(vec![ScalarValue::Int32(Some(v))]))
+            .collect();
+        let partitioning =
+            Partitioning::Range(RangePartitioning::try_new(ordering, 
split_points)?);
+        let base = Arc::new(TestMemoryExec::try_new(&[], Arc::clone(schema), 
None)?);
+        Ok(Arc::new(RepartitionExec::try_new(base, partitioning)?))
+    }
+
+    #[test]
+    fn test_can_interleave_range_matching() -> Result<()> {

Review Comment:
   what do you think about making this a general distribution since that seems 
to be lacking in the file. A matrixs test case like:
   1. matching hash
   2. matching range
   3. subset hash
   4. subset range
   
   then equivalently below we can have the ngative test for distributions that 
interleave shouldnt accept:
   1. mixing hash and range
   2. incompatible hashes
   3. incompatible ranges
   4. sort and null order variants for ranges
   5. etc
   
   just a thought lmk



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to